weblayer: ensures WebContents doesn't hide during rotation

During a rotation, when BrowserFragment is retained and the containing
activity is recreated, the following sequence of events happens:
1. destroyAttachmentState
2. createAttachmentState
3. onFragmentStart <- when this is called isAttachedToWindow() is false
4. onFragmentResume <- when this is called isAttachedToWindow() is false

BrowserImpl maintained some state to avoid doing certain
things during a configuration change, but that state was reset
in onFragmentStart(). The problem is onFragmentStart()
triggers updating the visibility of the WebContents, and at
the time onFragmentStart() is called isAttachedToWindow() returns
false, meaning the WebContents was hidden.

Hiding the WebContents is bad for sites like youtube which
stop playing video.

Then fix is to delay resetting state until onViewAttachedToWindow().

There is one other subtle change to TabImpl. Specifically
getViewController() is changed to return null if the
BrowserViewController hasn't been updated yet. This is necessary
because the code updates the active state of the tabs *and* then
the BrowserViewController. This means if when updating the active
state of a Tab the Tab calls back to BrowserViewController the
state isn't right, and may cause NPEs.

BUG=1075744
TEST=TabTest.testRotationDoesntChangeVisibility

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

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

@@ -228,9 +228,12 @@ public class InstrumentationActivityTestRule
     }
 
     public boolean executeScriptAndExtractBoolean(String script) {
+        return executeScriptAndExtractBoolean(script, true /* useSeparateIsolate */);
+    }
+
+    public boolean executeScriptAndExtractBoolean(String script, boolean useSeparateIsolate) {
         try {
-            return executeScriptSync(script, true /* useSeparateIsolate */)
-                    .getBoolean(Tab.SCRIPT_RESULT_KEY);
+            return executeScriptSync(script, useSeparateIsolate).getBoolean(Tab.SCRIPT_RESULT_KEY);
         } catch (JSONException e) {
             throw new RuntimeException(e);
         }

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

@@ -18,6 +18,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import org.chromium.base.test.util.CallbackHelper;
+import org.chromium.content_public.browser.test.util.CriteriaHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.weblayer.Browser;
 import org.chromium.weblayer.Tab;
@@ -264,4 +265,26 @@ public class TabTest {
         hidden = mActivityTestRule.executeScriptAndExtractBoolean("document.hidden;");
         Assert.assertTrue(hidden);
     }
+
+    @Test
+    @SmallTest
+    @MinWebLayerVersion(87) // Bug fix in 87.
+    // This is a regression test for https://crbug.com/1075744 .
+    public void testRotationDoesntChangeVisibility() throws Exception {
+        String url = mActivityTestRule.getTestDataURL("rotation.html");
+        mActivity = mActivityTestRule.launchShellWithUrl(url);
+        mActivity.setRetainInstance(true);
+        Assert.assertNotNull(mActivity);
+
+        // Touch to trigger fullscreen and rotation.
+        EventUtils.simulateTouchCenterOfView(mActivity.getWindow().getDecorView());
+
+        // Wait for the page to be told the orientation changed.
+        CriteriaHelper.pollInstrumentationThread(() -> {
+            return mActivityTestRule.executeScriptAndExtractBoolean("gotOrientationChange", false);
+        });
+
+        // The WebContents should not have been hidden as a result of the rotation.
+        Assert.assertFalse(mActivityTestRule.executeScriptAndExtractBoolean("gotHide", false));
+    }
 }

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

@@ -72,7 +72,14 @@ public class BrowserImpl extends IBrowser.Stub implements View.OnAttachStateChan
     private final UrlBarControllerImpl mUrlBarController;
     private boolean mFragmentStarted;
     private boolean mFragmentResumed;
-    private boolean mFragmentStoppedForConfigurationChange;
+
+    // Tracks whether the fragment is in the middle of a configuration change and was attached when
+    // the configuration change started. This is set to true in onFragmentStop() and false when
+    // isViewAttachedToWindow() is true in either onViewAttachedToWindow() or onFragmentStarted().
+    // It's important to only set this to false when isViewAttachedToWindow() is true, as otherwise
+    // the WebContents may be prematurely hidden.
+    private boolean mInConfigurationChangeAndWasAttached;
+
     // Cache the value instead of querying system every time.
     private Boolean mPasswordEchoEnabled;
     private Boolean mDarkThemeEnabled;
