[Android] Stop hiding the RWHV layer subtree when hiding the widget

content/browser/renderer_host/render_widget_host_view_android.cc

 // Copyright (c) 2012 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.
 
 #include "content/browser/renderer_host/render_widget_host_view_android.h"
 
 #include <android/bitmap.h>
 
 #include "base/android/build_info.h"
 #include "base/basictypes.h"
@@ skipping 578 matching lines @@
 
 bool RenderWidgetHostViewAndroid::IsSurfaceAvailableForCopy() const {
   return HasValidFrame();
 }
 
 void RenderWidgetHostViewAndroid::Show() {
   if (is_showing_)
     return;
 
   is_showing_ = true;
-  if (layer_.get())
-    layer_->SetHideLayerAndSubtree(false);
-
-  if (overscroll_controller_)
-    overscroll_controller_->Enable();
-
-  frame_evictor_->SetVisible(true);
-
-  if (!host_ || !host_->is_hidden())
-    return;
-
-  host_->WasShown(ui::LatencyInfo());
-
-  if (content_view_core_) {
-    StartObservingRootWindow();
-    RequestVSyncUpdate(BEGIN_FRAME);
-  }
+  ShowInternal();
 }
 
 void RenderWidgetHostViewAndroid::Hide() {
   if (!is_showing_)
     return;
 
   is_showing_ = false;
-  if (layer_.get() && locks_on_frame_count_ == 0)
-    layer_->SetHideLayerAndSubtree(true);
 
-  if (overscroll_controller_)
-    overscroll_controller_->Disable();
-
-  frame_evictor_->SetVisible(false);
-  // We don't know if we will ever get a frame if we are hiding the renderer, so
-  // we need to cancel all requests
-  AbortPendingReadbackRequests();
-
-  RunAckCallbacks(cc::SurfaceDrawStatus::DRAW_SKIPPED);
-
-  if (!host_ || host_->is_hidden())
-    return;
-
-  // Inform the renderer that we are being hidden so it can reduce its resource
-  // utilization.
-  host_->WasHidden();
-
-  StopObservingRootWindow();
+  bool hide_frontbuffer = true;
+  bool stop_observing_root_window = true;
+  HideInternal(hide_frontbuffer, stop_observing_root_window);
 }
 
 bool RenderWidgetHostViewAndroid::IsShowing() {
   // ContentViewCoreImpl represents the native side of the Java
   // ContentViewCore.  It being NULL means that it is not attached
   // to the View system yet, so we treat this RWHVA as hidden.
   return is_showing_ && content_view_core_;
 }
 
 void RenderWidgetHostViewAndroid::LockCompositingSurface() {
@@ skipping 826 matching lines @@
       web_contents_impl->media_web_contents_observer()->OnFrameInfoUpdated();
   }
 #endif  // defined(VIDEO_HOLE)
 }
 
 void RenderWidgetHostViewAndroid::AcceleratedSurfaceInitialized(int route_id) {
   // TODO: remove need for the surface id here
   accelerated_surface_route_id_ = route_id;
 }
 
