Skip to content

Fix broken play services checks #462

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Dec 22, 2016
Merged

Fix broken play services checks #462

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a59b111
Fix play services check
SUPERCILEX Dec 20, 2016
89add90
Fix rotation bugs
SUPERCILEX Dec 20, 2016
2361932
Merge remote-tracking branch 'samtstern/email-refactor' into api-avai…
SUPERCILEX Dec 20, 2016
a57fdf3
Fix merge mistakes
SUPERCILEX Dec 20, 2016
a289a01
Fix more conflicts
SUPERCILEX Dec 21, 2016
53a99f7
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into api-av…
SUPERCILEX Dec 22, 2016
6bf3220
Finish with an IdpResponse
SUPERCILEX Dec 22, 2016
c056b0f
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into api-av…
SUPERCILEX Dec 22, 2016
e2b195d
Address review comments
SUPERCILEX Dec 22, 2016
659bc5f
Cleanup
SUPERCILEX Dec 22, 2016
c05964e
Remove a truck load of unused code
SUPERCILEX Dec 22, 2016
ca699bb
Fix typo
SUPERCILEX Dec 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 62 additions & 6 deletions auth/src/main/java/com/firebase/ui/auth/KickoffActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,40 +1,96 @@
package com.firebase.ui.auth;

import android.app.Dialog;
import android.content.Context;
import android.content.DialogInterface;
import android.content.Intent;
import android.net.ConnectivityManager;
import android.os.Bundle;
import android.util.Log;

import com.firebase.ui.auth.ui.ActivityHelper;
import com.firebase.ui.auth.ui.AppCompatBase;
import com.firebase.ui.auth.ui.ExtraConstants;
import com.firebase.ui.auth.ui.FlowParameters;
import com.firebase.ui.auth.util.signincontainer.SignInDelegate;
import com.google.android.gms.common.GoogleApiAvailability;

