Remove synchronous draw from content interface

This allows a bunch of code to be deleted as well.

Need to fix up unit tests which was still using synchronous draw, and
thus having wrong expectations.

Change-Id: Ia5504545f213bf709951eb2213dc40aaca4fc6be
Reviewed-on: https://chromium-review.googlesource.com/949085
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541729}

android_webview/browser/browser_view_renderer.cc

@@ -9,7 +9,6 @@
 
 #include "android_webview/browser/browser_view_renderer_client.h"
 #include "android_webview/browser/compositor_frame_consumer.h"
-#include "android_webview/common/aw_switches.h"
 #include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/logging.h"
@@ -94,8 +93,6 @@ BrowserViewRenderer::BrowserViewRenderer(
     const scoped_refptr<base::SingleThreadTaskRunner>& ui_task_runner)
     : client_(client),
       ui_task_runner_(ui_task_runner),
-      sync_on_draw_hardware_(base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kSyncOnDrawHardware)),
       current_compositor_frame_consumer_(nullptr),
       compositor_(nullptr),
       is_paused_(false),
@@ -234,28 +231,13 @@ bool BrowserViewRenderer::OnDrawHardware() {
   gfx::Rect viewport_rect_for_tile_priority =
       ComputeViewportRectForTilePriority();
 
-  scoped_refptr<content::SynchronousCompositor::FrameFuture> future; // Async.
-  content::SynchronousCompositor::Frame frame; // Sync.
-  if (sync_on_draw_hardware_) {
-    // TODO(boliu): Remove the synchronous call path here.
-    frame = compositor_->DemandDrawHw(size_, viewport_rect_for_tile_priority,
-                                      transform_for_tile_priority);
-  } else {
-    future = compositor_->DemandDrawHwAsync(
-        size_, viewport_rect_for_tile_priority, transform_for_tile_priority);
-  }
-
-  if (!frame.frame && !future) {
-    TRACE_EVENT_INSTANT0("android_webview", "NoNewFrame",
-                         TRACE_EVENT_SCOPE_THREAD);
-    return current_compositor_frame_consumer_->HasFrameOnUI();
-  }
-
+  scoped_refptr<content::SynchronousCompositor::FrameFuture> future =
+      compositor_->DemandDrawHwAsync(size_, viewport_rect_for_tile_priority,
+                                     transform_for_tile_priority);
   std::unique_ptr<ChildFrame> child_frame = std::make_unique<ChildFrame>(
-      std::move(future), frame.layer_tree_frame_sink_id, std::move(frame.frame),
-      compositor_id_, viewport_rect_for_tile_priority.IsEmpty(),
-      transform_for_tile_priority, offscreen_pre_raster_,
-      external_draw_constraints_.is_layer);
+      std::move(future), compositor_id_,
+      viewport_rect_for_tile_priority.IsEmpty(), transform_for_tile_priority,
+      offscreen_pre_raster_, external_draw_constraints_.is_layer);
 
   ReturnUnusedResource(
       current_compositor_frame_consumer_->SetFrameOnUI(std::move(child_frame)));

android_webview/browser/browser_view_renderer.h

@@ -179,7 +179,6 @@ class BrowserViewRenderer : public content::SynchronousCompositorClient,
 
   BrowserViewRendererClient* const client_;
   const scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
-  const bool sync_on_draw_hardware_;
   CompositorFrameConsumer* current_compositor_frame_consumer_;
   std::set<CompositorFrameConsumer*> compositor_frame_consumers_;
 

android_webview/browser/browser_view_renderer_unittest.cc

