Hide fullscreen rotation jank

content/browser/renderer_host/render_widget_host_view_android.cc

Index: content/browser/renderer_host/render_widget_host_view_android.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc
index b9898a869d9c3e42e0e206cce3d8230a93d8aee9..0c804f258cd16e0bdbba93e38419e16aeab4cd73 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.cc
+++ b/content/browser/renderer_host/render_widget_host_view_android.cc
@@ -29,6 +29,7 @@
 #include "cc/output/copy_output_request.h"
 #include "cc/output/copy_output_result.h"
 #include "cc/output/latency_info_swap_promise.h"
+#include "cc/quads/solid_color_draw_quad.h"
 #include "cc/resources/single_release_callback.h"
 #include "cc/surfaces/surface.h"
 #include "cc/surfaces/surface_hittest.h"
@@ -559,6 +560,25 @@ RenderWidgetHostViewAndroid::GetRenderWidgetHost() const {
 }
 
 void RenderWidgetHostViewAndroid::WasResized() {
+  // If we're entering a fullscreen transition, show black until the transition
+  // is completed.
+  if (content_view_core_ &&
+      view_.GetPhysicalBackingSize() != prev_physical_backing_size_) {
+    bool is_fullscreen = content_view_core_->GetWebContents()->IsFullscreen();
+    if (is_fullscreen || was_fullscreen_) {
+      fullscreen_transitioning_ = true;
+
+      // Immediately queue a black frame to hide jank while we wait to receive a
+      // "good" frame via SubmitCompositorFrame.
+      cc::CompositorFrame frame;
+      frame.render_pass_list.push_back(CreateBlackRenderPass());
+      frame.metadata.root_background_color = SK_ColorBLACK;
+      delegated_frame_host_->ForceFrame(std::move(frame));
+      UpdateBackgroundColor(SK_ColorBLACK);
+    }
+    was_fullscreen_ = is_fullscreen;
+    prev_physical_backing_size_ = view_.GetPhysicalBackingSize();

[Khushal, 2017/05/24 05:14:22]

I'm a bit confused about saving the previous fullscreen and size state here. We
just want to make sure that for fullscreen transitions, the physical backing
size matches the frame size right? Would it be sufficient to do:

bool IsInFullscreenTransition() const {
   if (content_view_core_->IsFullscreen() || current_frame_is_fullscreen_) {
      return (content_view_core_->IsFullscreen() !=
current_frame_is_fullscreen_)
                || (view_.GetPhysicalBackingSize() != current_surface_size_);
  }
}

The current_frame stuff stored based on the CompositorFrameMetadata from the
last frame.

// If either the browser or renderer's frame is in fullscreen state, fallback to
a black frame when sizes mismatch.

}

[steimel, 2017/05/24 19:32:58]

Hmm I may be able to simplify some of the logic, but the issue with this is that
it doesn't take into account the top_controls stuff, and we'll need a bool that
keeps track of whether or not we're still in a fullscreen transition since
coming out of one we'll hit a point where both
content_view_core_->IsFullscreen() and current_frame_is_fullscreen_ are false,
but we're still blocking frames for other reasons. But we'll only want to check
those reasons when we're mid-transition if that makes sense. We can discuss this
more offline. I'm also planning to cover this logic on the document I'm working
on

+  }
   host_->WasResized();
 }
 
@@ -1181,6 +1201,67 @@ void RenderWidgetHostViewAndroid::DidCreateNewRendererCompositorFrameSink(
   surface_returned_resources_.clear();
 }
 
