Skip to content

Commit 2276051

Browse files
committed
Revert "Fixing memory corruption when passing additional scopes."
This breaks the C++ native plugin, and needs to be approved. This reverts commit 0e654db. Change-Id: Ia37d4399610ad336862163f4ad7ab6c0b3c16926
1 parent 0e654db commit 2276051

File tree

3 files changed

+131
-114
lines changed

3 files changed

+131
-114
lines changed

.gitignore

-2
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ ProjectSettings/
1616
Temp/
1717
obj/
1818

19-
.DS_Store
20-
2119
# Don't check in the resolver
2220
GoogleSignInPlugin/Assets/PlayServicesResolver/
2321

staging/native/src/android/google_signin.cc

+67-55
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
1-
// Copyright (C) 2017 Google Inc. All Rights Reserved.
1+
// Copyright 2018 Google Inc. All rights reserved.
22
//
3-
// Licensed under the Apache License, Version 2.0 (the "License");
4-
// you may not use this file except in compliance with the License.
5-
// You may obtain a copy of the License at
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
66
//
7-
// http://www.apache.org/licenses/LICENSE-2.0
7+
// http://www.apache.org/licenses/LICENSE-2.0
88
//
9-
// Unless required by applicable law or agreed to in writing, software
10-
// distributed under the License is distributed on an "AS IS" BASIS,
11-
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12-
// See the License for the specific language governing permissions and
13-
// limitations under the License.
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
1414

1515
#include "google_signin.h"
1616
#include <android/log.h>
1717
#include <cassert>
1818
#include "google_signin_user_impl.h"
19-
#include "jni_init.h"
19+
#include "jni_context.h"
2020

