-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Completely rewrite CredentialsApiHelper.java and hopefully fix memory leak #566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ba0091d
43bcaed
ec84f7d
3ba8b69
499e719
a57bf05
26a3aea
0e3400d
782800c
530e753
15fbed3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,12 +23,15 @@ | |
import android.support.annotation.Nullable; | ||
import android.support.annotation.StyleRes; | ||
import android.support.annotation.VisibleForTesting; | ||
import android.support.v4.app.FragmentActivity; | ||
|
||
import com.facebook.login.LoginManager; | ||
import com.firebase.ui.auth.ui.FlowParameters; | ||
import com.firebase.ui.auth.ui.idp.AuthMethodPickerActivity; | ||
import com.firebase.ui.auth.util.CredentialTaskApi; | ||
import com.firebase.ui.auth.util.CredentialsApiHelper; | ||
import com.firebase.ui.auth.util.GoogleApiClientTaskHelper; | ||
import com.firebase.ui.auth.util.GoogleSignInHelper; | ||
import com.firebase.ui.auth.util.Preconditions; | ||
import com.firebase.ui.auth.util.signincontainer.SmartLockBase; | ||
import com.google.android.gms.auth.api.Auth; | ||
|
@@ -313,10 +316,12 @@ public static int getDefaultTheme() { | |
* Signs the current user out, if one is signed in. | ||
* | ||
* @param activity The activity requesting the user be signed out. | ||
* @return a task which, upon completion, signals that the user has been signed out | ||
* ({@code result.isSuccess()}, or that the sign-out attempt failed unexpectedly | ||
* ({@code !result.isSuccess()}). | ||
* @return a task which, upon completion, signals that the user has been signed out ({@code | ||
* result.isSuccess()}, or that the sign-out attempt failed unexpectedly ({@code | ||
* !result.isSuccess()}). | ||
* @deprecated use {@link #signOut(FragmentActivity)} instead | ||
*/ | ||
@Deprecated | ||
public Task<Void> signOut(@NonNull Activity activity) { | ||
// Get helper for Google Sign In and Credentials API | ||
GoogleApiClientTaskHelper taskHelper = GoogleApiClientTaskHelper.getInstance(activity); | ||
|
@@ -325,7 +330,7 @@ public Task<Void> signOut(@NonNull Activity activity) { | |
.addApi(Auth.GOOGLE_SIGN_IN_API, GoogleSignInOptions.DEFAULT_SIGN_IN); | ||
|
||
// Get Credentials Helper | ||
CredentialsApiHelper credentialsHelper = CredentialsApiHelper.getInstance(taskHelper); | ||
CredentialTaskApi credentialsHelper = CredentialsApiHelper.getInstance(taskHelper); | ||
|
||
// Firebase Sign out | ||
mAuth.signOut(); | ||
|
@@ -359,8 +364,62 @@ public Void then(@NonNull Task<GoogleApiClient> task) throws Exception { | |
* silently. | ||
* | ||
* @param activity the calling {@link Activity}. | ||
* @deprecated use {@link #delete(FragmentActivity)} instead | ||
*/ | ||
@Deprecated | ||
public Task<Void> delete(@NonNull Activity activity) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I can tell the difference between Could this all be simplified by having both of the
And then these two There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @samtstern To address this comment and this one, I didn't add an interface or a good name because my intention for us to throw this away in FBUI There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2.0 could be a while away, let's try and keep things clean in the meantime. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @samtstern SGTM, I'll leave things deprecated so we know to remove that stuff if we go that route, but I keep things clean in the meantime. |
||
// Initialize SmartLock helper | ||
GoogleApiClientTaskHelper gacHelper = GoogleApiClientTaskHelper.getInstance(activity); | ||
gacHelper.getBuilder().addApi(Auth.CREDENTIALS_API); | ||
CredentialTaskApi credentialHelper = CredentialsApiHelper.getInstance(gacHelper); | ||
|
||
return getDeleteTask(credentialHelper); | ||
} | ||
|
||
/** | ||
* Signs the current user out, if one is signed in. | ||
* | ||
* @param activity the activity requesting the user be signed out | ||
* @return A task which, upon completion, signals that the user has been signed out ({@link | ||
* Task#isSuccessful()}, or that the sign-out attempt failed unexpectedly !{@link | ||
* Task#isSuccessful()}). | ||
*/ | ||
public Task<Void> signOut(@NonNull FragmentActivity activity) { | ||
// Get Credentials Helper | ||
GoogleSignInHelper credentialsHelper = GoogleSignInHelper.getInstance(activity); | ||
|
||
// Firebase Sign out | ||
mAuth.signOut(); | ||
|
||
// Disable credentials auto sign-in | ||
Task<Status> disableCredentialsTask = credentialsHelper.disableAutoSignIn(); | ||
|
||
// Google sign out | ||
Task<Status> signOutTask = credentialsHelper.signOut(); | ||
|
||
// Facebook sign out | ||
LoginManager.getInstance().logOut(); | ||
|
||
// Wait for all tasks to complete | ||
return Tasks.whenAll(disableCredentialsTask, signOutTask); | ||
} | ||
|
||
/** | ||
* Delete the use from FirebaseAuth and delete any associated credentials from the Credentials | ||
* API. Returns a {@link Task} that succeeds if the Firebase Auth user deletion succeeds and | ||
* fails if the Firebase Auth deletion fails. Credentials deletion failures are handled | ||
* silently. | ||
* | ||
* @param activity the calling {@link Activity}. | ||
*/ | ||
public Task<Void> delete(@NonNull FragmentActivity activity) { | ||
// Initialize SmartLock helper | ||
CredentialTaskApi credentialHelper = GoogleSignInHelper.getInstance(activity); | ||
|
||
return getDeleteTask(credentialHelper); | ||
} | ||
|
||
private Task<Void> getDeleteTask(CredentialTaskApi credentialHelper) { | ||
FirebaseUser firebaseUser = FirebaseAuth.getInstance().getCurrentUser(); | ||
if (firebaseUser == null) { | ||
// If the current user is null, return a failed task immediately | ||
|
@@ -370,11 +429,6 @@ public Task<Void> delete(@NonNull Activity activity) { | |
// Delete the Firebase user | ||
Task<Void> deleteUserTask = firebaseUser.delete(); | ||
|
||
// Initialize SmartLock helper | ||
GoogleApiClientTaskHelper gacHelper = GoogleApiClientTaskHelper.getInstance(activity); | ||
gacHelper.getBuilder().addApi(Auth.CREDENTIALS_API); | ||
CredentialsApiHelper credentialHelper = CredentialsApiHelper.getInstance(gacHelper); | ||
|
||
// Get all SmartLock credentials associated with the user | ||
List<Credential> credentials = SmartLockBase.credentialsFromFirebaseUser(firebaseUser); | ||
|
||
|
@@ -557,8 +611,8 @@ public SignInIntentBuilder setTosUrl(@Nullable String tosUrl) { | |
* <p>If no providers are explicitly specified by calling this method, then the email | ||
* provider is the default supported provider. | ||
* | ||
* @param idpConfigs a list of {@link IdpConfig}s, where each {@link IdpConfig} contains | ||
* the configuration parameters for the IDP. | ||
* @param idpConfigs a list of {@link IdpConfig}s, where each {@link IdpConfig} contains the | ||
* configuration parameters for the IDP. | ||
* @see IdpConfig | ||
*/ | ||
public SignInIntentBuilder setProviders(@NonNull List<IdpConfig> idpConfigs) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package com.firebase.ui.auth.util; | ||
|
||
import com.google.android.gms.auth.api.credentials.Credential; | ||
import com.google.android.gms.common.api.Status; | ||
import com.google.android.gms.tasks.Task; | ||
|
||
public interface CredentialTaskApi { | ||
Task<Status> disableAutoSignIn(); | ||
|
||
Task<Status> delete(Credential credential); | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
package com.firebase.ui.auth.util; | ||
|
||
import android.os.Bundle; | ||
import android.support.annotation.NonNull; | ||
import android.support.annotation.Nullable; | ||
import android.support.v4.app.FragmentActivity; | ||
|
||
import com.google.android.gms.common.ConnectionResult; | ||
import com.google.android.gms.common.api.GoogleApiClient; | ||
import com.google.android.gms.common.api.Result; | ||
import com.google.android.gms.common.api.ResultCallback; | ||
import com.google.android.gms.tasks.OnCompleteListener; | ||
import com.google.android.gms.tasks.OnSuccessListener; | ||
import com.google.android.gms.tasks.Task; | ||
import com.google.android.gms.tasks.TaskCompletionSource; | ||
|
||
import java.net.ConnectException; | ||
import java.util.concurrent.atomic.AtomicInteger; | ||
|
||
/** | ||
* A {@link Task} based wrapper to get a connect {@link GoogleApiClient}. | ||
*/ | ||
public abstract class GoogleApiHelper implements GoogleApiClient.ConnectionCallbacks, GoogleApiClient.OnConnectionFailedListener { | ||
private static final AtomicInteger SAFE_ID = new AtomicInteger(0); | ||
|
||
protected GoogleApiClient mClient; | ||
private TaskCompletionSource<Bundle> mGoogleApiConnectionTask = new TaskCompletionSource<>(); | ||
|
||
protected GoogleApiHelper(FragmentActivity activity, GoogleApiClient.Builder builder) { | ||
builder.enableAutoManage(activity, getSafeAutoManageId(), this); | ||
builder.addConnectionCallbacks(this); | ||
mClient = builder.build(); | ||
} | ||
|
||
public static int getSafeAutoManageId() { | ||
return SAFE_ID.getAndIncrement(); | ||
} | ||
|
||
public Task<Bundle> getConnectedApiTask() { | ||
return mGoogleApiConnectionTask.getTask(); | ||
} | ||
|
||
@Override | ||
public void onConnected(@Nullable Bundle bundle) { | ||
mGoogleApiConnectionTask.setResult(bundle); | ||
} | ||
|
||
@Override | ||
public void onConnectionSuspended(int i) { | ||
// Just wait | ||
} | ||
|
||
@Override | ||
public void onConnectionFailed(@NonNull ConnectionResult result) { | ||
mGoogleApiConnectionTask.setException(new ConnectException(result.toString())); | ||
} | ||
|
||
protected static final class TaskResultCaptor<R extends Result> implements ResultCallback<R> { | ||
private TaskCompletionSource<R> mSource; | ||
|
||
public TaskResultCaptor(TaskCompletionSource<R> source) { | ||
mSource = source; | ||
} | ||
|
||
@Override | ||
public void onResult(@NonNull R result) { | ||
mSource.setResult(result); | ||
} | ||
} | ||
|
||
protected static class ExceptionForwarder<TResult> implements OnCompleteListener<TResult> { | ||
private TaskCompletionSource mSource; | ||
private OnSuccessListener<TResult> mListener; | ||
|
||
public ExceptionForwarder(TaskCompletionSource source, | ||
OnSuccessListener<TResult> listener) { | ||
mSource = source; | ||
mListener = listener; | ||
} | ||
|
||
@Override | ||
public void onComplete(@NonNull Task<TResult> task) { | ||
if (task.isSuccessful()) { | ||
mListener.onSuccess(task.getResult()); | ||
} else { | ||
mSource.setException(task.getException()); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me so happy: because our new
signOut
method usesFragmentActivity
which extendsActivity
, people will be automatically upgraded. How's that for customer service! 😄