public class KickoffActivity extends AppCompatBase {
private static final String TAG = "KickoffActivity";
private static final int RC_PLAY_SERVICES = 1;

private boolean mIsWaitingForPlayServices = false;

@Override
protected void onCreate(Bundle savedInstance) {
super.onCreate(savedInstance);
if (savedInstance == null) {
SignInDelegate.delegate(this, mActivityHelper.getFlowParams());
if (savedInstance == null || !savedInstance.getBoolean(ExtraConstants.HAS_EXISTING_INSTANCE)) {
if (!hasNetworkConnection()) {
Log.d(TAG, "No network connection");
finish(ErrorCodes.NO_NETWORK, IdpResponse.getErrorCodeIntent(ErrorCodes.NO_NETWORK));
return;
}

GoogleApiAvailability apiAvailability = GoogleApiAvailability.getInstance();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although you're getting rid of PlayServicesHelper I'd still like to centralize all Play services availability logic somewhere. Even if the class is just a host for a singleton of GoogleApiAvailability that's a win for future testing. Play services has given me a lot of trouble in testing (although you seem to have simplified a lot of it here).

Dialog errorDialog = apiAvailability.getErrorDialog(
this,
apiAvailability.isGooglePlayServicesAvailable(this),
RC_PLAY_SERVICES,
new DialogInterface.OnCancelListener() {
@Override
public void onCancel(DialogInterface dialog) {
finish(ResultCodes.CANCELED, new Intent());
}
});

// The error dialog will be null if isGooglePlayServicesAvailable returned SUCCESS
if (errorDialog == null) {
SignInDelegate.delegate(KickoffActivity.this,
mActivityHelper.getFlowParams());
} else {
errorDialog.show();
mIsWaitingForPlayServices = true;
}
}
}

@Override
public void onSaveInstanceState(Bundle outState) {
// It doesn't matter what we put here, we just don't want outState to be empty
outState.putBoolean(ExtraConstants.HAS_EXISTING_INSTANCE, true);
outState.putBoolean(ExtraConstants.HAS_EXISTING_INSTANCE, !mIsWaitingForPlayServices);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of HAS_EXISTING _INSTANCE to store the inverse of mIsWaitingForPlayServices is a little confusing.

Can we do two separate extras? One which is always true like before and one which stores the state of mIsWaitingForPlayServices.

super.onSaveInstanceState(outState);
}

@Override
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
super.onActivityResult(requestCode, resultCode, data);
SignInDelegate delegate = SignInDelegate.getInstance(this);
if (delegate != null) {
delegate.onActivityResult(requestCode, resultCode, data);
if (requestCode == RC_PLAY_SERVICES) {
if (resultCode == ResultCodes.OK) {
SignInDelegate.delegate(KickoffActivity.this,
mActivityHelper.getFlowParams());
} else {
finish(ResultCodes.CANCELED,
IdpResponse.getErrorCodeIntent(ErrorCodes.UNKNOWN_ERROR));
}
} else {
SignInDelegate delegate = SignInDelegate.getInstance(this);
if (delegate != null) delegate.onActivityResult(requestCode, resultCode, data);
}
}


/**
* Check if there is an active or soon-to-be-active network connection.
*/
private boolean hasNetworkConnection() {
ConnectivityManager manager =
(ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE);

return manager != null
&& manager.getActiveNetworkInfo() != null
&& manager.getActiveNetworkInfo().isConnectedOrConnecting();
}

public static Intent createIntent(Context context, FlowParameters flowParams) {
return ActivityHelper.createBaseIntent(context, KickoffActivity.class, flowParams);
}
Expand Down
104 changes: 0 additions & 104 deletions auth/src/main/java/com/firebase/ui/auth/util/PlayServicesHelper.java
View file
Open in desktop

This file was deleted.

5 changes: 1 addition & 4 deletions auth/src/main/java/com/firebase/ui/auth/util/signincontainer/SaveSmartLock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import com.firebase.ui.auth.ui.FlowParameters;
import com.firebase.ui.auth.ui.FragmentHelper;
import com.firebase.ui.auth.util.GoogleApiConstants;
import com.firebase.ui.auth.util.PlayServicesHelper;
import com.google.android.gms.auth.api.Auth;
import com.google.android.gms.auth.api.credentials.Credential;
import com.google.android.gms.common.ConnectionResult;
Expand Down Expand Up @@ -169,9 +168,7 @@ private void finish() {
public void saveCredentialsOrFinish(FirebaseUser firebaseUser,
@Nullable String password,
@Nullable IdpResponse response) {
if (!mHelper.getFlowParams().smartLockEnabled
|| !PlayServicesHelper.getInstance(getContext()).isPlayServicesAvailable()
|| getActivity().isFinishing()) {
if (!mHelper.getFlowParams().smartLockEnabled || getActivity().isFinishing()) {
finish();
return;
Copy link
Collaborator Author

@SUPERCILEX SUPERCILEX Dec 20, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samtstern Is there a reason for keeping this here? Since KickOffActivity acts as the gatekeeper, shouldn't play services always be available when we get here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be true, fine with your new change.

}
Expand Down
50 changes: 0 additions & 50 deletions auth/src/main/java/com/firebase/ui/auth/util/signincontainer/SignInDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package com.firebase.ui.auth.util.signincontainer;

import android.content.Context;
import android.content.DialogInterface;
import android.content.Intent;
import android.content.IntentSender;
import android.net.ConnectivityManager;
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
Expand All @@ -15,8 +12,6 @@
import android.util.Log;

import com.firebase.ui.auth.AuthUI;
import com.firebase.ui.auth.ErrorCodes;
import com.firebase.ui.auth.IdpResponse;
import com.firebase.ui.auth.R;
import com.firebase.ui.auth.ResultCodes;
import com.firebase.ui.auth.ui.ExtraConstants;
Expand All @@ -27,7 +22,6 @@
import com.firebase.ui.auth.ui.idp.AuthMethodPickerActivity;
import com.firebase.ui.auth.util.CredentialsApiHelper;
import com.firebase.ui.auth.util.GoogleApiConstants;
import com.firebase.ui.auth.util.PlayServicesHelper;
import com.google.android.gms.auth.api.Auth;
import com.google.android.gms.auth.api.credentials.Credential;
import com.google.android.gms.auth.api.credentials.CredentialRequest;
Expand Down Expand Up @@ -64,7 +58,6 @@ public class SignInDelegate extends SmartLockBase<CredentialRequestResult> {
private static final int RC_IDP_SIGNIN = 3;
private static final int RC_AUTH_METHOD_PICKER = 4;
private static final int RC_EMAIL_FLOW = 5;
private static final int RC_PLAY_SERVICES = 6;

private Credential mCredential;

Expand All @@ -76,34 +69,6 @@ public void onCreate(Bundle savedInstance) {
return;
}

if (!hasNetworkConnection()) {
Log.d(TAG, "No network connection");
finish(ErrorCodes.NO_NETWORK, IdpResponse.getErrorCodeIntent(ErrorCodes.NO_NETWORK));
return;
}

// Make Google Play Services available at the correct version, if possible
boolean madeAvailable =
PlayServicesHelper
.getInstance(getActivity())
.makePlayServicesAvailable(getActivity(), RC_PLAY_SERVICES,
new DialogInterface.OnCancelListener() {
@Override
public void onCancel(DialogInterface dialogInterface) {
Log.w(TAG,
"playServices:dialog.onCancel()");
finish(ResultCodes.CANCELED,
IdpResponse.getErrorCodeIntent(ErrorCodes.UNKNOWN_ERROR));
}
});

if (!madeAvailable
|| !PlayServicesHelper.getInstance(getActivity()).isPlayServicesAvailable()) {
Log.w(TAG, "playServices: could not make available.");
finish(ResultCodes.CANCELED, IdpResponse.getErrorCodeIntent(ErrorCodes.UNKNOWN_ERROR));
return;
}

if (mHelper.getFlowParams().smartLockEnabled) {
mHelper.showLoadingDialog(R.string.progress_dialog_loading);

Expand Down Expand Up @@ -185,9 +150,6 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) {
case RC_EMAIL_FLOW:
finish(resultCode, data);
break;
case RC_PLAY_SERVICES:
if (resultCode != ResultCodes.OK) finish(resultCode, data);
break;
default:
IdpSignInContainer signInContainer = IdpSignInContainer.getInstance(getActivity());
if (signInContainer != null) {
Expand Down Expand Up @@ -355,18 +317,6 @@ private void redirectToIdpSignIn(String email, String accountType) {
}
}

/**
* Check if there is an active or soon-to-be-active network connection.
*/
private boolean hasNetworkConnection() {
ConnectivityManager manager =
(ConnectivityManager) getActivity().getSystemService(Context.CONNECTIVITY_SERVICE);

return manager != null
&& manager.getActiveNetworkInfo() != null
&& manager.getActiveNetworkInfo().isConnectedOrConnecting();
}

public static void delegate(FragmentActivity activity, FlowParameters params) {
FragmentManager fm = activity.getSupportFragmentManager();
Fragment fragment = fm.findFragmentByTag(TAG);
Expand Down
11 changes: 0 additions & 11 deletions auth/src/test/java/com/firebase/ui/auth/testhelpers/TestHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
import com.firebase.ui.auth.AuthUI.IdpConfig;
import com.firebase.ui.auth.IdpResponse;
import com.firebase.ui.auth.ui.FlowParameters;
import com.google.android.gms.common.ConnectionResult;
import com.google.android.gms.common.GoogleApiAvailability;
import com.google.firebase.FirebaseApp;
import com.google.firebase.FirebaseOptions;
import com.google.firebase.auth.FirebaseUser;
Expand All @@ -33,7 +31,6 @@

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertNull;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -79,14 +76,6 @@ public static FirebaseUser makeMockFirebaseUser() {
return mockFirebaseUser;
}

public static GoogleApiAvailability makeMockGoogleApiAvailability() {
GoogleApiAvailability availability = mock(GoogleApiAvailability.class);
when(availability.isGooglePlayServicesAvailable(any(Context.class)))
.thenReturn(ConnectionResult.SUCCESS);

return availability;
}

public static void verifySmartLockSave(String providerId, String email, String password) {
ArgumentCaptor<FirebaseUser> userArgumentCaptor =
ArgumentCaptor.forClass(FirebaseUser.class);
Expand Down
2 changes: 0 additions & 2 deletions auth/src/test/java/com/firebase/ui/auth/ui/email/RegisterEmailActivityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import com.firebase.ui.auth.testhelpers.FirebaseAuthWrapperImplShadow;
import com.firebase.ui.auth.testhelpers.TestConstants;
import com.firebase.ui.auth.testhelpers.TestHelper;
import com.firebase.ui.auth.util.PlayServicesHelper;
import com.google.firebase.auth.AuthResult;
import com.google.firebase.auth.EmailAuthProvider;
import com.google.firebase.auth.FirebaseUser;
Expand Down Expand Up @@ -67,7 +66,6 @@ private RegisterEmailActivity createActivity(String email) {
@Before
public void setUp() {
TestHelper.initializeApp(RuntimeEnvironment.application);
PlayServicesHelper.sApiAvailability = TestHelper.makeMockGoogleApiAvailability();
}

@Test
Expand Down
2 changes: 0 additions & 2 deletions auth/src/test/java/com/firebase/ui/auth/ui/email/WelcomeBackPasswordPromptTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import com.firebase.ui.auth.testhelpers.TestConstants;
import com.firebase.ui.auth.testhelpers.TestHelper;
import com.firebase.ui.auth.ui.accountlink.WelcomeBackPasswordPrompt;
import com.firebase.ui.auth.util.PlayServicesHelper;
import com.google.firebase.auth.AuthResult;
import com.google.firebase.auth.EmailAuthProvider;
import com.google.firebase.auth.FirebaseUser;
Expand Down Expand Up @@ -61,7 +60,6 @@ public class WelcomeBackPasswordPromptTest {
@Before
public void setUp() {
TestHelper.initializeApp(RuntimeEnvironment.application);
PlayServicesHelper.sApiAvailability = TestHelper.makeMockGoogleApiAvailability();
}

private WelcomeBackPasswordPrompt createActivity() {
Expand Down
Loading