@@ -157,7 +164,7 @@ public class BrowserImpl extends IBrowser.Stub implements View.OnAttachStateChan
         mWindowAndroid = windowAndroid;
         mEmbedderActivityContext = embedderAppContext;
         mViewController = new BrowserViewController(
-                windowAndroid, this, mViewControllerState, mFragmentStoppedForConfigurationChange);
+                windowAndroid, this, mViewControllerState, mInConfigurationChangeAndWasAttached);
         mLocaleReceiver = new LocaleChangedBroadcastReceiver(windowAndroid.getContext().get());
         mPasswordEchoEnabled = null;
     }
@@ -495,15 +502,17 @@ public class BrowserImpl extends IBrowser.Stub implements View.OnAttachStateChan
     }
 
     public void onFragmentStart() {
-        mFragmentStoppedForConfigurationChange = false;
         mFragmentStarted = true;
+        if (mViewAttachedToWindow) {
+            mInConfigurationChangeAndWasAttached = false;
+        }
         BrowserImplJni.get().onFragmentStart(mNativeBrowser);
         updateAllTabs();
         checkPreferences();
     }
 
     public void onFragmentStop(boolean forConfigurationChange) {
-        mFragmentStoppedForConfigurationChange = forConfigurationChange;
+        mInConfigurationChangeAndWasAttached = forConfigurationChange;
         mFragmentStarted = false;
         updateAllTabs();
     }
@@ -527,8 +536,8 @@ public class BrowserImpl extends IBrowser.Stub implements View.OnAttachStateChan
         return mFragmentResumed;
     }
 
-    public boolean isFragmentStoppedForConfigurationChange() {
-        return mFragmentStoppedForConfigurationChange;
+    public boolean isInConfigurationChangeAndWasAttached() {
+        return mInConfigurationChangeAndWasAttached;
     }
 
     public boolean isViewAttachedToWindow() {
@@ -538,6 +547,9 @@ public class BrowserImpl extends IBrowser.Stub implements View.OnAttachStateChan
     @Override
     public void onViewAttachedToWindow(View v) {
         mViewAttachedToWindow = true;
+        if (mFragmentStarted) {
+            mInConfigurationChangeAndWasAttached = false;
+        }
         updateAllTabsViewAttachedState();
     }
 

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

@@ -453,7 +453,7 @@ public final class TabImpl extends ITab.Stub implements LoginPrompt.Observer {
     public boolean isVisible() {
         return mBrowser.getActiveTab() == this
                 && ((mBrowser.isStarted() && mBrowser.isViewAttachedToWindow())
-                        || mBrowser.isFragmentStoppedForConfigurationChange());
+                        || mBrowser.isInConfigurationChangeAndWasAttached());
     }
 
     private void updateWebContentsVisibility() {
@@ -1102,7 +1102,12 @@ public final class TabImpl extends ITab.Stub implements LoginPrompt.Observer {
      */
     @Nullable
     private BrowserViewController getViewController() {
-        return (mBrowser.getActiveTab() == this) ? mBrowser.getPossiblyNullViewController() : null;
+        if (mBrowser.getActiveTab() != this) 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.
+        BrowserViewController viewController = mBrowser.getPossiblyNullViewController();
+        return viewController != null && viewController.getTab() == this ? viewController : null;
     }
 
     @VisibleForTesting

weblayer/test/data/rotation.html

+<html>
+  <body style="height:5000px">
+    <p>A (mostly) empty page.</p>
+  </body>
+  <script>
+    var gotHide = false;
+    var gotOrientationChange = false;
+    async function doRequestFullscreen() {
+        return document.documentElement.requestFullscreen();
+    }
+    async function toggleFullscreen() {
+      if (!document.fullscreenElement) {
+          await doRequestFullscreen();
+          await screen.orientation.lock("landscape");
+      } else {
+        document.exitFullscreen();
+      }
+    }
+    document.addEventListener("visibilitychange", function() {
+      if (document.visibilityState !== 'visible') {
+        gotHide = true;
+      }
+    });
+    window.addEventListener("orientationchange", function() {
+      gotOrientationChange = true;
+    });
+    document.addEventListener('touchend', function(e) { toggleFullscreen(); }, false);
+  </script>
+</html>