Skip to content

Commit 7944823

Browse files
SUPERCILEXsamtstern
SUPERCILEX
authored and
samtstern
committed
Fb mem leak + remove BuildConfig.DEBUG (#442)
1 parent 44a4f27 commit 7944823

File tree

5 files changed

+67
-56
lines changed

5 files changed

+67
-56
lines changed

‎auth/src/main/java/com/firebase/ui/auth/provider/FacebookProvider.java

Lines changed: 52 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import com.facebook.login.LoginManager;
3232
import com.facebook.login.LoginResult;
3333
import com.firebase.ui.auth.AuthUI;
34-
import com.firebase.ui.auth.BuildConfig;
3534
import com.firebase.ui.auth.IdpResponse;
3635
import com.firebase.ui.auth.R;
3736
import com.google.firebase.auth.AuthCredential;
@@ -45,13 +44,15 @@
4544

4645
public class FacebookProvider implements IdpProvider, FacebookCallback<LoginResult> {
4746
private static final String TAG = "FacebookProvider";
48-
protected static final String ERROR = "err";
49-
protected static final String ERROR_MSG = "err_msg";
5047
private static final String EMAIL = "email";
5148
private static final String PUBLIC_PROFILE = "public_profile";
52-
private static final CallbackManager CALLBACK_MANAGER = CallbackManager.Factory.create();
49+
private static final String ERROR = "err";
50+
private static final String ERROR_MSG = "err_msg";
51+
52+
private static CallbackManager sCallbackManager;
5353

5454
private final List<String> mScopes;
55+
// DO NOT USE DIRECTLY: see onSuccess(String, LoginResult) and onFailure(Bundle) below
5556
private IdpCallback mCallbackObject;
5657

5758
public FacebookProvider(Context appContext, AuthUI.IdpConfig idpConfig, @StyleRes int theme) {
@@ -86,8 +87,9 @@ public String getProviderId() {
8687

8788
@Override
8889
public void startLogin(Activity activity) {
90+
sCallbackManager = CallbackManager.Factory.create();
8991
LoginManager loginManager = LoginManager.getInstance();
90-
loginManager.registerCallback(CALLBACK_MANAGER, this);
92+
loginManager.registerCallback(sCallbackManager, this);
9193

9294
List<String> permissionsList = new ArrayList<>(mScopes);
9395

@@ -111,18 +113,11 @@ public void setAuthenticationCallback(IdpCallback callback) {
111113

112114
@Override
113115
public void onActivityResult(int requestCode, int resultCode, Intent data) {
114-
CALLBACK_MANAGER.onActivityResult(requestCode, resultCode, data);
116+
sCallbackManager.onActivityResult(requestCode, resultCode, data);
115117
}
116118

117119
@Override
118120
public void onSuccess(final LoginResult loginResult) {
119-
if (BuildConfig.DEBUG) {
120-
Log.d(TAG, "Login to facebook successful with Application Id: "
121-
+ loginResult.getAccessToken().getApplicationId()
122-
+ " with Token: "
123-
+ loginResult.getAccessToken().getToken());
124-
}
125-
126121
GraphRequest request = GraphRequest.newMeRequest(
127122
loginResult.getAccessToken(),
128123
new GraphRequest.GraphJSONObjectCallback() {
@@ -131,19 +126,19 @@ public void onCompleted(JSONObject object, GraphResponse response) {
131126
FacebookRequestError requestError = response.getError();
132127
if (requestError != null) {
133128
Log.e(TAG, "Received Facebook error: " + requestError.getErrorMessage());
134-
mCallbackObject.onFailure(new Bundle());
129+
onFailure(new Bundle());
135130
return;
136131
}
137132
if (object == null) {
138133
Log.w(TAG, "Received null response from Facebook GraphRequest");
139-
mCallbackObject.onFailure(new Bundle());
134+
onFailure(new Bundle());
140135
} else {
141136
try {
142137
String email = object.getString("email");
143-
mCallbackObject.onSuccess(createIDPResponse(loginResult, email));
138+
onSuccess(email, loginResult);
144139
} catch (JSONException e) {
145140
Log.e(TAG, "JSON Exception reading from Facebook GraphRequest", e);
146-
mCallbackObject.onFailure(new Bundle());
141+
onFailure(new Bundle());
147142
}
148143
}
149144
}
@@ -155,27 +150,11 @@ public void onCompleted(JSONObject object, GraphResponse response) {
155150
request.executeAsync();
156151
}
157152

158-
private IdpResponse createIDPResponse(LoginResult loginResult, String email) {
159-
return new IdpResponse(
160-
FacebookAuthProvider.PROVIDER_ID,
161-
email,
162-
loginResult.getAccessToken().getToken());
163-
}
164-
165-
public static AuthCredential createAuthCredential(IdpResponse response) {
166-
if (!response.getProviderType().equals(FacebookAuthProvider.PROVIDER_ID)) {
167-
return null;
168-
}
169-
return FacebookAuthProvider
170-
.getCredential(response.getIdpToken());
171-
}
172-
173153
@Override
174154
public void onCancel() {
175155
Bundle extra = new Bundle();
176156
extra.putString(ERROR, "cancelled");
177-
mCallbackObject.onFailure(extra);
178-
157+
onFailure(extra);
179158
}
180159

181160
@Override
@@ -184,6 +163,44 @@ public void onError(FacebookException error) {
184163
Bundle extra = new Bundle();
185164
extra.putString(ERROR, "error");
186165
extra.putString(ERROR_MSG, error.getMessage());
187-
mCallbackObject.onFailure(extra);
166+
onFailure(extra);
167+
}
168+
169+
private IdpResponse createIdpResponse(String email, LoginResult loginResult) {
170+
return new IdpResponse(
171+
FacebookAuthProvider.PROVIDER_ID,
172+
email,
173+
loginResult.getAccessToken().getToken());
174+
}
175+
176+
private void onSuccess(String email, LoginResult loginResult) {
177+
gcCallbackManager();
178+
mCallbackObject.onSuccess(createIdpResponse(email, loginResult));
179+
}
180+
181+
private void onFailure(Bundle bundle) {
182+
gcCallbackManager();
183+
mCallbackObject.onFailure(bundle);
184+
}
185+
186+
private void gcCallbackManager() {
187+
// sCallbackManager must be static to prevent it from being destroyed if the activity
188+
// containing FacebookProvider dies.
189+
// In startLogin(Activity), LoginManager#registerCallback(CallbackManager, FacebookCallback)
190+
// stores the FacebookCallback parameter--in this case a FacebookProvider instance--into
191+
// a HashMap in the CallbackManager instance, sCallbackManager.
192+
// Because FacebookProvider which contains an instance of an activity, mCallbackObject,
193+
// is contained in sCallbackManager, that activity will not be garbage collected.
194+
// Thus, we have leaked an Activity.
195+
sCallbackManager = null;
196+
}
197+
198+
199+
public static AuthCredential createAuthCredential(IdpResponse response) {
200+
if (!response.getProviderType().equals(FacebookAuthProvider.PROVIDER_ID)) {
201+
return null;
202+
}
203+
return FacebookAuthProvider
204+
.getCredential(response.getIdpToken());
188205
}
189206
}

‎auth/src/main/java/com/firebase/ui/auth/provider/GoogleProvider.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public void disconnect() {
111111
}
112112
}
113113

114-
private IdpResponse createIDPResponse(GoogleSignInAccount account) {
114+
private IdpResponse createIdpResponse(GoogleSignInAccount account) {
115115
return new IdpResponse(
116116
GoogleAuthProvider.PROVIDER_ID, account.getEmail(), account.getIdToken());
117117
}
@@ -122,7 +122,7 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) {
122122
GoogleSignInResult result = Auth.GoogleSignInApi.getSignInResultFromIntent(data);
123123
if (result != null) {
124124
if (result.isSuccess()) {
125-
mIDPCallback.onSuccess(createIDPResponse(result.getSignInAccount()));
125+
mIDPCallback.onSuccess(createIdpResponse(result.getSignInAccount()));
126126
} else {
127127
onError(result);
128128
}

‎auth/src/main/java/com/firebase/ui/auth/provider/TwitterProvider.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public void startLogin(Activity activity) {
6161

6262
@Override
6363
public void success(Result<TwitterSession> result) {
64-
mCallbackObject.onSuccess(createIDPResponse(result.data));
64+
mCallbackObject.onSuccess(createIdpResponse(result.data));
6565
}
6666

6767
@Override
@@ -70,6 +70,15 @@ public void failure(TwitterException exception) {
7070
mCallbackObject.onFailure(new Bundle());
7171
}
7272

73+
private IdpResponse createIdpResponse(TwitterSession twitterSession) {
74+
return new IdpResponse(
75+
TwitterAuthProvider.PROVIDER_ID,
76+
null,
77+
twitterSession.getAuthToken().token,
78+
twitterSession.getAuthToken().secret);
79+
}
80+
81+
7382
public static AuthCredential createAuthCredential(IdpResponse response) {
7483
if (!response.getProviderType().equalsIgnoreCase(TwitterAuthProvider.PROVIDER_ID)){
7584
return null;
@@ -78,13 +87,4 @@ public static AuthCredential createAuthCredential(IdpResponse response) {
7887
response.getIdpToken(),
7988
response.getIdpSecret());
8089
}
81-
82-
83-
private IdpResponse createIDPResponse(TwitterSession twitterSession) {
84-
return new IdpResponse(
85-
TwitterAuthProvider.PROVIDER_ID,
86-
null,
87-
twitterSession.getAuthToken().token,
88-
twitterSession.getAuthToken().secret);
89-
}
9090
}

‎auth/src/main/java/com/firebase/ui/auth/ui/idp/AuthMethodPickerActivity.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525

2626
import com.firebase.ui.auth.AuthUI;
2727
import com.firebase.ui.auth.AuthUI.IdpConfig;
28-
import com.firebase.ui.auth.BuildConfig;
2928
import com.firebase.ui.auth.IdpResponse;
3029
import com.firebase.ui.auth.R;
3130
import com.firebase.ui.auth.ResultCodes;
@@ -102,10 +101,8 @@ private void populateIdpList(List<IdpConfig> providers) {
102101
findViewById(R.id.email_provider).setVisibility(View.VISIBLE);
103102
break;
104103
default:
105-
if (BuildConfig.DEBUG) {
106-
Log.d(TAG, "Encountered unknown IDPProvider parcel with type: "
107-
+ idpConfig.getProviderId());
108-
}
104+
Log.e(TAG, "Encountered unknown IDPProvider parcel with type: "
105+
+ idpConfig.getProviderId());
109106
}
110107
}
111108

‎auth/src/main/java/com/firebase/ui/auth/util/FirebaseAuthWrapperImpl.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import android.support.v4.app.FragmentActivity;
2424
import android.util.Log;
2525

26-
import com.firebase.ui.auth.BuildConfig;
2726
import com.google.android.gms.auth.api.Auth;
2827
import com.google.android.gms.auth.api.credentials.CredentialPickerConfig;
2928
import com.google.android.gms.auth.api.credentials.HintRequest;
@@ -207,9 +206,7 @@ private <T> T await(@NonNull Task<T> curTask) {
207206
Tasks.await(curTask, mTimeoutMs, TimeUnit.MILLISECONDS);
208207
return curTask.getResult();
209208
} catch (ExecutionException | InterruptedException | TimeoutException e) {
210-
if (BuildConfig.DEBUG) {
211-
Log.w(TAG, "API request dispatch failed", e);
212-
}
209+
Log.w(TAG, "API request dispatch failed", e);
213210
return null;
214211
}
215212
}

0 commit comments

Comments
 (0)