Skip to content

Commit

Permalink
Fixing GMSCore version incompatibility
Browse files Browse the repository at this point in the history
Calling GMSCore's isGooglePlayServicesAvailable(context) uses our SDK to
check what corresponding version of the GMSCore app we need on the
device. This assumption (that we need the GMSCore app to be ahead of the
SDK) hasn't been true for years for most Google apps, and that
requirement is breaking us when we update to bleeding-edge GMSCore SDKs.

This is currently blocking new users from signing into Canary.

Testing: Was able to reproduce error message with local build:
https://photos.app.goo.gl/e9PJ9cmhRaeH7THt6
With this change, error message did not appear, and signin button was
not greyed out. This is consistent with another local build made before
the breaking roll occurred.

TBR: nyquist, mattreynolds, bsazonov for urgent refactor
Bug: 1144669, 1145211
Change-Id: Iea18fc7340222bbf6027e210318cca6c68105a4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518219
Commit-Queue: Sam Maier <[email protected]>
Reviewed-by: Boris Sazonov <[email protected]>
Reviewed-by: Reilly Grant <[email protected]>
Reviewed-by: Theresa  <[email protected]>
Cr-Commit-Position: refs/heads/master@{#824035}
  • Loading branch information
Sam Maier authored and Commit Bot committed Nov 4, 2020
1 parent 21e4b95 commit 0eeafca
Show file tree
Hide file tree
Showing 24 changed files with 154 additions and 71 deletions.
2 changes: 2 additions & 0 deletions chrome/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ android_library("chrome_java") {
"//third_party/android_deps:androidx_preference_preference_java",
"//third_party/android_deps:androidx_recyclerview_recyclerview_java",
"//third_party/android_deps:androidx_viewpager_viewpager_java",
"//third_party/android_deps:chromium_play_services_availability_java",
"//third_party/android_deps:com_google_code_findbugs_jsr305_java",
"//third_party/android_deps:com_google_dagger_dagger_java",
"//third_party/android_deps:com_google_guava_listenablefuture_java",
Expand Down Expand Up @@ -915,6 +916,7 @@ junit_binary("chrome_junit_tests") {
"//third_party/android_deps:androidx_swiperefreshlayout_swiperefreshlayout_java",
"//third_party/android_deps:androidx_test_core_java",
"//third_party/android_deps:androidx_test_runner_java",
"//third_party/android_deps:chromium_play_services_availability_shadows_java",
"//third_party/android_deps:com_google_dagger_dagger_java",
"//third_party/android_deps:com_googlecode_java_diff_utils_diffutils_java",
"//third_party/android_deps:protobuf_lite_runtime_java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@

import android.os.Build;

import com.google.android.gms.common.ConnectionResult;
import com.google.android.gms.common.GoogleApiAvailability;

import org.chromium.base.ContextUtils;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace;
Expand All @@ -17,6 +14,7 @@
import org.chromium.components.background_task_scheduler.BackgroundTaskSchedulerFactory;
import org.chromium.components.background_task_scheduler.TaskIds;
import org.chromium.components.background_task_scheduler.TaskInfo;
import org.chromium.gms.ChromiumPlayServicesAvailability;

/** Java-side implementation of the component update scheduler using the BackgroundTaskScheduler. */
@JNINamespace("component_updater")
Expand All @@ -37,9 +35,8 @@ public class UpdateScheduler {
@CalledByNative
/* package */ static boolean isAvailable() {
return Build.VERSION.SDK_INT >= Build.VERSION_CODES.M
|| GoogleApiAvailability.getInstance().isGooglePlayServicesAvailable(
ContextUtils.getApplicationContext())
== ConnectionResult.SUCCESS;
|| ChromiumPlayServicesAvailability.chromiumIsGooglePlayServicesAvailable(
ContextUtils.getApplicationContext());
}

/* package */ void onStartTaskBeforeNativeLoaded(TaskFinishedCallback callback) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.components.embedder_support.util.Origin;
import org.chromium.content_public.browser.UiThreadTaskTraits;
import org.chromium.gms.ChromiumPlayServicesAvailability;

/**
* Utility class for external authentication tools.
Expand Down Expand Up @@ -251,7 +252,7 @@ public boolean canUseFirstPartyGooglePlayServices() {
protected int checkGooglePlayServicesAvailable(final Context context) {
// TODO(crbug.com/577190): Temporarily allowing disk access until more permanent fix is in.
try (StrictModeContext ignored = StrictModeContext.allowDiskWrites()) {
return GoogleApiAvailability.getInstance().isGooglePlayServicesAvailable(context);
return ChromiumPlayServicesAvailability.isGooglePlayServicesAvailable(context);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,32 @@
import static org.junit.Assert.assertTrue;

import com.google.android.gms.common.ConnectionResult;
import com.google.android.gms.common.GoogleApiAvailability;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.annotation.Config;
import org.robolectric.shadows.gms.Shadows;
import org.robolectric.shadows.gms.common.ShadowGoogleApiAvailability;

import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.base.test.util.Feature;
import org.chromium.gms.shadows.ShadowChromiumPlayServicesAvailability;

/** Unit tests for GooglePlayServicesChecker. */
@RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE, shadows = {ShadowGoogleApiAvailability.class})
@Config(manifest = Config.NONE, shadows = {ShadowChromiumPlayServicesAvailability.class})
public class BackgroundSyncGooglePlayServicesCheckerTest {

@Test
@Feature("BackgroundSync")
public void testDisableLogicWhenGooglePlayServicesReturnsSuccess() {
Shadows.shadowOf(GoogleApiAvailability.getInstance())
.setIsGooglePlayServicesAvailable(ConnectionResult.SUCCESS);
ShadowChromiumPlayServicesAvailability.setIsGooglePlayServicesAvailable(
ConnectionResult.SUCCESS);
assertFalse(GooglePlayServicesChecker.shouldDisableBackgroundSync());
}

@Test
@Feature("BackgroundSync")
public void testDisableLogicWhenGooglePlayServicesReturnsError() {
Shadows.shadowOf(GoogleApiAvailability.getInstance())
.setIsGooglePlayServicesAvailable(ConnectionResult.SERVICE_VERSION_UPDATE_REQUIRED);
ShadowChromiumPlayServicesAvailability.setIsGooglePlayServicesAvailable(
ConnectionResult.SERVICE_VERSION_UPDATE_REQUIRED);
assertTrue(GooglePlayServicesChecker.shouldDisableBackgroundSync());
}
}
1 change: 1 addition & 0 deletions chrome/test/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ android_library("chrome_java_test_support") {
"//third_party/android_deps:androidx_fragment_fragment_java",
"//third_party/android_deps:androidx_lifecycle_lifecycle_common_java",
"//third_party/android_deps:androidx_recyclerview_recyclerview_java",
"//third_party/android_deps:chromium_play_services_availability_java",
"//third_party/android_deps:com_google_code_findbugs_jsr305_java",
"//third_party/android_deps:espresso_java",
"//third_party/android_deps:material_design_java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
import android.support.test.InstrumentationRegistry;
import android.text.TextUtils;

import com.google.android.gms.common.ConnectionResult;
import com.google.android.gms.common.GoogleApiAvailability;

import org.junit.rules.TestRule;
import org.junit.runners.model.InitializationError;

Expand All @@ -26,6 +23,7 @@
import org.chromium.components.policy.test.annotations.Policies;
import org.chromium.content_public.browser.test.ContentJUnit4ClassRunner;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.gms.ChromiumPlayServicesAvailability;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
Expand Down Expand Up @@ -154,9 +152,8 @@ private boolean isVrDonEnabled() {
protected boolean restrictionApplies(String restriction) {
if (TextUtils.equals(
restriction, ChromeRestriction.RESTRICTION_TYPE_GOOGLE_PLAY_SERVICES)
&& (ConnectionResult.SUCCESS
!= GoogleApiAvailability.getInstance().isGooglePlayServicesAvailable(
getTargetContext()))) {
&& (!ChromiumPlayServicesAvailability.chromiumIsGooglePlayServicesAvailable(
getTargetContext()))) {
return true;
}
if (TextUtils.equals(restriction, ChromeRestriction.RESTRICTION_TYPE_OFFICIAL_BUILD)
Expand Down
1 change: 1 addition & 0 deletions components/background_task_scheduler/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ if (is_android) {
"//base:base_java_test_support",
"//base:base_junit_test_support",
"//components/background_task_scheduler:public_java",
"//third_party/android_deps:chromium_play_services_availability_shadows_java",
"//third_party/android_deps:robolectric_all_java",
"//third_party/junit",
"//third_party/mockito:mockito_java",
Expand Down
1 change: 1 addition & 0 deletions components/background_task_scheduler/internal/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ if (is_android) {
"//components/background_task_scheduler:public_java",
"//content/public/android:content_java",
"//third_party/android_deps:androidx_annotation_annotation_java",
"//third_party/android_deps:chromium_play_services_availability_java",
"//third_party/android_deps:protobuf_lite_runtime_java",
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;

import com.google.android.gms.common.ConnectionResult;
import com.google.android.gms.common.GoogleApiAvailability;
import com.google.android.gms.gcm.GcmNetworkManager;
import com.google.android.gms.gcm.OneoffTask;
import com.google.android.gms.gcm.PeriodicTask;
Expand All @@ -22,6 +20,7 @@
import org.chromium.base.ThreadUtils;
import org.chromium.components.background_task_scheduler.TaskInfo;
import org.chromium.components.background_task_scheduler.TaskParameters;
import org.chromium.gms.ChromiumPlayServicesAvailability;

import java.util.concurrent.TimeUnit;

Expand Down Expand Up @@ -246,8 +245,7 @@ public void cancel(Context context, int taskId) {
}

private GcmNetworkManager getGcmNetworkManager(Context context) {
if (GoogleApiAvailability.getInstance().isGooglePlayServicesAvailable(context)
== ConnectionResult.SUCCESS) {
if (ChromiumPlayServicesAvailability.chromiumIsGooglePlayServicesAvailable(context)) {
return GcmNetworkManager.getInstance(context);
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@

import android.os.Bundle;

import com.google.android.gms.common.ConnectionResult;
import com.google.android.gms.common.GoogleApiAvailability;
import com.google.android.gms.gcm.GcmNetworkManager;
import com.google.android.gms.gcm.OneoffTask;
import com.google.android.gms.gcm.PeriodicTask;
Expand All @@ -25,8 +23,6 @@
import org.junit.runner.RunWith;
import org.robolectric.annotation.Config;
import org.robolectric.shadow.api.Shadow;
import org.robolectric.shadows.gms.Shadows;
import org.robolectric.shadows.gms.common.ShadowGoogleApiAvailability;

import org.chromium.base.ContextUtils;
import org.chromium.base.test.BaseRobolectricTestRunner;
Expand All @@ -35,13 +31,14 @@
import org.chromium.components.background_task_scheduler.TaskIds;
import org.chromium.components.background_task_scheduler.TaskInfo;
import org.chromium.components.background_task_scheduler.TaskParameters;
import org.chromium.gms.shadows.ShadowChromiumPlayServicesAvailability;

import java.util.concurrent.TimeUnit;

/** Unit tests for {@link BackgroundTaskSchedulerGcmNetworkManager}. */
@RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE,
shadows = {ShadowGcmNetworkManager.class, ShadowGoogleApiAvailability.class})
shadows = {ShadowGcmNetworkManager.class, ShadowChromiumPlayServicesAvailability.class})
public class BackgroundTaskSchedulerGcmNetworkManagerTest {
ShadowGcmNetworkManager mGcmNetworkManager;

Expand All @@ -59,8 +56,7 @@ public class BackgroundTaskSchedulerGcmNetworkManagerTest {

@Before
public void setUp() {
Shadows.shadowOf(GoogleApiAvailability.getInstance())
.setIsGooglePlayServicesAvailable(ConnectionResult.SUCCESS);
ShadowChromiumPlayServicesAvailability.setChromiumIsGooglePlayServicesAvailable(true);
mGcmNetworkManager = (ShadowGcmNetworkManager) Shadow.extract(
GcmNetworkManager.getInstance(ContextUtils.getApplicationContext()));
BackgroundTaskSchedulerGcmNetworkManager.setClockForTesting(mClock);
Expand Down Expand Up @@ -301,8 +297,7 @@ public void testSchedule() {
@Test
@Feature("BackgroundTaskScheduler")
public void testScheduleNoGooglePlayServices() {
Shadows.shadowOf(GoogleApiAvailability.getInstance())
.setIsGooglePlayServicesAvailable(ConnectionResult.SERVICE_MISSING);
ShadowChromiumPlayServicesAvailability.setChromiumIsGooglePlayServicesAvailable(false);

TaskInfo.TimingInfo timingInfo =
TaskInfo.OneOffInfo.create().setWindowEndTimeMs(TIME_24_H_TO_MS).build();
Expand Down Expand Up @@ -332,8 +327,7 @@ public void testCancel() {
@Feature("BackgroundTaskScheduler")
public void testCancelNoGooglePlayServices() {
// This simulates situation where Google Play Services is uninstalled.
Shadows.shadowOf(GoogleApiAvailability.getInstance())
.setIsGooglePlayServicesAvailable(ConnectionResult.SERVICE_MISSING);
ShadowChromiumPlayServicesAvailability.setChromiumIsGooglePlayServicesAvailable(false);

TaskInfo.TimingInfo timingInfo =
TaskInfo.OneOffInfo.create().setWindowEndTimeMs(TIME_24_H_TO_MS).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@

import android.os.Build;

import com.google.android.gms.common.ConnectionResult;
import com.google.android.gms.common.GoogleApiAvailability;
import com.google.android.gms.gcm.GcmNetworkManager;

import org.junit.Before;
Expand All @@ -27,8 +25,6 @@
import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config;
import org.robolectric.shadow.api.Shadow;
import org.robolectric.shadows.gms.Shadows;
import org.robolectric.shadows.gms.common.ShadowGoogleApiAvailability;
import org.robolectric.util.ReflectionHelpers;

import org.chromium.base.ContextUtils;
Expand All @@ -37,13 +33,14 @@
import org.chromium.components.background_task_scheduler.BackgroundTaskScheduler;
import org.chromium.components.background_task_scheduler.TaskIds;
import org.chromium.components.background_task_scheduler.TaskInfo;
import org.chromium.gms.shadows.ShadowChromiumPlayServicesAvailability;

import java.util.concurrent.TimeUnit;

/** Unit tests for {@link BackgroundTaskScheduler}. */
@RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE,
shadows = {ShadowGcmNetworkManager.class, ShadowGoogleApiAvailability.class})
shadows = {ShadowGcmNetworkManager.class, ShadowChromiumPlayServicesAvailability.class})
public class BackgroundTaskSchedulerImplTest {
@Mock
private BackgroundTaskSchedulerDelegate mDelegate;
Expand All @@ -66,8 +63,7 @@ public void setUp() {
TestBackgroundTask.reset();

// Initialize Google Play Services and GCM Network Manager for upgrade testing.
Shadows.shadowOf(GoogleApiAvailability.getInstance())
.setIsGooglePlayServicesAvailable(ConnectionResult.SUCCESS);
ShadowChromiumPlayServicesAvailability.setChromiumIsGooglePlayServicesAvailable(true);
mGcmNetworkManager = (ShadowGcmNetworkManager) Shadow.extract(
GcmNetworkManager.getInstance(ContextUtils.getApplicationContext()));

Expand Down
1 change: 1 addition & 0 deletions components/signin/core/browser/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ android_library("java") {
"//net/android:net_java",
"//third_party/android_deps:android_support_v4_java",
"//third_party/android_deps:androidx_annotation_annotation_java",
"//third_party/android_deps:chromium_play_services_availability_java",
"//ui/android:ui_java",
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.chromium.base.ThreadUtils;
import org.chromium.base.library_loader.LibraryLoader;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.gms.ChromiumPlayServicesAvailability;

import java.io.IOException;

Expand Down Expand Up @@ -90,7 +91,7 @@ public void onReceive(final Context context, final Intent intent) {
protected void checkCanUseGooglePlayServices() throws AccountManagerDelegateException {
Context context = ContextUtils.getApplicationContext();
final int resultCode =
GoogleApiAvailability.getInstance().isGooglePlayServicesAvailable(context);
ChromiumPlayServicesAvailability.isGooglePlayServicesAvailable(context);
if (resultCode == ConnectionResult.SUCCESS) {
return;
}
Expand Down Expand Up @@ -263,9 +264,8 @@ public String getAccountGaiaId(String accountEmail) {
public boolean isGooglePlayServicesAvailable() {
// TODO(http://crbug.com/577190): Remove StrictMode override.
try (StrictModeContext ignored = StrictModeContext.allowDiskWrites()) {
int resultCode = GoogleApiAvailability.getInstance().isGooglePlayServicesAvailable(
return ChromiumPlayServicesAvailability.chromiumIsGooglePlayServicesAvailable(
ContextUtils.getApplicationContext());
return resultCode == ConnectionResult.SUCCESS;
}
}

Expand Down
1 change: 1 addition & 0 deletions services/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ if (is_android) {
"//skia/public/mojom:mojom_java",
"//third_party/android_deps:androidx_test_monitor_java",
"//third_party/android_deps:androidx_test_runner_java",
"//third_party/android_deps:chromium_play_services_availability_java",
"//third_party/android_support_test_runner:runner_java",
"//third_party/junit",
"//ui/gfx/geometry/mojom:mojom_java",
Expand Down
1 change: 1 addition & 0 deletions services/device/geolocation/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ if (is_android) {
"//components/location/android:location_java",
"//services/device/public/java:geolocation_java",
"//third_party/android_deps:androidx_annotation_annotation_java",
"//third_party/android_deps:chromium_play_services_availability_java",
]
annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import android.os.Bundle;

import com.google.android.gms.common.ConnectionResult;
import com.google.android.gms.common.GoogleApiAvailability;
import com.google.android.gms.common.api.GoogleApiClient;
import com.google.android.gms.common.api.GoogleApiClient.ConnectionCallbacks;
import com.google.android.gms.common.api.GoogleApiClient.OnConnectionFailedListener;
Expand All @@ -21,6 +20,7 @@
import org.chromium.base.Log;
import org.chromium.base.ThreadUtils;
import org.chromium.components.location.LocationUtils;
import org.chromium.gms.ChromiumPlayServicesAvailability;

/**
* This is a LocationProvider using Google Play Services.
Expand All @@ -42,8 +42,7 @@ public class LocationProviderGmsCore implements ConnectionCallbacks, OnConnectio
private LocationRequest mLocationRequest;

public static boolean isGooglePlayServicesAvailable(Context context) {
return GoogleApiAvailability.getInstance().isGooglePlayServicesAvailable(context)
== ConnectionResult.SUCCESS;
return ChromiumPlayServicesAvailability.chromiumIsGooglePlayServicesAvailable(context);
}

LocationProviderGmsCore(Context context) {
Expand Down
1 change: 1 addition & 0 deletions services/shape_detection/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ if (is_android) {
"//mojo/public/java/system:system_impl_java",
"//services/shape_detection/public/mojom:mojom_java",
"//skia/public/mojom:mojom_java",
"//third_party/android_deps:chromium_play_services_availability_java",
"//ui/gfx/geometry/mojom:mojom_java",
]
}
Expand Down
Loading

0 comments on commit 0eeafca

Please sign in to comment.