Unit Test for WebView animating in and out of screen

android_webview/browser/browser_view_renderer_unittest.cc

Index: android_webview/browser/browser_view_renderer_unittest.cc
diff --git a/android_webview/browser/browser_view_renderer_unittest.cc b/android_webview/browser/browser_view_renderer_unittest.cc
index b4a9d42227241cef82d61d71a47608eb57d52001..6e9483a5676453e2fd70f5e45d7c8fd7c662186f 100644
--- a/android_webview/browser/browser_view_renderer_unittest.cc
+++ b/android_webview/browser/browser_view_renderer_unittest.cc
@@ -59,4 +59,30 @@ class ClearViewTest : public RenderingTest {
 
 RENDERING_TEST_F(ClearViewTest);
 
+class TestForceInvalidateWithEmptyGlobalVisibleRect : public RenderingTest {

[boliu, 2015/03/12 22:54:01]

Does this actually fail without the fix?

DrawGL still happens with the contraints from the default FakeWindow values,
which I assume is different than what's initialized in BVR, which means BVR will
invalidate either way?

I think this test needs to be much more verbose, and actually change the
WillDrawOnRT to return a bool, and if false, then skip DrawGL(kModeDraw)

Then simulate the whole sequence, draw normally, and wait for the parent
constraints to arrive in BVR. Then move it offscreen (ie empty visible rect) and
skip DrawGL. Then move it back to same place, check parents contraints still
match, but we invalidate anyway.

[hush (inactive), 2015/03/12 23:12:00]

Yes, it fails without the fix. SharedRendererState won't post the constraints to
BVR because of this if-statement returned false:
https://code.google.com/p/chromium/codesearch#chromium/src/android_webview/br...


This test itself is still meaningful for asserting we forced the parent draw
constraints through.
I can add another test for the whole scene, something like
TestAnimatingInAndOutOfScreen

[boliu, 2015/03/12 23:16:39]

Without this fix, I don't see how that returns false. Shouldn't the
ParentConstraints copy in SRS need to be updated at least once? Which means at
least another invalidate, which means at least 2 draws?
But, but... you are not actually checking any of that stuff. If you need to
expose a test only class that returns the ParentConstraints from BVR, I support
that.

[hush (inactive), 2015/03/12 23:44:28]

Looks like you've found another bug in HardwareRenderer... The first time you
call DrawGL, the viewport_ will still be an empty 0x0 rect. Then it is posted to
SRS. I think we should've set the viewport before we post it the SRS.

However, even with that problem in hardware renderer fixed, SRS still won't post
the draw constraints to the child compositor because the surface rect does not
matter if is_layer == false.
Since both the old and new constraints is_layer == false, the only thing matters
is the transform. And it is identity matrix.

Anyways, the point is unless you set needs_force_invalidate_on_next_draw_gl, or
assign a non-identity transform matrix in fake_window, SRS won't post the draw
constraints to the child compositor. I agree this is a problem on the test
setup...

[boliu, 2015/03/12 23:56:37]

It's surprising what you find when you actually write tests...

You can fix that one separately too :p
Oh I see. Ok, as long as you verify those things in the test, then I'm fine.

+ public:
+  TestForceInvalidateWithEmptyGlobalVisibleRect() : on_draw_count_(0u) {}
+
+  void StartTest() override {
+    browser_view_renderer_->SetContinuousInvalidate(true);
+  }
+
+  void WillOnDraw() override {
+    on_draw_count_++;
+
+    // The second onDraw will be the result of a forced invalidation because
+    // an empty global visible rect was used in the first onDraw.
+    if (on_draw_count_ == 2u)
+      EndTest();
+
+    browser_view_renderer_->PrepareToDraw(gfx::Vector2d(), gfx::Rect());
+    browser_view_renderer_->SetContinuousInvalidate(false);
+  }
+
+ private:
+  size_t on_draw_count_;
+};
+
+RENDERING_TEST_F(TestForceInvalidateWithEmptyGlobalVisibleRect);
+
 }  // namespace android_webview