+void RenderWidgetHostViewAndroid::ShowInternal() {
+  DCHECK(is_showing_);
+
+  bool was_showing_frontbuffer =
+      layer_.get() && !layer_->hide_layer_and_subtree();
+  if (!was_showing_frontbuffer) {
+    if (layer_.get())
+      layer_->SetHideLayerAndSubtree(false);
+    frame_evictor_->SetVisible(true);
+  }
+
+  if (overscroll_controller_)
+    overscroll_controller_->Enable();
+
+  if (!host_ || !host_->is_hidden())
+    return;

[no sievers, 2015/03/19 20:21:52]

Can we move this early-out to the top?

We should never drop the frontbuffer, while not telling the renderer to hide.
Then it should also avoid redundant frame_evictor_->SetVisible() calls, and
don't need the |was_showing_frontbuffer| check.

[jdduke (slow), 2015/03/20 14:59:29]

We can probably move this call up, but I'm not sure about the hide call. 

+
+  host_->WasShown(ui::LatencyInfo());
+
+  if (content_view_core_) {
+    StartObservingRootWindow();
+    RequestVSyncUpdate(BEGIN_FRAME);
+  }
+}
+
+void RenderWidgetHostViewAndroid::HideInternal(
+    bool hide_frontbuffer,
+    bool stop_observing_root_window) {
+  if (hide_frontbuffer) {
+    if (layer_.get() && locks_on_frame_count_ == 0)
+      layer_->SetHideLayerAndSubtree(true);
+
+    frame_evictor_->SetVisible(false);
+  }
+
+  if (stop_observing_root_window)
+    StopObservingRootWindow();
+
+  if (overscroll_controller_)
+    overscroll_controller_->Disable();
+
+  // We don't know if we will ever get a frame if we are hiding the renderer, so
+  // we need to cancel all requests
+  AbortPendingReadbackRequests();
+
+  RunAckCallbacks(cc::SurfaceDrawStatus::DRAW_SKIPPED);
+
+  if (!host_ || host_->is_hidden())

[no sievers, 2015/03/19 20:21:52]

Same here.

[jdduke (slow), 2015/03/20 14:59:29]

We can't move this call up because the the host may be hidden but the
frontbuffer is still showing.

+    return;
+
+  // Inform the renderer that we are being hidden so it can reduce its resource
+  // utilization.
+  host_->WasHidden();
+}
+
 void RenderWidgetHostViewAndroid::AttachLayers() {
   if (!content_view_core_)
     return;
   if (!layer_.get())
     return;
 
   content_view_core_->AttachLayer(layer_);
   if (overscroll_controller_)
     overscroll_controller_->Enable();
   layer_->SetHideLayerAndSubtree(!is_showing_);
@@ skipping 11 matching lines @@
     overscroll_controller_->Disable();
 }
 
 void RenderWidgetHostViewAndroid::RequestVSyncUpdate(uint32 requests) {
   // The synchronous compositor does not requre BeginFrame messages.
   if (!using_browser_compositor_)
     requests &= FLUSH_INPUT;
 
   bool should_request_vsync = !outstanding_vsync_requests_ && requests;
   outstanding_vsync_requests_ |= requests;
+
+  // If the host has been hidden, defer vsync requests until it is shown
+  // again via |Show()|.
+  if (!host_ || host_->is_hidden())

[no sievers, 2015/03/18 21:04:48]

see question below

+    return;
+
   // Note that if we're not currently observing the root window, outstanding
   // vsync requests will be pushed if/when we resume observing in
   // |StartObservingRootWindow()|.
   if (observing_root_window_ && should_request_vsync)
     content_view_core_->GetWindowAndroid()->RequestVSyncUpdate();
 }
 
 void RenderWidgetHostViewAndroid::StartObservingRootWindow() {
   DCHECK(content_view_core_);
+  DCHECK(is_showing_);
   if (observing_root_window_)
     return;
 
   observing_root_window_ = true;
   content_view_core_->GetWindowAndroid()->AddObserver(this);
 
   // Clear existing vsync requests to allow a request to the new window.
   uint32 outstanding_vsync_requests = outstanding_vsync_requests_;
   outstanding_vsync_requests_ = 0;
   RequestVSyncUpdate(outstanding_vsync_requests);
@@ skipping 280 matching lines @@
     if (content_view_core_)
       obj = content_view_core_->GetJavaObject();
     manager->ToBrowserAccessibilityManagerAndroid()->SetContentViewCore(obj);
   }
 
   AttachLayers();
 
   if (!content_view_core_)
     return;
 
-  StartObservingRootWindow();
+  if (is_showing_)
+    StartObservingRootWindow();
 
   if (resize)
     WasResized();
 
   if (!selection_controller_)
     selection_controller_ = CreateSelectionController(this, content_view_core_);
 
   if (!overscroll_controller_ &&
       content_view_core_->GetWindowAndroid()->GetCompositor()) {
     overscroll_controller_ = CreateOverscrollController(content_view_core_);
@@ skipping 20 matching lines @@
       web_gesture.modifiers == blink::WebInputEvent::ShiftKey) {
     web_gesture.modifiers = 0;
   }
   SendGestureEvent(web_gesture);
 }
 
 void RenderWidgetHostViewAndroid::OnCompositingDidCommit() {
   RunAckCallbacks(cc::SurfaceDrawStatus::DRAWN);
 }
 
