Skip to content

Commit

Permalink
Fix memory leak in LoginManager and AppEventsLogger
Browse files Browse the repository at this point in the history
LoginManager's context and loginLogger both hold a reference to the
activity starting the login. This activity is leaked if the login process
is canceled. It is also leaked if finishLogin() is never called, e.g. when
the device's configuration/orientation is changed. finishLogin() only nulls
out context and loginLogger in case of a successful login.

This commit updates LoginManager and AppEventsLogger, so that it is not
required anymore to hold a reference to the activity starting the login.
  • Loading branch information
segoh committed Jun 2, 2015
1 parent 1cd38e2 commit b4c67f8
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 22 deletions.
12 changes: 6 additions & 6 deletions facebook/src/com/facebook/appevents/AppEventsLogger.java
Expand Up @@ -177,7 +177,7 @@ public enum FlushBehavior {
"_fbSourceApplicationHasBeenSet";

// Instance member variables
private final Context context;
private final String uiName;
private final AccessTokenAppIdPair accessTokenAppId;

private static Map<AccessTokenAppIdPair, SessionEventsState> stateMap =
Expand Down Expand Up @@ -650,7 +650,7 @@ private enum FlushResult {
*/
private AppEventsLogger(Context context, String applicationId, AccessToken accessToken) {
Validate.notNull(context, "context");
this.context = context;
uiName = Utility.getActivityName(context);

if (accessToken == null) {
accessToken = AccessToken.getCurrentAccessToken();
Expand Down Expand Up @@ -732,12 +732,12 @@ private void logEvent(
Bundle parameters,
boolean isImplicitlyLogged) {
AppEvent event = new AppEvent(
this.context,
uiName,
eventName,
valueToSum,
parameters,
isImplicitlyLogged);
logEvent(context, event, accessTokenAppId);
logEvent(applicationContext, event, accessTokenAppId);
}

private static void logEvent(final Context context,
Expand Down Expand Up @@ -1290,7 +1290,7 @@ static class AppEvent implements Serializable {
private String name;

public AppEvent(
Context context,
String uiName,
String eventName,
Double valueToSum,
Bundle parameters,
Expand All @@ -1305,7 +1305,7 @@ public AppEvent(

jsonObject.put("_eventName", eventName);
jsonObject.put("_logTime", System.currentTimeMillis() / 1000);
jsonObject.put("_ui", Utility.getActivityName(context));
jsonObject.put("_ui", uiName);

if (valueToSum != null) {
jsonObject.put("_valueToSum", valueToSum.doubleValue());
Expand Down
30 changes: 14 additions & 16 deletions facebook/src/com/facebook/login/LoginManager.java
Expand Up @@ -63,7 +63,6 @@ public class LoginManager {
private DefaultAudience defaultAudience = DefaultAudience.FRIENDS;
private LoginClient.Request pendingLoginRequest;
private HashMap<String, String> pendingLoggingExtras;
private Context context;
private LoginLogger loginLogger;

LoginManager() {
Expand Down Expand Up @@ -355,7 +354,7 @@ private void startLogin(

this.pendingLoginRequest = request;
this.pendingLoggingExtras = new HashMap<>();
this.context = startActivityDelegate.getActivityContext();
this.loginLogger = getLoggerForContext(startActivityDelegate.getActivityContext());

logStartLogin();

Expand Down Expand Up @@ -388,31 +387,36 @@ public boolean onActivityResult(int resultCode, Intent data) {
}
}

private LoginLogger getLogger() {
private LoginLogger getLoggerForContext(Context context) {
if (loginLogger == null ||
!loginLogger.getApplicationId().equals(
pendingLoginRequest.getApplicationId())) {
loginLogger = new LoginLogger(
return new LoginLogger(
context,
pendingLoginRequest.getApplicationId());
}
return loginLogger;
}

private void logStartLogin() {
getLogger().logStartLogin(pendingLoginRequest);
if (loginLogger != null) {
loginLogger.logStartLogin(pendingLoginRequest);
}
}

private void logCompleteLogin(LoginClient.Result.Code result, Map<String, String> resultExtras,
Exception exception) {
if (loginLogger == null) {
return;
}
if (pendingLoginRequest == null) {
// We don't expect this to happen, but if it does, log an event for diagnostic purposes.
getLogger().logUnexpectedError(
LoginLogger.EVENT_NAME_LOGIN_COMPLETE,
"Unexpected call to logCompleteLogin with null pendingAuthorizationRequest."
loginLogger.logUnexpectedError(
LoginLogger.EVENT_NAME_LOGIN_COMPLETE,
"Unexpected call to logCompleteLogin with null pendingAuthorizationRequest."
);
} else {
getLogger().logCompleteLogin(
loginLogger.logCompleteLogin(
pendingLoginRequest.getAuthId(),
pendingLoggingExtras,
result,
Expand Down Expand Up @@ -501,18 +505,12 @@ private void finishLogin(
|| (loginResult != null
&& loginResult.getRecentlyGrantedPermissions().size() == 0)) {
callback.onCancel();
return;
}
if (exception != null) {
} else if (exception != null) {
callback.onError(exception);
} else if (newToken != null) {
callback.onSuccess(loginResult);
}
}

// Cleanup saved context to avoid leaking
this.context = null;
this.loginLogger = null;
}

private static class ActivityStartActivityDelegate implements StartActivityDelegate {
Expand Down

0 comments on commit b4c67f8

Please sign in to comment.