weblayer: fixs possible NPE if a11y enabled during Tab creation

If a11y is enabled, then Tab creation may trigger calling some
code paths that assume mBrowser was non-null.

BUG=1136596
TEST=TabPrivateTest.testCreateTabWithAccessibilityEnabledCrashTest

Change-Id: I500fd70cb56b7d16c22284acb677c06ec57b536d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461714Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815357}

weblayer/browser/android/javatests/BUILD.gn

@@ -79,6 +79,7 @@ android_library("weblayer_private_java_tests") {
     "src/org/chromium/weblayer/test/PopupTest.java",
     "src/org/chromium/weblayer/test/ResourceLoadingTest.java",
     "src/org/chromium/weblayer/test/SiteSettingsTest.java",
+    "src/org/chromium/weblayer/test/TabPrivateTest.java",
     "src/org/chromium/weblayer/test/TranslateTest.java",
     "src/org/chromium/weblayer/test/UrlBarControllerTest.java",
   ]

weblayer/browser/android/javatests/src/org/chromium/weblayer/test/InstrumentationActivityTestRule.java

@@ -7,6 +7,7 @@ package org.chromium.weblayer.test;
 import android.app.Activity;
 import android.app.Instrumentation.ActivityMonitor;
 import android.content.ComponentName;
+import android.content.Context;
 import android.content.Intent;
 import android.content.pm.ActivityInfo;
 import android.net.Uri;
@@ -73,10 +74,12 @@ public class InstrumentationActivityTestRule
     }
 
     public WebLayer getWebLayer() {
-        return TestThreadUtils.runOnUiThreadBlockingNoException(() -> {
-            return WebLayer.loadSync(
-                    InstrumentationRegistry.getTargetContext().getApplicationContext());
-        });
+        return TestThreadUtils.runOnUiThreadBlockingNoException(
+                () -> { return WebLayer.loadSync(getContextForWebLayer()); });
+    }
+
+    public Context getContextForWebLayer() {
+        return InstrumentationRegistry.getTargetContext().getApplicationContext();
     }
 
     /**
@@ -103,7 +106,7 @@ public class InstrumentationActivityTestRule
         Assert.assertNotNull(activity);
         try {
             TestThreadUtils.runOnUiThreadBlocking(
-                    () -> activity.loadWebLayerSync(activity.getApplicationContext()));
+                    () -> activity.loadWebLayerSync(getContextForWebLayer()));
         } catch (ExecutionException e) {
             throw new RuntimeException(e);
         }

weblayer/browser/android/javatests/src/org/chromium/weblayer/test/TabPrivateTest.java

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.weblayer.test;
+
+import android.os.RemoteException;
+
+import androidx.test.filters.SmallTest;
+
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.content_public.browser.test.util.TestThreadUtils;
+import org.chromium.weblayer.TestWebLayer;
+import org.chromium.weblayer.shell.InstrumentationActivity;
+
+/**
+ * Tab tests that need to use WebLayerPrivate.
+ */
+@RunWith(WebLayerJUnit4ClassRunner.class)
+public class TabPrivateTest {
+    @Rule
+    public InstrumentationActivityTestRule mActivityTestRule =
+            new InstrumentationActivityTestRule();
+
+    private TestWebLayer getTestWebLayer() {
+        return TestWebLayer.getTestWebLayer(mActivityTestRule.getContextForWebLayer());
+    }
+
+    @Test
+    @SmallTest
+    public void testCreateTabWithAccessibilityEnabledCrashTest() throws Exception {
+        InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl("about:blank");
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            try {
+                getTestWebLayer().setAccessibilityEnabled(true);
+            } catch (RemoteException e) {
+                Assert.fail("Unable to enable accessibility");
+            }
+            activity.getBrowser().createTab();
+        });
+    }
+}

weblayer/browser/java/org/chromium/weblayer_private/TabImpl.java

@@ -101,7 +101,9 @@ public final class TabImpl extends ITab.Stub implements LoginPrompt.Observer {
     private FullscreenCallbackProxy mFullscreenCallbackProxy;
     private TabViewAndroidDelegate mViewAndroidDelegate;
     private GoogleAccountsCallbackProxy mGoogleAccountsCallbackProxy;
-    // BrowserImpl this TabImpl is in. This is only null during creation.
+    // BrowserImpl this TabImpl is in. This is null before attached to a Browser. While this is null
+    // before attached, there are code paths that may trigger calling methods before set.
+    @Nullable
     private BrowserImpl mBrowser;
     private LoginPrompt mLoginPrompt;
     /**
@@ -451,11 +453,15 @@ public final class TabImpl extends ITab.Stub implements LoginPrompt.Observer {
      * Returns whether this Tab is visible.
      */
     public boolean isVisible() {
-        return mBrowser.getActiveTab() == this
+        return isActiveTab()
                 && ((mBrowser.isStarted() && mBrowser.isViewAttachedToWindow())
                         || mBrowser.isInConfigurationChangeAndWasAttached());
     }
 
+    private boolean isActiveTab() {
+        return mBrowser != null && mBrowser.getActiveTab() == this;
+    }
+
     private void updateWebContentsVisibility() {
         boolean visibleNow = isVisible();
         boolean webContentsVisible = mWebContents.getVisibility() == Visibility.VISIBLE;
@@ -1037,6 +1043,8 @@ public final class TabImpl extends ITab.Stub implements LoginPrompt.Observer {
     }
 
     private void onBrowserControlsConstraintUpdated(int constraint) {
+        // WARNING: this may be called before attached. This means |mBrowser| may be null.
+
         // If something has overridden the FIP's SHOWN constraint, cancel FIP. This causes FIP to
         // dismiss when entering fullscreen.
         if (constraint != BrowserControlsState.SHOWN) {
@@ -1055,7 +1063,7 @@ public final class TabImpl extends ITab.Stub implements LoginPrompt.Observer {
         // happen). For js dialogs, the renderer's update will come when the dialog is hidden, and
         // since that animates from 0 height, it causes a flicker since the override is already set
         // to fully show. Thus, disable animation.
-        if (constraint == BrowserControlsState.SHOWN && mBrowser.getActiveTab() == this
+        if (constraint == BrowserControlsState.SHOWN && isActiveTab()
                 && !TabImplJni.get().isRendererControllingBrowserControlsOffsets(mNativeTab)) {
             mViewAndroidDelegate.setIgnoreRendererUpdates(true);
             if (viewController != null) viewController.showControls();
@@ -1102,7 +1110,7 @@ public final class TabImpl extends ITab.Stub implements LoginPrompt.Observer {
      */
     @Nullable
     private BrowserViewController getViewController() {
-        if (mBrowser.getActiveTab() != this) return null;
+        if (!isActiveTab()) return null;
         // During rotation it's possible for this to be called before BrowserViewController has been
         // updated. Verify BrowserViewController reflects this is the active tab before returning
         // it.