Don't expect ack for ViewMsg_OnResize if backing size is empty

The renderer won't send ack for ViewMsg_OnResize if backing size is empty.
This happens on Android in some cases, e.g. when a tab is created in
background because the backing is only created when the tab is shown.

Previously on Android we let RWHV return empty view bounds when
backing is empty but it prevents the renderer from getting the correct
view bounds. Now remove the above logic from RWHV_android and change
RWHI not to expect ack when backing size is empty.

Also changed RenderWidget not to send ack on the next paint in the cases
that the host is not expecting the ack. This doesn't change the actual
behavior because RenderWidget won't send UpdateRect corresponding to the
Resize request in the cases, but just ensures the correctness.

This is a part of the fix to bug 168568. Also need to change the Java code
to set the correct view size as early as possible.

BUG=168568
TEST=RenderWidgetHostTest.Resize (content_unittests), RenderWidgetTest.OnResize (content_browsertests)

Review URL: https://chromiumcodereview.appspot.com/14779010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@200210 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 88 insertions, 15 deletions

diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index b06d749..5c09f18 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -534,9 +534,9 @@ void RenderWidgetHostImpl::WasResized() {
       !side_payload_changed)
     return;
 
-  // We don't expect to receive an ACK when the requested size is empty or when
-  // the main viewport size didn't change.
-  if (!new_size.IsEmpty() && size_changed)
+  // We don't expect to receive an ACK when the requested size or the physical
+  // backing size is empty, or when the main viewport size didn't change.
+  if (!new_size.IsEmpty() && !physical_backing_size_.IsEmpty() && size_changed)
     resize_ack_pending_ = true;
 
   if (!Send(new ViewMsg_Resize(routing_id_, new_size, physical_backing_size_,
diff --git a/content/browser/renderer_host/render_widget_host_unittest.cc b/content/browser/renderer_host/render_widget_host_unittest.cc
index b28c0ce..60360ca 100644
--- a/content/browser/renderer_host/render_widget_host_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_unittest.cc
@@ -492,7 +492,8 @@ class TestView : public TestRenderWidgetHostView {
  public:
   explicit TestView(RenderWidgetHostImpl* rwh)
       : TestRenderWidgetHostView(rwh),
-        acked_event_count_(0) {
+        acked_event_count_(0),
+        use_fake_physical_backing_size_(false) {
   }
 
   // Sets the bounds returned by GetViewBounds.
@@ -511,6 +512,14 @@ class TestView : public TestRenderWidgetHostView {
     return unhandled_wheel_event_;
   }
 
+  void SetMockPhysicalBackingSize(const gfx::Size& mock_physical_backing_size) {
+    use_fake_physical_backing_size_ = true;
+    mock_physical_backing_size_ = mock_physical_backing_size;
+  }
+  void ClearMockPhysicalBackingSize() {
+    use_fake_physical_backing_size_ = false;
+  }
+
   // RenderWidgetHostView override.
   virtual gfx::Rect GetViewBounds() const OVERRIDE {
     return bounds_;
@@ -523,12 +532,19 @@ class TestView : public TestRenderWidgetHostView {
   virtual void UnhandledWheelEvent(const WebMouseWheelEvent& event) OVERRIDE {
     unhandled_wheel_event_ = event;
   }
+  virtual gfx::Size GetPhysicalBackingSize() const OVERRIDE {
+    if (use_fake_physical_backing_size_)
+      return mock_physical_backing_size_;
+    return TestRenderWidgetHostView::GetPhysicalBackingSize();
+  }
 
  protected:
   WebMouseWheelEvent unhandled_wheel_event_;
   WebTouchEvent acked_event_;
   int acked_event_count_;
   gfx::Rect bounds_;
+  bool use_fake_physical_backing_size_;
+  gfx::Size mock_physical_backing_size_;
 
   DISALLOW_COPY_AND_ASSIGN(TestView);
 };
@@ -871,10 +887,20 @@ TEST_F(RenderWidgetHostTest, Resize) {
   EXPECT_EQ(gfx::Size(), host_->in_flight_size_);
   EXPECT_FALSE(process_->sink().GetUniqueMessageMatching(ViewMsg_Resize::ID));
 
-  // Setting the bounds to a "real" rect should send out the notification.
+  // Setting the bounds to a "real" rect should send out the notification,
+  // but should not expect ack for empty physical backing size.
   gfx::Rect original_size(0, 0, 100, 100);
   process_->sink().ClearMessages();
   view_->set_bounds(original_size);
+  view_->SetMockPhysicalBackingSize(gfx::Size());
+  host_->WasResized();
+  EXPECT_FALSE(host_->resize_ack_pending_);
+  EXPECT_EQ(original_size.size(), host_->in_flight_size_);
+  EXPECT_TRUE(process_->sink().GetUniqueMessageMatching(ViewMsg_Resize::ID));
+
+  // Setting the bounds to a "real" rect should send out the notification.
+  process_->sink().ClearMessages();
+  view_->ClearMockPhysicalBackingSize();
   host_->WasResized();
   EXPECT_TRUE(host_->resize_ack_pending_);
   EXPECT_EQ(original_size.size(), host_->in_flight_size_);
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 539e7d5..9864694 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.cc
+++ b/content/browser/renderer_host/render_widget_host_view_android.cc
@@ -335,12 +335,6 @@ gfx::Rect RenderWidgetHostViewAndroid::GetViewBounds() const {
   if (!content_view_core_)
     return gfx::Rect();
 
-  // If the backing hasn't been initialized yet, report empty view bounds
-  // as well. Otherwise, we may end up stuck in a white-screen state because
-  // the resize ack is sent after swapbuffers.
-  if (GetPhysicalBackingSize().IsEmpty())
-    return gfx::Rect();
-
   gfx::Size size = content_view_core_->GetViewportSizeDip();
   gfx::Size offset = content_view_core_->GetViewportSizeOffsetDip();
   size.Enlarge(-offset.width(), -offset.height());
diff --git a/content/public/test/render_widget_test.cc b/content/public/test/render_widget_test.cc
index 8d8e897..9e19bc3 100644
--- a/content/public/test/render_widget_test.cc
+++ b/content/public/test/render_widget_test.cc
@@ -148,4 +148,44 @@ void RenderWidgetTest::OutputBitmapToFile(const SkBitmap& bitmap,
       bitmap_data->size()));
 }
 
+void RenderWidgetTest::TestOnResize() {
+  RenderWidget* widget = static_cast<RenderViewImpl*>(view_);
+
+  // The initial bounds is empty, so setting it to the same thing should do
+  // nothing.
+  widget->OnResize(gfx::Size(), gfx::Size(), 0, gfx::Rect(), false);
+  EXPECT_FALSE(widget->next_paint_is_resize_ack());
+
+  // Setting empty physical backing size should not send the ack.
+  widget->OnResize(gfx::Size(10, 10), gfx::Size(), 0, gfx::Rect(), false);
+  EXPECT_FALSE(widget->next_paint_is_resize_ack());
+
+  // Setting the bounds to a "real" rect should send the ack.
+  render_thread_->sink().ClearMessages();
+  gfx::Size size(100, 100);
+  widget->OnResize(size, size, 0, gfx::Rect(), false);
+  EXPECT_TRUE(widget->next_paint_is_resize_ack());
+  widget->DoDeferredUpdate();
+  ProcessPendingMessages();
+
+  const ViewHostMsg_UpdateRect* msg =
+      static_cast<const ViewHostMsg_UpdateRect*>(
+          render_thread_->sink().GetUniqueMessageMatching(
+              ViewHostMsg_UpdateRect::ID));
+  ASSERT_TRUE(msg);
+  ViewHostMsg_UpdateRect::Schema::Param params;
+  EXPECT_TRUE(ViewHostMsg_UpdateRect::Read(msg, &params));
+  EXPECT_TRUE(ViewHostMsg_UpdateRect_Flags::is_resize_ack(params.a.flags));
+  EXPECT_EQ(size, params.a.view_size);
+  render_thread_->sink().ClearMessages();
+
+  // Setting the same size again should not send the ack.
+  widget->OnResize(size, size, 0, gfx::Rect(), false);
+  EXPECT_FALSE(widget->next_paint_is_resize_ack());
+
+  // Resetting the rect to empty should not send the ack.
+  widget->OnResize(gfx::Size(), gfx::Size(), 0, gfx::Rect(), false);
+  EXPECT_FALSE(widget->next_paint_is_resize_ack());
+}
+
 }  // namespace content
diff --git a/content/public/test/render_widget_test.h b/content/public/test/render_widget_test.h
index 45a5f74..9884c2b 100644
--- a/content/public/test/render_widget_test.h
+++ b/content/public/test/render_widget_test.h
@@ -42,6 +42,9 @@ class RenderWidgetTest : public RenderViewTest {
   // Test for ResizeAndPaint.
   void TestResizeAndPaint();
 
+  // Test for OnResize and Resize.
+  void TestOnResize();
+
   // Helper function which returns true if the given bitmap contains the given
   // ARGB color and false otherwise.
   bool ImageContainsColor(const SkBitmap& bitmap, uint32 argb_color);
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index 948ee9f..29067b9 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -406,12 +406,19 @@ void RenderWidget::Resize(const gfx::Size& new_size,
     // Resize should have caused an invalidation of the entire view.
     DCHECK(new_size.IsEmpty() || is_accelerated_compositing_active_ ||
            paint_aggregator_.HasPendingUpdate());
-  } else if (!RenderThreadImpl::current()->short_circuit_size_updates()) {
+  } else if (!RenderThreadImpl::current() ||  // Will be NULL during unit tests.
+             !RenderThreadImpl::current()->short_circuit_size_updates()) {
+    resize_ack = NO_RESIZE_ACK;
+  }
+
+  if (new_size.IsEmpty() || physical_backing_size.IsEmpty()) {
+    // For empty size or empty physical_backing_size, there is no next paint
+    // (along with which to send the ack) until they are set to non-empty.
     resize_ack = NO_RESIZE_ACK;
   }
 
   // Send the Resize_ACK flag once we paint again if requested.
-  if (resize_ack == SEND_RESIZE_ACK && !new_size.IsEmpty())
+  if (resize_ack == SEND_RESIZE_ACK)
     set_next_paint_is_resize_ack();
 
   if (fullscreen_change)
@@ -419,8 +426,7 @@ void RenderWidget::Resize(const gfx::Size& new_size,
 
   // If a resize ack is requested and it isn't set-up, then no more resizes will
   // come in and in general things will go wrong.
-  DCHECK(resize_ack != SEND_RESIZE_ACK || new_size.IsEmpty() ||
-         next_paint_is_resize_ack());
+  DCHECK(resize_ack != SEND_RESIZE_ACK || next_paint_is_resize_ack());
 }
 
 void RenderWidget::OnClose() {
diff --git a/content/renderer/render_widget_browsertest.cc b/content/renderer/render_widget_browsertest.cc
index 8d70c91..8d72414 100644
--- a/content/renderer/render_widget_browsertest.cc
+++ b/content/renderer/render_widget_browsertest.cc
@@ -10,4 +10,8 @@ TEST_F(RenderWidgetTest, OnMsgPaintAtSize) {
   TestResizeAndPaint();
 }
 
+TEST_F(RenderWidgetTest, OnResize) {
+  TestOnResize();
+}
+
 }  // namespace content