+std::unique_ptr<cc::RenderPass>
+RenderWidgetHostViewAndroid::CreateBlackRenderPass() {
+  gfx::Rect rect(current_surface_size_);
+  std::unique_ptr<cc::RenderPass> rp = cc::RenderPass::Create();
+  rp->SetNew(1, rect, gfx::Rect(), gfx::Transform());
+  rp->has_transparent_background = false;
+  cc::SharedQuadState* sqs = rp->CreateAndAppendSharedQuadState();
+  sqs->SetAll(gfx::Transform(), rect, rect, rect, false, 1.0,
+              SkBlendMode::kColor, 1);
+  cc::SolidColorDrawQuad* quad =
+      rp->CreateAndAppendDrawQuad<cc::SolidColorDrawQuad>();
+  quad->SetNew(sqs, rect, rect, SK_ColorBLACK, false);
+  return rp;
+}
+
+bool RenderWidgetHostViewAndroid::CheckForFullscreenTransitionJank(
+    const cc::CompositorFrameMetadata& metadata) {
+  // Sometimes we'll receive a new frame before WasResized initializes a
+  // fullscreen transition. In that case, we'll block everything until
+  // WasResized is called.
+  if (content_view_core_ &&
+      content_view_core_->GetWebContents()->IsFullscreen() != was_fullscreen_) {
+    return true;
+  }
+
+  // We only check for fullscreen jank during a fullscreen transition.
+  if (fullscreen_transitioning_) {
+    bool top_controls_transitioning = metadata.top_controls_shown_ratio > 0 &&
+                                      metadata.top_controls_shown_ratio < 1;
+    bool bottom_controls_transitioning =
+        metadata.bottom_controls_shown_ratio > 0 &&
+        metadata.bottom_controls_shown_ratio < 1;
+    bool mismatched_fullscreen_states =
+        content_view_core_ &&
+        (content_view_core_->GetWebContents()->IsFullscreen() !=
+         metadata.is_fullscreen);
+    bool mismatched_layer_sizes =
+        view_.GetPhysicalBackingSize() != metadata.device_viewport_size;

[Khushal, 2017/05/24 05:14:23]

Shouldn't we be comparing this with the |current_surface_size_|?

[steimel, 2017/05/24 19:32:58]

Well, view_.GetPhysicalBackingSize is the "correct" physical size we're looking
for, and metadata.device_viewport_size is the physical size of the incoming
frame. current_surface_size_ is the size of the last good frame we received.
Unless you mean that I would compare what the new frame *would* set
current_surface_size_ to with view_.GetPhysicalBackingSize, which could
potentially work but I haven't checked it out.

[Khushal, 2017/05/25 03:33:40]

Yup, my bad for not stating that clearly. But you should compare it to the size
of the root render pass, which is the size of the incoming frame.

[steimel, 2017/06/06 03:07:53]

Done.

+
+    gfx::Size modified_scrollable_viewport_size =
+        gfx::ToFlooredSize(gfx::ScaleSize(metadata.scrollable_viewport_size,
+                                          metadata.page_scale_factor));
+    bool controls_will_transition =

[Khushal, 2017/05/24 05:14:23]

Could you describe what the behaviour is for top controls during the transition?
I think we go into this persistent fullscreen state that hides the toolbar
android view in fullscreen. But I'm not totally clear on how that affects the
composited toolbar. And what happens in the out of fullscreen transition.

[steimel, 2017/05/24 19:32:58]

So this particular check is something that's used when exiting fullscreen. There
is jank that happens as the top controls expands from shown_ratio == 0 to
shown_ratio == 1. This is normally hidden via the top_controls_transitioning
condition above. However, there are times where the exiting fullscreen
transition gets to a "good" state before the top controls begin their expansion.
This means that the fullscreen_transition will be marked done and we'll be
showing frames and therefore miss the chance to hide that jank. I don't have a
full understanding of the details, but content_view_core_->GetViewSize seems to
be aware that it's about to lose the height space from the controls before they
start coming down which is why this check is true when the controls are coming
down but haven't started yet. I did just realize that for completeness I should
probably check that shown_ratio == 0 and that DoBrowserControlsShrinkBlinkSize()
== true (I should also check that bool for the transitioning conditions).

+        content_view_core_ && (content_view_core_->GetViewSize().width() ==
+                                   modified_scrollable_viewport_size.width() &&
+                               content_view_core_->GetViewSize().height() !=
+                                   modified_scrollable_viewport_size.height());
+
+    if (top_controls_transitioning || bottom_controls_transitioning ||
+        mismatched_fullscreen_states || mismatched_layer_sizes ||
+        controls_will_transition) {
+      return true;
+    } else {
+      // Done with current fullscreen transition.
+      fullscreen_transitioning_ = false;
+    }
+  }
+
+  return false;
+}
+
 void RenderWidgetHostViewAndroid::SubmitCompositorFrame(
     const cc::LocalSurfaceId& local_surface_id,
     cc::CompositorFrame frame) {
@@ -1189,6 +1270,14 @@ void RenderWidgetHostViewAndroid::SubmitCompositorFrame(
     return;
   }
 
+  if (CheckForFullscreenTransitionJank(frame.metadata)) {
+    // Block this frame since it will look janky.
+    frame.render_pass_list.clear();
+    frame.render_pass_list.push_back(CreateBlackRenderPass());
+    frame.metadata.root_background_color = SK_ColorBLACK;
+    UpdateBackgroundColor(SK_ColorBLACK);
+  }
+
   last_scroll_offset_ = frame.metadata.root_scroll_offset;
   DCHECK(!frame.render_pass_list.empty());