@@ -269,17 +269,13 @@ class CompositorNoFrameTest : public RenderingTest {
   }
 
   void DidOnDraw(bool success) override {
+    // OnDraw should succeed even when there are no frames from compositor.
+    EXPECT_TRUE(success);
     if (0 == on_draw_count_) {
-      // Should fail as there has been no frames from compositor.
-      EXPECT_FALSE(success);
       browser_view_renderer_->PostInvalidate(ActiveCompositor());
     } else if (1 == on_draw_count_) {
-      // Should succeed with frame from compositor.
-      EXPECT_TRUE(success);
       browser_view_renderer_->PostInvalidate(ActiveCompositor());
     } else if (2 == on_draw_count_) {
-      // Should still succeed with last frame, even if no frame from compositor.
-      EXPECT_TRUE(success);
       EndTest();
     }
     on_draw_count_++;

android_webview/browser/child_frame.cc

@@ -13,16 +13,12 @@ namespace android_webview {
 
 ChildFrame::ChildFrame(
     scoped_refptr<content::SynchronousCompositor::FrameFuture> frame_future,
-    uint32_t layer_tree_frame_sink_id,
-    std::unique_ptr<viz::CompositorFrame> frame,
     const CompositorID& compositor_id,
     bool viewport_rect_for_tile_priority_empty,
     const gfx::Transform& transform_for_tile_priority,
     bool offscreen_pre_raster,
     bool is_layer)
     : frame_future(std::move(frame_future)),
-      layer_tree_frame_sink_id(layer_tree_frame_sink_id),
-      frame(std::move(frame)),
       compositor_id(compositor_id),
       viewport_rect_for_tile_priority_empty(
           viewport_rect_for_tile_priority_empty),

android_webview/browser/child_frame.h

@@ -25,8 +25,6 @@ class ChildFrame {
  public:
   ChildFrame(
       scoped_refptr<content::SynchronousCompositor::FrameFuture> frame_future,
-      uint32_t layer_tree_frame_sink_id,
-      std::unique_ptr<viz::CompositorFrame> frame,
       const CompositorID& compositor_id,
       bool viewport_rect_for_tile_priority_empty,
       const gfx::Transform& transform_for_tile_priority,
@@ -37,11 +35,10 @@ class ChildFrame {
   // Helper to move frame from |frame_future| to |frame|.
   void WaitOnFutureIfNeeded();
 
-  // This is used in async ondraw path. The frame is either in |frame_future|
-  // or |frame|. It's illegal if both are non-null.
+  // The frame is either in |frame_future| or |frame|. It's illegal if both
+  // are non-null.
   scoped_refptr<content::SynchronousCompositor::FrameFuture> frame_future;
-  // These two fields are not const to make async path easier.
-  uint32_t layer_tree_frame_sink_id;
+  uint32_t layer_tree_frame_sink_id = 0u;
   std::unique_ptr<viz::CompositorFrame> frame;
   // The id of the compositor this |frame| comes from.
   const CompositorID compositor_id;

android_webview/browser/compositor_frame_consumer.h

@@ -51,7 +51,6 @@ class CompositorFrameConsumer {
       ReturnedResourcesMap* returned_resource_map) = 0;
   virtual bool ReturnedResourcesEmptyOnUI() const = 0;
   virtual ChildFrameQueue PassUncommittedFrameOnUI() = 0;
-  virtual bool HasFrameOnUI() const = 0;
   virtual void DeleteHardwareRendererOnUI() = 0;
 
  protected:

android_webview/browser/render_thread_manager.cc

@@ -178,27 +178,14 @@ gfx::Vector2d RenderThreadManager::GetScrollOffsetOnRT() {
 std::unique_ptr<ChildFrame> RenderThreadManager::SetFrameOnUI(
     std::unique_ptr<ChildFrame> new_frame) {
   DCHECK(new_frame);
-  base::AutoLock lock(lock_);
-
   has_received_frame_ = true;
 
+  base::AutoLock lock(lock_);
   if (child_frames_.empty()) {
     child_frames_.emplace_back(std::move(new_frame));
     return nullptr;
   }
   std::unique_ptr<ChildFrame> uncommitted_frame;
-  if (new_frame->frame) {
-    // Optimization for synchronous path.
-    // TODO(boliu): Remove when synchronous path is fully removed.
-    DCHECK_LE(child_frames_.size(), 1u);
-    if (!child_frames_.empty()) {
-      uncommitted_frame = std::move(child_frames_.front());
-      child_frames_.pop_front();
-    }
-    child_frames_.emplace_back(std::move(new_frame));
-    return uncommitted_frame;
-  }
-
   DCHECK_LE(child_frames_.size(), 2u);
   ChildFrameQueue pruned_frames =
       HardwareRenderer::WaitAndPruneFrameQueue(&child_frames_);
@@ -402,11 +389,6 @@ void RenderThreadManager::SetCompositorFrameProducer(
   compositor_frame_producer_ = compositor_frame_producer;
 }
 
-bool RenderThreadManager::HasFrameOnUI() const {
-  base::AutoLock lock(lock_);
-  return has_received_frame_;
-}
-
 bool RenderThreadManager::HasFrameForHardwareRendererOnRT() const {
   base::AutoLock lock(lock_);
   return !child_frames_.empty();

android_webview/browser/render_thread_manager.h

@@ -55,7 +55,6 @@ class RenderThreadManager : public CompositorFrameConsumer {
       ReturnedResourcesMap* returned_resource_map) override;
   bool ReturnedResourcesEmptyOnUI() const override;
   ChildFrameQueue PassUncommittedFrameOnUI() override;
-  bool HasFrameOnUI() const override;
   void DeleteHardwareRendererOnUI() override;
 
   // Render thread methods.

android_webview/browser/test/rendering_test.cc

@@ -10,7 +10,6 @@
 #include "android_webview/browser/browser_view_renderer.h"
 #include "android_webview/browser/child_frame.h"
 #include "android_webview/browser/render_thread_manager.h"
-#include "android_webview/common/aw_switches.h"
 #include "base/command_line.h"
 #include "base/location.h"
 #include "base/message_loop/message_loop.h"
@@ -48,9 +47,6 @@ class TestBrowserViewRenderer : public BrowserViewRenderer {
 }  // namespace
 
 RenderingTest::RenderingTest() : message_loop_(new base::MessageLoop) {
-  // TODO(boliu): Update unit tests to async code path.
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kSyncOnDrawHardware);
   ui_task_runner_ = base::ThreadTaskRunnerHandle::Get();
 }
 

android_webview/common/aw_switches.cc

@@ -7,7 +7,6 @@
 namespace switches {
 
 const char kEnableWebViewVariations[] = "enable-webview-variations";
-const char kSyncOnDrawHardware[] = "sync-on-draw-hardware";
 const char kWebViewSandboxedRenderer[] = "webview-sandboxed-renderer";
 
 // used to enable safebrowsing functionality in webview

android_webview/common/aw_switches.h

@@ -8,7 +8,6 @@
 namespace switches {
 
 extern const char kEnableWebViewVariations[];
-extern const char kSyncOnDrawHardware[];
 extern const char kWebViewSandboxedRenderer[];
 extern const char kWebViewEnableSafeBrowsingSupport[];
 extern const char kWebViewDisableSafeBrowsingSupport[];

content/browser/android/synchronous_compositor_host.h

@@ -51,10 +51,6 @@ class SynchronousCompositorHost : public SynchronousCompositor,
   ~SynchronousCompositorHost() override;
 
   // SynchronousCompositor overrides.
-  SynchronousCompositor::Frame DemandDrawHw(
-      const gfx::Size& viewport_size,
-      const gfx::Rect& viewport_rect_for_tile_priority,
-      const gfx::Transform& transform_for_tile_priority) override;
   scoped_refptr<FrameFuture> DemandDrawHwAsync(
       const gfx::Size& viewport_size,
       const gfx::Rect& viewport_rect_for_tile_priority,
@@ -100,6 +96,10 @@ class SynchronousCompositorHost : public SynchronousCompositor,
 
   SynchronousCompositorHost(RenderWidgetHostViewAndroid* rwhva,
                             bool use_in_proc_software_draw);
+  SynchronousCompositor::Frame DemandDrawHw(
+      const gfx::Size& viewport_size,
+      const gfx::Rect& viewport_rect_for_tile_priority,
+      const gfx::Transform& transform_for_tile_priority);
   bool DemandDrawSwInProc(SkCanvas* canvas);
   void SetSoftwareDrawSharedMemoryIfNeeded(size_t stride, size_t buffer_size);
   void SendZeroMemory();

content/public/browser/android/synchronous_compositor.h

@@ -79,12 +79,6 @@ class CONTENT_EXPORT SynchronousCompositor {
   // |viewport_size| is the current size to improve results during resize.
   // |viewport_rect_for_tile_priority| and |transform_for_tile_priority| are
   // used to customize the tiling decisions of compositor.
-  virtual Frame DemandDrawHw(
-      const gfx::Size& viewport_size,
-      const gfx::Rect& viewport_rect_for_tile_priority,
-      const gfx::Transform& transform_for_tile_priority) = 0;
-
-  // Same as DemandDrawHw, but uses asynchronous IPC messages.
   virtual scoped_refptr<FrameFuture> DemandDrawHwAsync(
       const gfx::Size& viewport_size,
       const gfx::Rect& viewport_rect_for_tile_priority,

content/public/test/test_synchronous_compositor_android.cc

@@ -26,19 +26,14 @@ void TestSynchronousCompositor::SetClient(SynchronousCompositorClient* client) {
     client_->DidInitializeCompositor(this, process_id_, routing_id_);
 }
 
-SynchronousCompositor::Frame TestSynchronousCompositor::DemandDrawHw(
-    const gfx::Size& viewport_size,
-    const gfx::Rect& viewport_rect_for_tile_priority,
-    const gfx::Transform& transform_for_tile_priority) {
-  return std::move(hardware_frame_);
-}
-
 scoped_refptr<SynchronousCompositor::FrameFuture>
 TestSynchronousCompositor::DemandDrawHwAsync(
     const gfx::Size& viewport_size,
     const gfx::Rect& viewport_rect_for_tile_priority,
     const gfx::Transform& transform_for_tile_priority) {
-  return nullptr;
+  auto future = base::MakeRefCounted<FrameFuture>();
+  future->SetFrame(std::move(hardware_frame_));
+  return future;
 }
 
 void TestSynchronousCompositor::ReturnResources(
@@ -63,8 +58,9 @@ bool TestSynchronousCompositor::DemandDrawSw(SkCanvas* canvas) {
 void TestSynchronousCompositor::SetHardwareFrame(
     uint32_t layer_tree_frame_sink_id,
     std::unique_ptr<viz::CompositorFrame> frame) {
-  hardware_frame_.layer_tree_frame_sink_id = layer_tree_frame_sink_id;
-  hardware_frame_.frame = std::move(frame);
+  hardware_frame_ = std::make_unique<Frame>();
+  hardware_frame_->layer_tree_frame_sink_id = layer_tree_frame_sink_id;
+  hardware_frame_->frame = std::move(frame);
 }
 
 TestSynchronousCompositor::ReturnedResources::ReturnedResources()

content/public/test/test_synchronous_compositor_android.h

@@ -23,11 +23,6 @@ class CONTENT_EXPORT TestSynchronousCompositor : public SynchronousCompositor {
 
   void SetClient(SynchronousCompositorClient* client);
 
-  // SynchronousCompositor overrides.
-  SynchronousCompositor::Frame DemandDrawHw(
-      const gfx::Size& viewport_size,
-      const gfx::Rect& viewport_rect_for_tile_priority,
-      const gfx::Transform& transform_for_tile_priority) override;
   scoped_refptr<FrameFuture> DemandDrawHwAsync(
       const gfx::Size& viewport_size,
       const gfx::Rect& viewport_rect_for_tile_priority,
@@ -61,7 +56,7 @@ class CONTENT_EXPORT TestSynchronousCompositor : public SynchronousCompositor {
   SynchronousCompositorClient* client_;
   const int process_id_;
   const int routing_id_;
-  SynchronousCompositor::Frame hardware_frame_;
+  std::unique_ptr<Frame> hardware_frame_;
   FrameAckArray frame_ack_array_;
 
   DISALLOW_COPY_AND_ASSIGN(TestSynchronousCompositor);