2121
#define TAG "native-googlesignin"
2222
#define HELPER_CLASSNAME "com/google/googlesignin/GoogleSignInHelper"
@@ -85,7 +85,8 @@ public static void nativeOnResult(long requestHandle, int result,
8585
"Lcom/google/android/gms/auth/api/signin/GoogleSignInAccount;" \
8686
")V"
8787

88-
namespace googlesignin {
88+
namespace google {
89+
namespace signin {
8990

9091
class GoogleSignInFuture;
9192

@@ -94,13 +95,13 @@ class GoogleSignInFuture;
9495
// For the public methods see google_signin.h for details.
9596
class GoogleSignIn::GoogleSignInImpl {
9697
public:
97-
jobject activity_;
98+
JNIContext jni_;
9899
GoogleSignInFuture *current_result_;
99100
Configuration *current_configuration_;
100101

101102
// Constructs the implementation providing the Java activity to use when
102103
// making calls.
103-
GoogleSignInImpl(jobject activity);
104+
GoogleSignInImpl(jobject activity, JavaVM *vm);
104105
~GoogleSignInImpl();
105106

106107
void Configure(const Configuration &configuration);
@@ -121,8 +122,8 @@ class GoogleSignIn::GoogleSignInImpl {
121122
void Disconnect();
122123

123124
// Native method implementation for the Java class.
124-
static void NativeOnAuthResult(JNIEnv *env, jobject obj, jlong handle,
125-
jint result, jobject user);
125+
static void NativeOnAuthResult(JNIContext &jni_context, jobject obj,
126+
jlong handle, jint result, jobject user);
126127

127128
private:
128129
void CallConfigure();
@@ -176,18 +177,16 @@ class GoogleSignInFuture : public Future<GoogleSignIn::SignInResult> {
176177
};
177178

178179
// Constructs a new instance. The static members are initialized if need-be.
179-
GoogleSignIn::GoogleSignInImpl::GoogleSignInImpl(jobject activity)
180-
: current_result_(nullptr), current_configuration_(nullptr) {
181-
JNIEnv *env = GetJniEnv();
182-
183-
activity_ = env->NewGlobalRef(activity);
180+
GoogleSignIn::GoogleSignInImpl::GoogleSignInImpl(jobject activity, JavaVM *vm)
181+
: jni_(activity, vm), current_result_(nullptr),
182+
current_configuration_(nullptr) {
183+
JNIEnv *env = jni_.GetJniEnv();
184184

185185
if (!helper_clazz_) {
186186
// Find the java helper class and initialize it.
187-
helper_clazz_ = FindClass(HELPER_CLASSNAME, activity);
187+
helper_clazz_ = jni_.FindClass(HELPER_CLASSNAME);
188188

189189
assert(helper_clazz_);
190-
191190
if (helper_clazz_) {
192191
helper_clazz_ = (jclass)env->NewGlobalRef(helper_clazz_);
193192
env->RegisterNatives(helper_clazz_, methods,
@@ -209,71 +208,74 @@ GoogleSignIn::GoogleSignInImpl::GoogleSignInImpl(jobject activity)
209208
}
210209

211210
GoogleSignIn::GoogleSignInImpl::~GoogleSignInImpl() {
212-
JNIEnv *env = GetJniEnv();
213-
214-
env->DeleteGlobalRef(activity_);
215-
activity_ = nullptr;
216211
delete current_result_;
217212
current_result_ = nullptr;
218213
}
219214

220215
void GoogleSignIn::GoogleSignInImpl::EnableDebugLogging(bool flag) {
221-
JNIEnv *env = GetJniEnv();
216+
JNIEnv *env = jni_.GetJniEnv();
222217

223218
env->CallStaticVoidMethod(helper_clazz_, enable_debug_method_, flag);
224-
225219
}
226220

227221
void GoogleSignIn::GoogleSignInImpl::Configure(
228222
const Configuration &configuration) {
229223
delete current_configuration_;
230224
current_configuration_ = new Configuration(configuration);
231225

226+
if (configuration.web_client_id) {
227+
current_configuration_->web_client_id = strdup(configuration.web_client_id);
228+
}
229+
232230
delete current_result_;
233231
current_result_ = new GoogleSignInFuture();
234232

235233
CallConfigure();
236234
}
237235

238236
void GoogleSignIn::GoogleSignInImpl::CallConfigure() {
239-
JNIEnv *env = GetJniEnv();
237+
JNIEnv *env = jni_.GetJniEnv();
240238

241239
if (!current_configuration_) {
242240
__android_log_print(ANDROID_LOG_ERROR, TAG, "configuration is null!?");
243241
return;
244242
}
245243
jstring j_web_client_id =
246-
current_configuration_->web_client_id.empty() ? nullptr
247-
: env->NewStringUTF(current_configuration_->web_client_id.c_str());
244+
current_configuration_->web_client_id
245+
? env->NewStringUTF(current_configuration_->web_client_id)
246+
: nullptr;
248247

249248
jstring j_account_name =
250-
current_configuration_->account_name.empty() ? nullptr
251-
: env->NewStringUTF(current_configuration_->account_name.c_str());
249+
current_configuration_->account_name
250+
? env->NewStringUTF(current_configuration_->account_name)
251+
: nullptr;
252252

253253
jobjectArray j_auth_scopes = nullptr;
254254

255-
if (current_configuration_->additional_scopes.size() > 0) {
256-
jclass string_clazz = FindClass("java/lang/String", activity_);
255+
if (current_configuration_->additional_scope_count > 0) {
256+
jclass string_clazz = jni_.FindClass("java/lang/String");
257257
j_auth_scopes = env->NewObjectArray(
258-
current_configuration_->additional_scopes.size(), string_clazz, nullptr);
258+
current_configuration_->additional_scope_count, string_clazz, nullptr);
259259

260-
for (int i = 0; i < current_configuration_->additional_scopes.size(); i++) {
260+
for (int i = 0; i < current_configuration_->additional_scope_count; i++) {
261261
env->SetObjectArrayElement(
262262
j_auth_scopes, i,
263-
env->NewStringUTF(current_configuration_->additional_scopes[i].c_str()));
263+
env->NewStringUTF(current_configuration_->additional_scopes[i]));
264264
}
265265
env->DeleteLocalRef(string_clazz);
266266
}
267267

268+
jobject j_activity = jni_.GetActivity();
268269
env->CallStaticVoidMethod(
269-
helper_clazz_, config_method_, activity_,
270+
helper_clazz_, config_method_, j_activity,
270271
current_configuration_->use_game_signin, j_web_client_id,
271272
current_configuration_->request_auth_code,
272273
current_configuration_->force_token_refresh,
273274
current_configuration_->request_email,
274275
current_configuration_->request_id_token,
275276
current_configuration_->hide_ui_popups, j_account_name, j_auth_scopes,
276277
reinterpret_cast<jlong>(current_result_));
278+
env->DeleteLocalRef(j_activity);
277279

278280
if (j_web_client_id) {
279281
env->DeleteLocalRef(j_web_client_id);
@@ -289,32 +291,36 @@ void GoogleSignIn::GoogleSignInImpl::CallConfigure() {
289291
}
290292

291293
Future<GoogleSignIn::SignInResult> &GoogleSignIn::GoogleSignInImpl::SignIn() {
292-
JNIEnv *env = GetJniEnv();
294+
JNIEnv *env = jni_.GetJniEnv();
293295

294296
if (current_result_) {
295297
current_result_->SetResult(nullptr);
296298
}
297299

298300
CallConfigure();
299301

300-
env->CallStaticVoidMethod(helper_clazz_, signin_method_, activity_,
302+
jobject j_activity = jni_.GetActivity();
303+
env->CallStaticVoidMethod(helper_clazz_, signin_method_, j_activity,
301304
(jlong)current_result_);
305+
env->DeleteLocalRef(j_activity);
302306

303307
return *current_result_;
304308
}
305309

306310
Future<GoogleSignIn::SignInResult>
307311
&GoogleSignIn::GoogleSignInImpl::SignInSilently() {
308-
JNIEnv *env = GetJniEnv();
312+
JNIEnv *env = jni_.GetJniEnv();
309313

310314
if (current_result_) {
311315
current_result_->SetResult(nullptr);
312316
}
313317

314318
CallConfigure();
315319

316-
env->CallStaticVoidMethod(helper_clazz_, signinsilently_method_, activity_,
320+
jobject j_activity = jni_.GetActivity();
321+
env->CallStaticVoidMethod(helper_clazz_, signinsilently_method_, j_activity,
317322
(jlong)current_result_);
323+
env->DeleteLocalRef(j_activity);
318324

319325
return *current_result_;
320326
}
@@ -327,30 +333,34 @@ const Future<GoogleSignIn::SignInResult>
327333

328334
// Signs out.
329335
void GoogleSignIn::GoogleSignInImpl::SignOut() {
330-
JNIEnv *env = GetJniEnv();
336+
JNIEnv *env = jni_.GetJniEnv();
331337

338+
jobject j_activity = jni_.GetActivity();
332339
__android_log_print(ANDROID_LOG_INFO, TAG,
333340
"helper: %x method: %x activity: %x",
334341
(uintptr_t)helper_clazz_, (uintptr_t)signin_method_,
335-
(uintptr_t)activity_);
342+
(uintptr_t)j_activity);
336343

337-
env->CallStaticVoidMethod(helper_clazz_, signout_method_, activity_);
344+
env->CallStaticVoidMethod(helper_clazz_, signout_method_, j_activity);
345+
env->DeleteLocalRef(j_activity);
338346
}
339347

340348
// Signs out.
341349
void GoogleSignIn::GoogleSignInImpl::Disconnect() {
342-
JNIEnv *env = GetJniEnv();
350+
JNIEnv *env = jni_.GetJniEnv();
343351

344-
env->CallStaticVoidMethod(helper_clazz_, disconnect_method_, activity_);
352+
jobject j_activity = jni_.GetActivity();
353+
env->CallStaticVoidMethod(helper_clazz_, disconnect_method_, j_activity);
354+
env->DeleteLocalRef(j_activity);
345355
}
346356

347-
void GoogleSignIn::GoogleSignInImpl::NativeOnAuthResult(
348-
JNIEnv *env, jobject obj, jlong handle, jint result, jobject user) {
357+
void GoogleSignIn::GoogleSignInImpl::NativeOnAuthResult(JNIContext &jni_context,
358+
jobject obj, jlong handle, jint result, jobject user) {
349359
GoogleSignInFuture *future = reinterpret_cast<GoogleSignInFuture *>(handle);
350360
if (future) {
351361
SignInResult *rc = new GoogleSignIn::SignInResult();
352362
rc->StatusCode = result;
353-
rc->User = GoogleSignInUserImpl::UserFromAccount(user);
363+
rc->User = GoogleSignInUserImpl::UserFromAccount(jni_context, user);
354364

355365
if (rc->User) {
356366
__android_log_print(ANDROID_LOG_INFO, TAG, "User Display Name is %s",
@@ -362,8 +372,9 @@ void GoogleSignIn::GoogleSignInImpl::NativeOnAuthResult(
362372

363373
// Public class implementation. These are called by external callers to use the
364374
// Google Sign-in API.
365-
GoogleSignIn::GoogleSignIn(jobject activity)
366-
: impl_(new GoogleSignInImpl(activity)) {}
375+
376+
GoogleSignIn::GoogleSignIn(jobject activity, JavaVM *vm)
377+
: impl_(new GoogleSignInImpl(activity, vm)) {}
367378

368379
void GoogleSignIn::EnableDebugLogging(bool flag) {
369380
impl_->EnableDebugLogging(flag);
@@ -389,4 +400,5 @@ void GoogleSignIn::SignOut() { impl_->SignOut(); }
389400

390401
void GoogleSignIn::Disconnect() { impl_->Disconnect(); }
391402

392-
} // namespace googlesignin
403+
} // namespace signin
404+
} // namespace google

0 commit comments

Comments
 (0)