content: Fix race that can cause premature resize ack

When the main thread receives DidCommitAndSwap from the impl thread and
the main thread hasn't commited after resize, DidCommitAndSwap belongs
to the previous commit and should not be used for acking the resize.
In other words, (Resize,DidCommit,DidCommitAndSwap) should ack the
resize but (DidCommit,Resize,DidCommitAndSwap) should not.

Change-Id: Ib55c4acb82a90560e1a75d7327e08321b1bf7f0c
Reviewed-on: https://chromium-review.googlesource.com/883909Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533305}

content/renderer/render_widget.cc

@@ -1015,7 +1015,10 @@ void RenderWidget::DidCommitAndDrawCompositorFrame() {
   // tab_capture_performancetest.cc.
   TRACE_EVENT0("gpu", "RenderWidget::DidCommitAndDrawCompositorFrame");
 
-  DidResizeOrRepaintAck();
+  // If we haven't commited yet, then this method was called as a response to a
+  // previous commit and should not be used to ack the resize.
+  if (did_commit_after_resize_)
+    DidResizeOrRepaintAck();
 
   for (auto& observer : render_frames_)
     observer.DidCommitAndDrawCompositorFrame();
@@ -1024,7 +1027,9 @@ void RenderWidget::DidCommitAndDrawCompositorFrame() {
   DidInitiatePaint();
 }
 
-void RenderWidget::DidCommitCompositorFrame() {}
+void RenderWidget::DidCommitCompositorFrame() {
+  did_commit_after_resize_ = true;
+}
 
 void RenderWidget::DidCompletePageScaleAnimation() {}
 
@@ -1313,6 +1318,12 @@ void RenderWidget::Resize(const ResizeParams& params) {
 
   screen_info_ = params.screen_info;
 
+  // If this resize needs to be acked, make sure we ack it only after we commit.
+  // It is possible to get DidCommitAndDraw calls that belong to the previous
+  // commit, in which case we should not ack this resize.
+  if (params.needs_resize_ack)
+    did_commit_after_resize_ = false;
+
   RenderThreadImpl* render_thread = RenderThreadImpl::current();
   if (render_thread)
     render_thread->SetRenderingColorSpace(screen_info_.color_space);

content/renderer/render_widget.h

@@ -963,6 +963,11 @@ class CONTENT_EXPORT RenderWidget
 
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
+  // Keep track of whether we committed after the latest resize that needs to be
+  // acked was received. This helps us make sure we don't ack a resize before
+  // it's committed.
+  bool did_commit_after_resize_ = false;
+
   base::WeakPtrFactory<RenderWidget> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderWidget);

content/renderer/render_widget_browsertest.cc

@@ -85,6 +85,7 @@ TEST_F(RenderWidgetTest, OnResize) {
   EXPECT_EQ(resize_params.needs_resize_ack, next_paint_is_resize_ack());
 
   // Clear the flag.
+  widget()->DidCommitCompositorFrame();
   widget()->DidCommitAndDrawCompositorFrame();
 
   // Setting the same size again should not send the ack.