Ensure initial top controls gets propagated when in VR for new RWHVA.

In some cases the Metadata for a RWHVA is set before VR code calls
SetIsInVr on the RWHVA, so we miss the initial top controls update.

(The only reason we care about VR-ness when updating the frame metadata
is that sending the initial top controls update breaks interstitials)

Bug: 837807
Change-Id: Ic108fa324725e7c126c9f721c27e4c5f99521af6
Reviewed-on: https://chromium-review.googlesource.com/1042500Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555918}

chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java

@@ -26,7 +26,6 @@ import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tabmodel.TabModel.TabSelectionType;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabModelObserver;
-import org.chromium.chrome.browser.vr_shell.VrShellDelegate;
 import org.chromium.chrome.browser.widget.ControlContainer;
 import org.chromium.content.browser.ContentVideoView;
 import org.chromium.content_public.common.BrowserControlsState;
@@ -355,9 +354,7 @@ public class ChromeFullscreenManager
         return mTopControlContainerHeight;
     }
 
-    /**
-     * @return The height of the bottom controls in pixels.
-     */
+    @Override
     public int getBottomControlsHeight() {
         return mBottomControlContainerHeight;
     }
@@ -620,18 +617,6 @@ public class ChromeFullscreenManager
     @Override
     public void setPositionsForTab(float topControlsOffset, float bottomControlsOffset,
             float topContentOffset) {
-        if (VrShellDelegate.isInVr()) {
-            VrShellDelegate.rawTopContentOffsetChanged(topContentOffset);
-            // The dip scale of java UI and WebContents are different while in VR, leading to a
-            // mismatch in size in pixels when converting from dips. Since we hide the controls in
-            // VR anyways, just set the offsets to what they're supposed to be with the controls
-            // hidden.
-            // TODO(mthiesse): Should we instead just set the top controls height to be 0 while in
-            // VR?
-            topControlsOffset = -getTopControlsHeight();
-            bottomControlsOffset = getBottomControlsHeight();
-            topContentOffset = 0;
-        }
         float rendererTopControlOffset =
                 Math.round(Math.max(topControlsOffset, -getTopControlsHeight()));
         float rendererBottomControlOffset =

chrome/android/java/src/org/chromium/chrome/browser/fullscreen/FullscreenManager.java

@@ -50,6 +50,11 @@ public abstract class FullscreenManager {
      */
     public abstract int getTopControlsHeight();
 
+    /**
+     * @return The height of the bottom controls in pixels.
+     */
+    public abstract int getBottomControlsHeight();
+
     /**
      * @return The ratio that the browser controls are off screen; this will be a number [0,1]
      *         where 1 is completely hidden and 0 is completely shown.

chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java

@@ -2093,7 +2093,7 @@ public class Tab
             mInfoBarContainer = null;
         }
 
-        mControlsOffsetHelper.clearPreviousPositions();
+        mControlsOffsetHelper.destroy();
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/tab/TabBrowserControlsOffsetHelper.java

@@ -12,11 +12,13 @@ import org.chromium.base.ObserverList;
 import org.chromium.chrome.browser.fullscreen.FullscreenManager;
 import org.chromium.chrome.browser.tabmodel.TabModelImpl;
 import org.chromium.chrome.browser.util.FeatureUtilities;
+import org.chromium.chrome.browser.vr_shell.VrShellDelegate;
+import org.chromium.chrome.browser.vr_shell.VrShellDelegate.VrModeObserver;
 
 /**
  * Handles browser controls offset for a Tab.
  */
-public class TabBrowserControlsOffsetHelper {
+public class TabBrowserControlsOffsetHelper implements VrModeObserver {
     /**
      * Maximum duration for the control container slide-in animation. Note that this value matches
      * the one in browser_controls_offset_manager.cc.
@@ -50,11 +52,17 @@ public class TabBrowserControlsOffsetHelper {
      */
     private ValueAnimator mControlsAnimator;
 
+    /**
+     * Whether the browser is currently in VR mode.
+     */
+    private boolean mIsInVr;
+
     /**
      * @param tab The {@link Tab} that this class is associated with.
      */
     TabBrowserControlsOffsetHelper(Tab tab) {
         mTab = tab;
+        VrShellDelegate.registerVrModeObserver(this);
     }
 
     /**
@@ -171,7 +179,18 @@ public class TabBrowserControlsOffsetHelper {
         final FullscreenManager manager = mTab.getFullscreenManager();
         if (manager == null) return;
 
-        if (toNonFullscreen) {
+        if (mIsInVr) {
+            VrShellDelegate.rawTopContentOffsetChanged(topContentOffset);
+            // The dip scale of java UI and WebContents are different while in VR, leading to a
+            // mismatch in size in pixels when converting from dips. Since we hide the controls in
+            // VR anyways, just set the offsets to what they're supposed to be with the controls
+            // hidden.
+            // TODO(mthiesse): Should we instead just set the top controls height to be 0 while in
+            // VR?
+            topControlsOffset = -manager.getTopControlsHeight();
+            bottomControlsOffset = manager.getBottomControlsHeight();
+            topContentOffset = 0;
+        } else if (toNonFullscreen) {
             manager.setPositionsForTabToNonFullscreen();
         } else {
             manager.setPositionsForTab(topControlsOffset, bottomControlsOffset, topContentOffset);
@@ -228,4 +247,24 @@ public class TabBrowserControlsOffsetHelper {
         });
         mControlsAnimator.start();
     }
+
+    @Override
+    public void onEnterVr() {
+        mIsInVr = true;
+        resetPositions();
+    }
+
+    @Override
+    public void onExitVr() {
+        mIsInVr = false;
+        resetPositions();
+    }
+
+    /**
+     * Cleans up internal state, unregistering any observers.
+     */
+    public void destroy() {
+        clearPreviousPositions();
+        VrShellDelegate.unregisterVrModeObserver(this);
+    }
 }

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -183,7 +183,9 @@ RenderWidgetHostViewAndroid::RenderWidgetHostViewAndroid(
       frame_evictor_(new viz::FrameEvictor(this)),
       observing_root_window_(false),
       prev_top_shown_pix_(0.f),
+      prev_top_controls_translate_(0.f),
       prev_bottom_shown_pix_(0.f),
+      prev_bottom_controls_translate_(0.f),
       page_scale_(1.f),
       min_page_scale_(1.f),
       max_page_scale_(1.f),
@@ -1259,25 +1261,32 @@ void RenderWidgetHostViewAndroid::OnFrameMetadataUpdated(
 
   bool top_changed = !FloatEquals(top_shown_pix, prev_top_shown_pix_);
   // TODO(carlosil, https://crbug.com/825765): Remove the IsInVR() check here,
-  // which is a temporary hack. When interstitial pages load they set the
-  // top controls offset to 0, and if we don't ignore that hit targeting on
-  // interstitial pages breaks. The fix is still crucial for VR as VR needs the
-  // top controls to be initially hidden correctly.
+  // which is a temporary hack. Interstitial pages are not committed navigations
+  // and their metadata updates never leave the content layer, so Chrome and the
+  // content layer end up mismatched and hit testing is offset. They rely on the
+  // previous (erroneous) behavior of ignoring the initial control offset update
+  // if offset is 0. The fix is still crucial for VR as VR needs the top
+  // controls to be initially hidden correctly (so we don't want the offset of 0
+  // to get ignored. Tracking bug for the interstitial work to fix this by
+  // converting interstitials to committed navigations is
+  // https://crbug.com/755632.
+  float top_translate = top_shown_pix - top_controls_pix;
   if (top_changed || (!controls_initialized_ && IsInVR())) {
-    float translate = top_shown_pix - top_controls_pix;
-    view_.OnTopControlsChanged(translate, top_shown_pix);
-    prev_top_shown_pix_ = top_shown_pix;
+    view_.OnTopControlsChanged(top_translate, top_shown_pix);
   }
+  prev_top_shown_pix_ = top_shown_pix;
+  prev_top_controls_translate_ = top_translate;
 
   float bottom_controls_pix = frame_metadata.bottom_controls_height * to_pix;
   float bottom_shown_pix =
       bottom_controls_pix * frame_metadata.bottom_controls_shown_ratio;
   bool bottom_changed = !FloatEquals(bottom_shown_pix, prev_bottom_shown_pix_);
+  float bottom_translate = bottom_controls_pix - bottom_shown_pix;
   if (bottom_changed || (!controls_initialized_ && IsInVR())) {
-    float translate = bottom_controls_pix - bottom_shown_pix;
-    view_.OnBottomControlsChanged(translate, bottom_shown_pix);
-    prev_bottom_shown_pix_ = bottom_shown_pix;
+    view_.OnBottomControlsChanged(bottom_translate, bottom_shown_pix);
   }
+  prev_bottom_shown_pix_ = bottom_shown_pix;
+  prev_bottom_controls_translate_ = bottom_translate;
   controls_initialized_ = true;
 
   page_scale_ = frame_metadata.page_scale_factor;
@@ -1823,6 +1832,17 @@ void RenderWidgetHostViewAndroid::SetIsInVR(bool is_in_vr) {
     touch_selection_controller_ = CreateSelectionController(
         touch_selection_controller_client_manager_.get(), view_.parent());
   }
+
+  if (is_in_vr_ && controls_initialized_) {
+    // TODO(carlosil, https://crbug.com/825765): See the TODO in
+    // RenderWidgetHostViewAndroid::OnFrameMetadataUpdated. RWHVA isn't
+    // initialized with VR state so the initial frame metadata top controls
+    // height can be dropped when a new RWHVA is created.
+    view_.OnTopControlsChanged(prev_top_controls_translate_,
+                               prev_top_shown_pix_);
+    view_.OnBottomControlsChanged(prev_bottom_controls_translate_,
+                                  prev_bottom_shown_pix_);
+  }
 }
 
 bool RenderWidgetHostViewAndroid::IsInVR() const {

content/browser/renderer_host/render_widget_host_view_android.h

@@ -479,7 +479,9 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid
 
   bool controls_initialized_ = false;
   float prev_top_shown_pix_;
+  float prev_top_controls_translate_;
   float prev_bottom_shown_pix_;
+  float prev_bottom_controls_translate_;
   float page_scale_;
   float min_page_scale_;
   float max_page_scale_;