Improve timing of Android resize transitions.

Currently, when the screen resizes on Android (typically due to an
orientation change), Android immediately starts animating once it
receives any post-resize frame. Unfortunately, this frame typically
isn't the right size yet, and even if the overall frame is the right
size, the renderer contents are not.

To avoid this we now:
- Take the compositor lock when a renderer is resized and release it
  when we produce a frame of the right size.
- Check whether the browser frame being swapped is the right size
  before releasing the OS-level lock which triggers the animation.

Bug: 835102
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I58c2633f5fc94b4e472e6f182309e4d96f17353d
Reviewed-on: https://chromium-review.googlesource.com/1031129
Commit-Queue: Eric Karl <ericrk@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557075}

chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java

@@ -318,7 +318,7 @@ private void didSwapFrame(int pendingFrameCount) {
     }
 
     @CalledByNative
-    private void didSwapBuffers() {
+    private void didSwapBuffers(boolean swappedCurrentSize) {
         // If we're in the middle of a surface swap, then see if we've received a new frame yet for
         // the new surface before hiding the outgoing surface.
         if (mFramesUntilHideBackground > 1) {
@@ -333,7 +333,10 @@ private void didSwapBuffers() {
             mCompositorSurfaceManager.doneWithUnownedSurface();
         }
 
-        runDrawFinishedCallbacks();
+        // Only run our draw finished callbacks if the frame we swapped was the correct size.
+        if (swappedCurrentSize) {
+            runDrawFinishedCallbacks();
+        }
     }
 
     /**

chrome/browser/android/compositor/compositor_view.cc

@@ -127,9 +127,11 @@ void CompositorView::DidSwapFrame(int pending_frames) {
   Java_CompositorView_didSwapFrame(env, obj_, pending_frames);
 }
 
-void CompositorView::DidSwapBuffers() {
+void CompositorView::DidSwapBuffers(const gfx::Size& swap_size) {
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_CompositorView_didSwapBuffers(env, obj_);
+  bool swapped_current_size =
+      swap_size == gfx::Size(content_width_, content_height_);
+  Java_CompositorView_didSwapBuffers(env, obj_, swapped_current_size);
 }
 
 ui::UIResourceProvider* CompositorView::GetUIResourceProvider() {

chrome/browser/android/compositor/compositor_view.h

@@ -89,7 +89,7 @@ class CompositorView : public content::CompositorClient,
   // CompositorClient implementation:
   void UpdateLayerTreeHost() override;
   void DidSwapFrame(int pending_frames) override;
-  void DidSwapBuffers() override;
+  void DidSwapBuffers(const gfx::Size& swap_size) override;
   ui::UIResourceProvider* GetUIResourceProvider();
 
  private:

components/viz/common/display/renderer_settings.h

@@ -33,6 +33,7 @@ class VIZ_COMMON_EXPORT RendererSettings {
   bool allow_overlays = true;
   bool dont_round_texture_sizes_for_pixel_tests = false;
   int highp_threshold_min = 0;
+  bool auto_resize_output_surface = true;
 
   int slow_down_compositing_scale_factor = 1;
 

components/viz/service/display/display.cc

@@ -315,7 +315,8 @@ bool Display::DrawAndSwap() {
   gfx::Size surface_size;
   bool have_damage = false;
   auto& last_render_pass = *frame.render_pass_list.back();
-  if (last_render_pass.output_rect.size() != current_surface_size_ &&
+  if (settings_.auto_resize_output_surface &&
+      last_render_pass.output_rect.size() != current_surface_size_ &&
       last_render_pass.damage_rect == last_render_pass.output_rect &&
       !current_surface_size_.IsEmpty()) {
     // Resize the output rect to the current surface size so that we won't

content/browser/renderer_host/compositor_impl_android.cc

@@ -257,7 +257,7 @@ class AndroidOutputSurface : public viz::OutputSurface {
  public:
   AndroidOutputSurface(
       scoped_refptr<ui::ContextProviderCommandBuffer> context_provider,
-      base::Closure swap_buffers_callback)
+      base::RepeatingCallback<void(gfx::Size)> swap_buffers_callback)
       : viz::OutputSurface(std::move(context_provider)),
         swap_buffers_callback_(std::move(swap_buffers_callback)),
         overlay_candidate_validator_(
@@ -272,9 +272,10 @@ class AndroidOutputSurface : public viz::OutputSurface {
     if (LatencyInfoHasSnapshotRequest(frame.latency_info))
       GetCommandBufferProxy()->SetSnapshotRequested();
 
-    auto callback = base::BindOnce(
-        &AndroidOutputSurface::OnSwapBuffersCompleted,
-        weak_ptr_factory_.GetWeakPtr(), std::move(frame.latency_info));
+    auto callback =
+        base::BindOnce(&AndroidOutputSurface::OnSwapBuffersCompleted,
+                       weak_ptr_factory_.GetWeakPtr(),
+                       std::move(frame.latency_info), frame.size);
     uint32_t flags = 0;
     if (frame.need_presentation_feedback)
       flags |= gpu::SwapBuffersFlags::kPresentationFeedback;
@@ -347,9 +348,10 @@ class AndroidOutputSurface : public viz::OutputSurface {
   }
 
   void OnSwapBuffersCompleted(std::vector<ui::LatencyInfo> latency_info,
+                              gfx::Size swap_size,
                               const gpu::SwapBuffersCompleteParams& params) {
     client_->DidReceiveSwapBuffersAck(params.swap_response.swap_id);
-    swap_buffers_callback_.Run();
+    swap_buffers_callback_.Run(swap_size);
     UpdateLatencyInfoOnSwap(params.swap_response, &latency_info);
     RenderWidgetHostImpl::OnGpuSwapBuffersCompleted(latency_info);
     latency_tracker_.OnGpuSwapBuffersCompleted(latency_info);
@@ -362,7 +364,7 @@ class AndroidOutputSurface : public viz::OutputSurface {
 
  private:
   viz::OutputSurfaceClient* client_ = nullptr;
-  base::Closure swap_buffers_callback_;
+  base::RepeatingCallback<void(gfx::Size)> swap_buffers_callback_;
   std::unique_ptr<viz::OverlayCandidateValidator> overlay_candidate_validator_;
   ui::LatencyTracker latency_tracker_;
 
@@ -852,6 +854,7 @@ void CompositorImpl::InitializeDisplay(
   viz::RendererSettings renderer_settings;
   renderer_settings.allow_antialiasing = false;
   renderer_settings.highp_threshold_min = 2048;
+  renderer_settings.auto_resize_output_surface = false;
   auto* gpu_memory_buffer_manager = BrowserMainLoop::GetInstance()
                                         ->gpu_channel_establish_factory()
                                         ->GetGpuMemoryBufferManager();
@@ -880,8 +883,8 @@ void CompositorImpl::InitializeDisplay(
   host_->SetLayerTreeFrameSink(std::move(layer_tree_frame_sink));
 }
 
-void CompositorImpl::DidSwapBuffers() {
-  client_->DidSwapBuffers();
+void CompositorImpl::DidSwapBuffers(gfx::Size swap_size) {
+  client_->DidSwapBuffers(swap_size);
 }
 
 cc::UIResourceId CompositorImpl::CreateUIResource(

content/browser/renderer_host/compositor_impl_android.h

@@ -160,7 +160,7 @@ class CONTENT_EXPORT CompositorImpl
   void InitializeDisplay(
       std::unique_ptr<viz::OutputSurface> display_output_surface,
       scoped_refptr<viz::ContextProvider> context_provider);
-  void DidSwapBuffers();
+  void DidSwapBuffers(gfx::Size swap_size);
 
   bool HavePendingReadbacks();
 

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -264,8 +264,15 @@ void RenderWidgetHostViewAndroid::InitAsFullscreen(
 }
 
 bool RenderWidgetHostViewAndroid::SynchronizeVisualProperties() {
-  if (delegated_frame_host_)
+  if (delegated_frame_host_) {
     delegated_frame_host_->SynchronizeVisualProperties();
+
+    // TODO(ericrk): This can be removed once surface synchronization is
+    // enabled. https://crbug.com/835102
+    delegated_frame_host_->PixelSizeWillChange(
+        GetCompositorViewportPixelSize());
+  }
+
   return host()->SynchronizeVisualProperties();
 }
 

content/public/browser/android/compositor_client.h

@@ -7,6 +7,7 @@
 
 #include "base/macros.h"
 #include "content/common/content_export.h"
+#include "ui/gfx/geometry/size.h"
 
 namespace content {
 
@@ -21,7 +22,7 @@ class CONTENT_EXPORT CompositorClient {
   virtual void DidSwapFrame(int pending_frames) {}
 
   // This is called on all swap buffers, regardless of cause.
-  virtual void DidSwapBuffers() {}
+  virtual void DidSwapBuffers(const gfx::Size& swap_size) {}
 
  protected:
   CompositorClient() {}

ui/android/delegated_frame_host_android.cc

@@ -4,6 +4,7 @@
 
 #include "ui/android/delegated_frame_host_android.h"
 
+#include "base/android/build_info.h"
 #include "base/bind.h"
 #include "base/logging.h"
 #include "cc/layers/solid_color_layer.h"
@@ -25,6 +26,16 @@ namespace ui {
 
 namespace {
 
+// Wait up to 5 seconds for the first frame to be produced. Having Android
+// display a placeholder for a longer period of time is preferable to drawing
+// nothing, and the first frame can take a while on low-end systems.
+static const int64_t kFirstFrameTimeoutSeconds = 5;
+
+// Wait up to 1 second for a frame of the correct size to be produced. Android
+// OS will only wait 4 seconds, so we limit this to 1 second to make sure we
+// have always produced a frame before the OS stops waiting.
+static const int64_t kResizeTimeoutSeconds = 1;
+
 constexpr viz::LocalSurfaceId kInvalidLocalSurfaceId;
 
 scoped_refptr<cc::SurfaceLayer> CreateSurfaceLayer(
@@ -99,7 +110,12 @@ void DelegatedFrameHostAndroid::SubmitCompositorFrame(
     support_->SubmitCompositorFrame(local_surface_id, std::move(frame),
                                     std::move(hit_test_region_list));
   }
+
   compositor_attach_until_frame_lock_.reset();
+
+  DCHECK(content_layer_);
+  if (content_layer_->bounds() == expected_pixel_size_)
+    compositor_pending_resize_lock_.reset();
 }
 
 void DelegatedFrameHostAndroid::DidNotProduceFrame(
@@ -196,8 +212,8 @@ void DelegatedFrameHostAndroid::AttachToCompositor(
   // frame from being produced. If we already have delegated content, no need
   // to take the lock.
   if (compositor->IsDrawingFirstVisibleFrame() && !HasDelegatedContent()) {
-    compositor_attach_until_frame_lock_ =
-        compositor->GetCompositorLock(this, base::TimeDelta::FromSeconds(5));
+    compositor_attach_until_frame_lock_ = compositor->GetCompositorLock(
+        this, base::TimeDelta::FromSeconds(kFirstFrameTimeoutSeconds));
   }
   compositor->AddChildFrameSink(frame_sink_id_);
   client_->SetBeginFrameSource(&begin_frame_source_);
@@ -208,6 +224,7 @@ void DelegatedFrameHostAndroid::DetachFromCompositor() {
   if (!registered_parent_compositor_)
     return;
   compositor_attach_until_frame_lock_.reset();
+  compositor_pending_resize_lock_.reset();
   client_->SetBeginFrameSource(nullptr);
   support_->SetNeedsBeginFrame(false);
   registered_parent_compositor_->RemoveChildFrameSink(frame_sink_id_);
@@ -221,6 +238,26 @@ void DelegatedFrameHostAndroid::SynchronizeVisualProperties() {
   // delegated_frame_host.cc. https://crbug.com/801350
 }
 
+void DelegatedFrameHostAndroid::PixelSizeWillChange(
+    const gfx::Size& pixel_size) {
+  // We never take the resize lock unless we're on O+, as previous versions of
+  // Android won't wait for us to produce the correct sized frame and will end
+  // up looking worse.
+  if (base::android::BuildInfo::GetInstance()->sdk_int() <
+      base::android::SDK_VERSION_OREO) {
+    return;
+  }
+
+  expected_pixel_size_ = pixel_size;
+  if (registered_parent_compositor_) {
+    if (!content_layer_ || content_layer_->bounds() != expected_pixel_size_) {
+      compositor_pending_resize_lock_ =
+          registered_parent_compositor_->GetCompositorLock(
+              this, base::TimeDelta::FromSeconds(kResizeTimeoutSeconds));
+    }
+  }
+}
+
 void DelegatedFrameHostAndroid::DidReceiveCompositorFrameAck(
     const std::vector<viz::ReturnedResource>& resources) {
   client_->ReclaimResources(resources);

ui/android/delegated_frame_host_android.h

@@ -89,6 +89,10 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
 
   void SynchronizeVisualProperties();
 
+  // Called when we begin a resize operation. Takes the compositor lock until we
+  // receive a frame of the expected size.
+  void PixelSizeWillChange(const gfx::Size& pixel_size);
+
   // Returns the ID for the current Surface. Returns an invalid ID if no
   // surface exists (!HasDelegatedContent()).
   const viz::SurfaceId& SurfaceId() const;
@@ -143,11 +147,19 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
   const bool enable_surface_synchronization_;
   viz::ParentLocalSurfaceIdAllocator local_surface_id_allocator_;
 
+  // The size we are resizing to. Once we receive a frame of this size we can
+  // release any resize compositor lock.
+  gfx::Size expected_pixel_size_;
+
   // A lock that is held from the point at which we attach to the compositor to
   // the point at which we submit our first frame to the compositor. This
   // ensures that the compositor doesn't swap without a frame available.
   std::unique_ptr<ui::CompositorLock> compositor_attach_until_frame_lock_;
 
+  // A lock that is held from the point we begin resizing this frame to the
+  // point at which we receive a frame of the correct size.
+  std::unique_ptr<ui::CompositorLock> compositor_pending_resize_lock_;
+
   DISALLOW_COPY_AND_ASSIGN(DelegatedFrameHostAndroid);
 };