+void RenderWidgetHostViewAndroid::OnVisibilityChanged(bool visible) {
+  DCHECK(is_showing_);
+  if (visible) {
+    ShowInternal();
+  } else {
+    bool hide_frontbuffer = true;
+    bool stop_observing_root_window = false;
+    HideInternal(hide_frontbuffer, stop_observing_root_window);
+  }
+}
+
 void RenderWidgetHostViewAndroid::OnAttachCompositor() {
   DCHECK(content_view_core_);
   if (!overscroll_controller_)
     overscroll_controller_ = CreateOverscrollController(content_view_core_);
 }
 
 void RenderWidgetHostViewAndroid::OnDetachCompositor() {
   DCHECK(content_view_core_);
   DCHECK(using_browser_compositor_);
   RunAckCallbacks(cc::SurfaceDrawStatus::DRAW_SKIPPED);
   overscroll_controller_.reset();
 }
 
 void RenderWidgetHostViewAndroid::OnVSync(base::TimeTicks frame_time,
                                           base::TimeDelta vsync_period) {
   TRACE_EVENT0("cc,benchmark", "RenderWidgetHostViewAndroid::OnVSync");
-  if (!host_)
+  if (!host_ || host_->is_hidden())

[no sievers, 2015/03/18 21:04:48]

Doesn't the renderer turn beginFrame off when it gets hidden?  And why do we
need this check now and didn't before, is it because we now might keep observing
the root window while the window is not visible (and Hide() was not called)
while previously we relied on being completely hidden and cut off from
notifications?

Maybe it's good paranoia, but it does make it a bit more confusing on what the
vsync and beginFrame contract really is. We could also consider not ticking in
WindowAndroid if it's hidden.

[jdduke (slow), 2015/03/20 14:59:29]

Right, this preserves the original behavior which is having a hard cutoff of
BeginFrame dispatch. The compositor *should* unsubscribe from BeginFrame after
RenderWidget receives the hidden notification, but I kept the hidden check as a
precauation. I'd be OK with removing it, but I'm not sure there's a great
non-racy way to verify that we get the unsubscribe call.
Yeah, I'm fine restoring the original code, as our begin frame subscription
logic should in theory be enough to guard here. Either way.

     return;
 
   const uint32 current_vsync_requests = outstanding_vsync_requests_;
   outstanding_vsync_requests_ = 0;
 
   if (current_vsync_requests & FLUSH_INPUT)
     host_->FlushInput();
 
   if (current_vsync_requests & BEGIN_FRAME ||
       current_vsync_requests & PERSISTENT_BEGIN_FRAME) {
     SendBeginFrame(frame_time, vsync_period);
   }
 
   if (current_vsync_requests & PERSISTENT_BEGIN_FRAME)
     RequestVSyncUpdate(PERSISTENT_BEGIN_FRAME);
 }
 
 void RenderWidgetHostViewAndroid::OnAnimate(base::TimeTicks begin_frame_time) {
   if (Animate(begin_frame_time))
     SetNeedsAnimate();
 }
 
+void RenderWidgetHostViewAndroid::OnActivityResumed() {
+  DCHECK(is_showing_);
+  ShowInternal();
+}
+
+void RenderWidgetHostViewAndroid::OnActivityPaused() {
+  DCHECK(is_showing_);
+  bool hide_frontbuffer = false;
+  bool stop_observing_root_window = false;
+  HideInternal(hide_frontbuffer, stop_observing_root_window);
+}
+
 void RenderWidgetHostViewAndroid::OnLostResources() {
   ReleaseLocksOnSurface();
   if (layer_.get())
     DestroyDelegatedContent();
   DCHECK(ack_callbacks_.empty());
   // We should not loose a frame if we have readback requests pending.
   DCHECK(readbacks_waiting_for_frame_.empty());
 }
 
 // static
@@ skipping 127 matching lines @@
   results->orientationAngle = display.RotationAsDegree();
   results->orientationType =
       RenderWidgetHostViewBase::GetOrientationTypeForMobile(display);
   gfx::DeviceDisplayInfo info;
   results->depth = info.GetBitsPerPixel();
   results->depthPerComponent = info.GetBitsPerComponent();
   results->isMonochrome = (results->depthPerComponent == 0);
 }
 
 } // namespace content