Refactor GestureNavigation to eliminate code redundancy

content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc

Index: content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
diff --git a/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc b/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
index bd9ab089c1c2f788b0bd666b99a807d112fd6dd5..61cec017a2513934ea27cba96d4e39c6d6dbff15 100644
--- a/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
+++ b/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
@@ -4,10 +4,12 @@
 
 #include "content/browser/web_contents/aura/overscroll_navigation_overlay.h"
 
+#include <vector>
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/web_contents/web_contents_view.h"
 #include "content/common/frame_messages.h"
 #include "content/common/view_messages.h"
+#include "content/public/browser/overscroll_configuration.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/test/test_render_frame_host.h"
 #include "content/test/test_render_view_host.h"
@@ -15,22 +17,63 @@
 #include "ui/aura/test/test_windows.h"
 #include "ui/aura/window.h"
 #include "ui/aura_extra/image_window_delegate.h"
+#include "ui/compositor/scoped_animation_duration_scale_mode.h"
+#include "ui/compositor/scoped_layer_animation_settings.h"
+#include "ui/compositor/test/layer_animator_test_controller.h"
+#include "ui/events/gesture_detection/gesture_configuration.h"
+#include "ui/events/test/event_generator.h"
 #include "ui/gfx/codec/png_codec.h"
+#include "ui/gfx/frame_time.h"
 
 namespace content {
 
-class OverscrollNavigationOverlayTest : public RenderViewHostImplTestHarness {
+// A subclass of TestWebContents that offers a fake content window.
+class OverscrollTestWebContents : public TestWebContents {
  public:
-  OverscrollNavigationOverlayTest() {}
-  ~OverscrollNavigationOverlayTest() override {}
+  ~OverscrollTestWebContents() override {}
+
+  static OverscrollTestWebContents* Create(
+      BrowserContext* browser_context,
+      SiteInstance* instance,
+      scoped_ptr<aura::Window> fake_native_view,
+      scoped_ptr<aura::Window> fake_contents_window) {
+    OverscrollTestWebContents* web_contents = new OverscrollTestWebContents(
+        browser_context, fake_native_view.Pass(), fake_contents_window.Pass());
+    web_contents->Init(WebContents::CreateParams(browser_context, instance));
+    web_contents->RenderFrameCreated(web_contents->GetMainFrame());
+    return web_contents;
+  }
 
-  gfx::Image CreateDummyScreenshot() {
-    SkBitmap bitmap;
-    bitmap.allocN32Pixels(1, 1);
-    bitmap.eraseColor(SK_ColorWHITE);
-    return gfx::Image::CreateFrom1xBitmap(bitmap);
+  gfx::NativeView GetNativeView() override { return fake_native_view_.get(); }
+
+  gfx::NativeView GetContentNativeView() override {
+    return fake_contents_window_.get();
   }
 
+ protected:
+  explicit OverscrollTestWebContents(
+      BrowserContext* browser_context,
+      scoped_ptr<aura::Window> fake_native_view,
+      scoped_ptr<aura::Window> fake_contents_window)
+      : TestWebContents(browser_context),
+        fake_native_view_(fake_native_view.Pass()),
+        fake_contents_window_(fake_contents_window.Pass()) {}
+
+ private:
+  scoped_ptr<aura::Window> fake_native_view_;
+  scoped_ptr<aura::Window> fake_contents_window_;
+};
+
+class OverscrollNavigationOverlayTest : public RenderViewHostImplTestHarness {
+ public:
+  OverscrollNavigationOverlayTest()
+      : first_("https://www.google.com"),
+        second_("http://www.chromium.org"),
+        third_("https://www.kernel.org/"),
+        fourth_("https://github.com/") {}
+
+  ~OverscrollNavigationOverlayTest() override {}
+
   void SetDummyScreenshotOnNavEntry(NavigationEntry* entry) {
     SkBitmap bitmap;
     bitmap.allocN32Pixels(1, 1);
@@ -51,29 +94,62 @@ class OverscrollNavigationOverlayTest : public RenderViewHostImplTestHarness {
   }
 
   void PerformBackNavigationViaSliderCallbacks() {
-    // Sets slide direction to SLIDE_BACK, sets screenshot from NavEntry at
+    // Sets slide direction to BACK, sets screenshot from NavEntry at
     // offset -1  on layer_delegate_.
-    delete GetOverlay()->CreateBackLayer();
+    scoped_ptr<aura::Window> window(GetOverlay()->CreateBackWindow());
     // Performs BACK navigation, sets image from layer_delegate_ on
     // image_delegate_.
-    GetOverlay()->OnWindowSlideCompleting();
-    GetOverlay()->OnWindowSlideCompleted(scoped_ptr<ui::Layer>());
+    GetOverlay()->OnOverscrollCompleting();
+    if (window) {

[mfomitchev, 2015/04/02 22:50:37]

Can we add checks for !window case as well?

[Nina, 2015/04/07 14:02:08]

Done.

+      EXPECT_TRUE(contents()->cross_navigation_pending());
+      EXPECT_EQ(GetOverlay()->direction_, OverscrollNavigationOverlay::BACK);
+    }
+    window->SetBounds(gfx::Rect(root_window()->bounds().size()));
+    GetOverlay()->OnOverscrollCompleted(window.Pass());
   }
 
+  // Tests URLs.
+  const GURL first_;
+  const GURL second_;
+  const GURL third_;
+  const GURL fourth_;
+
  protected:
   // RenderViewHostImplTestHarness:
   void SetUp() override {
     RenderViewHostImplTestHarness::SetUp();
 
-    const GURL first("https://www.google.com");
-    contents()->NavigateAndCommit(first);
+    // Set up the fake web contents native view.
+    scoped_ptr<aura::Window> fake_native_view(new aura::Window(nullptr));
+    fake_native_view->Init(aura::WINDOW_LAYER_SOLID_COLOR);
+    root_window()->AddChild(fake_native_view.get());
+    fake_native_view->SetBounds(gfx::Rect(root_window()->bounds().size()));
+
+    // Set up the fake contents window.
+    scoped_ptr<aura::Window> fake_contents_window(new aura::Window(nullptr));
+    fake_contents_window->Init(aura::WINDOW_LAYER_SOLID_COLOR);
+    root_window()->AddChild(fake_contents_window.get());
+    fake_contents_window->SetBounds(gfx::Rect(root_window()->bounds().size()));
+
+    // Replace the default test web contents with our custom class.
+    SetContents(OverscrollTestWebContents::Create(
+        browser_context(), SiteInstance::Create(browser_context()),

[mfomitchev, 2015/04/02 22:50:37]

Nit: I am surprised git cl format did this. I thought the chromium style was all
params on one line or one param per line. The old way was certainly more
readable.

[Nina, 2015/04/07 14:02:08]

Done.

+        fake_native_view.Pass(), fake_contents_window.Pass()));
+
+    contents()->NavigateAndCommit(first_);
     EXPECT_TRUE(controller().GetVisibleEntry());
     EXPECT_FALSE(controller().CanGoBack());
 
-    const GURL second("http://www.chromium.org");
-    contents()->NavigateAndCommit(second);
+    contents()->NavigateAndCommit(second_);
     EXPECT_TRUE(controller().CanGoBack());
 
+    contents()->NavigateAndCommit(third_);
+    EXPECT_TRUE(controller().CanGoBack());
+
+    contents()->NavigateAndCommit(fourth_);
+    EXPECT_TRUE(controller().CanGoBack());
+    EXPECT_FALSE(controller().CanGoForward());
+
     // Receive a paint update. This is necessary to make sure the size is set
     // correctly in RenderWidgetHostImpl.
     ViewHostMsg_UpdateRect_Params params;
@@ -86,22 +162,7 @@ class OverscrollNavigationOverlayTest : public RenderViewHostImplTestHarness {
     test_rvh()->ResetSizeAndRepaintPendingFlags();
 
     // Create the overlay, and set the contents of the overlay window.
-    overlay_.reset(new OverscrollNavigationOverlay(contents()));
-    aura_extra::ImageWindowDelegate* image_delegate =
-        new aura_extra::ImageWindowDelegate();
-    scoped_ptr<aura::Window> overlay_window(
-      aura::test::CreateTestWindowWithDelegate(
-          image_delegate,
-          0,
-          gfx::Rect(root_window()->bounds().size()),
-          root_window()));
-
-    overlay_->SetOverlayWindow(overlay_window.Pass(), image_delegate);
-    overlay_->StartObserving();
-
-    EXPECT_TRUE(overlay_->web_contents());
-    EXPECT_FALSE(overlay_->loading_complete_);
-    EXPECT_FALSE(overlay_->received_paint_update_);
+    overlay_.reset(new OverscrollNavigationOverlay(contents(), root_window()));
   }
 
   void TearDown() override {
@@ -119,67 +180,81 @@ class OverscrollNavigationOverlayTest : public RenderViewHostImplTestHarness {
   DISALLOW_COPY_AND_ASSIGN(OverscrollNavigationOverlayTest);
 };
 
-TEST_F(OverscrollNavigationOverlayTest, FirstVisuallyNonEmptyPaint_NoImage) {
-  ReceivePaintUpdate();
-  EXPECT_TRUE(GetOverlay()->received_paint_update_);
-  EXPECT_FALSE(GetOverlay()->loading_complete_);
-  // The paint update will hide the overlay.
-  EXPECT_FALSE(GetOverlay()->web_contents());
+// Tests that if a screenshot is available, it is set in the overlay window
+// delegate.
+TEST_F(OverscrollNavigationOverlayTest, WithScreenshot) {
+  SetDummyScreenshotOnNavEntry(controller().GetEntryAtOffset(-1));
+  PerformBackNavigationViaSliderCallbacks();
+  // Screenshot was set on NavEntry at offset -1.
+  EXPECT_TRUE(static_cast<aura_extra::ImageWindowDelegate*>(
+                  GetOverlay()->window_->delegate())->has_image());
 }
 
-TEST_F(OverscrollNavigationOverlayTest, FirstVisuallyNonEmptyPaint_WithImage) {
-  GetOverlay()->image_delegate_->SetImage(CreateDummyScreenshot());
+// Tests that if a screenshot is not available, no image is set in the overlay
+// window delegate.
+TEST_F(OverscrollNavigationOverlayTest, WithoutScreenshot) {
+  PerformBackNavigationViaSliderCallbacks();
+  // No screenshot was set on NavEntry at offset -1.
+  EXPECT_FALSE(static_cast<aura_extra::ImageWindowDelegate*>(
+                   GetOverlay()->window_->delegate())->has_image());
+}
 
-  ReceivePaintUpdate();
-  EXPECT_TRUE(GetOverlay()->received_paint_update_);
-  EXPECT_FALSE(GetOverlay()->loading_complete_);
-  // The paint update will hide the overlay.
-  EXPECT_FALSE(GetOverlay()->web_contents());
+// Tests that if a navigation is attempted but there is nothing to navigate to,
+// we return a null window.
+TEST_F(OverscrollNavigationOverlayTest, CannotNavigate) {
+  EXPECT_EQ(GetOverlay()->CreateFrontWindow(), nullptr);
 }
 
-TEST_F(OverscrollNavigationOverlayTest, LoadUpdateWithoutNonEmptyPaint) {
-  GetOverlay()->image_delegate_->SetImage(CreateDummyScreenshot());
-  process()->sink().ClearMessages();
+// Tests that if a navigation is cancelled, no navigation is performed and the
+// state is restored.
+TEST_F(OverscrollNavigationOverlayTest, CancelNavigation) {
+  scoped_ptr<aura::Window> window = GetOverlay()->CreateBackWindow();
+  EXPECT_EQ(GetOverlay()->direction_, OverscrollNavigationOverlay::BACK);
 
-  contents()->TestSetIsLoading(false);
-  EXPECT_TRUE(GetOverlay()->loading_complete_);
-  EXPECT_FALSE(GetOverlay()->received_paint_update_);
-  // The page load should hide the overlay.
-  EXPECT_FALSE(GetOverlay()->web_contents());
+  GetOverlay()->OnOverscrollCancelled();
+  EXPECT_FALSE(contents()->cross_navigation_pending());
+  EXPECT_EQ(GetOverlay()->direction_, OverscrollNavigationOverlay::NONE);
 }
 
-TEST_F(OverscrollNavigationOverlayTest, MultiNavigation_PaintUpdate) {
-  GetOverlay()->image_delegate_->SetImage(CreateDummyScreenshot());
-  SetDummyScreenshotOnNavEntry(controller().GetEntryAtOffset(-1));
-
+// Performs two navigations. The second navigation is cancelled, tests that the
+// first one worked correctly.
+TEST_F(OverscrollNavigationOverlayTest, CancelAfterSuccessfulNavigation) {
   PerformBackNavigationViaSliderCallbacks();
-  // Screenshot was set on NavEntry at offset -1.
-  EXPECT_TRUE(GetOverlay()->image_delegate_->has_image());
-  EXPECT_FALSE(GetOverlay()->received_paint_update_);
+  scoped_ptr<aura::Window> wrapper = GetOverlay()->CreateBackWindow();
+  EXPECT_EQ(GetOverlay()->direction_, OverscrollNavigationOverlay::BACK);
+
+  GetOverlay()->OnOverscrollCancelled();
+  EXPECT_EQ(GetOverlay()->direction_, OverscrollNavigationOverlay::NONE);
+
+  EXPECT_TRUE(contents()->cross_navigation_pending());
+  contents()->CommitPendingNavigation();
+  EXPECT_EQ(contents()->GetURL(), third_);
+}
 
+// Tests that an overscroll navigation that receives a paint update actually
+// stops observing.
+TEST_F(OverscrollNavigationOverlayTest, Navigation_PaintUpdate) {
+  PerformBackNavigationViaSliderCallbacks();
   ReceivePaintUpdate();
+
   // Paint updates until the navigation is committed typically represent updates
-  // for the previous page, so they shouldn't affect the flag.
-  EXPECT_FALSE(GetOverlay()->received_paint_update_);
+  // for the previous page, so we should still be observing.
+  EXPECT_TRUE(GetOverlay()->web_contents());
 
   contents()->CommitPendingNavigation();
   ReceivePaintUpdate();
-  // Navigation was committed and the paint update was received - the flag
-  // should now be updated.
-  EXPECT_TRUE(GetOverlay()->received_paint_update_);
 
+  // Navigation was committed and the paint update was received - we should no
+  // longer be observing.
   EXPECT_FALSE(GetOverlay()->web_contents());
+  EXPECT_EQ(contents()->GetURL(), third_);
 }
 
-TEST_F(OverscrollNavigationOverlayTest, MultiNavigation_LoadingUpdate) {
-  GetOverlay()->image_delegate_->SetImage(CreateDummyScreenshot());
-
+// Tests that an overscroll navigation that receives a loading update actually
+// stops observing.
+TEST_F(OverscrollNavigationOverlayTest, Navigation_LoadingUpdate) {
   PerformBackNavigationViaSliderCallbacks();
-  // No screenshot was set on NavEntry at offset -1.
-  EXPECT_FALSE(GetOverlay()->image_delegate_->has_image());
-  // Navigation was started, so the loading status flag should be reset.
-  EXPECT_FALSE(GetOverlay()->loading_complete_);
-
+  EXPECT_TRUE(GetOverlay()->web_contents());
   // DidStopLoading for any navigation should always reset the load flag and
   // dismiss the overlay even if the pending navigation wasn't committed -
   // this is a "safety net" in case we mis-identify the destination webpage
@@ -187,9 +262,70 @@ TEST_F(OverscrollNavigationOverlayTest, MultiNavigation_LoadingUpdate) {
   // navigation is in progress).
   contents()->TestSetIsLoading(true);
   contents()->TestSetIsLoading(false);
-  EXPECT_TRUE(GetOverlay()->loading_complete_);
-
   EXPECT_FALSE(GetOverlay()->web_contents());
+  contents()->CommitPendingNavigation();
+  EXPECT_EQ(contents()->GetURL(), third_);
+}
+
+// Tests that swapping the overlay window at the end of a gesture caused by the
+// start of a new overscroll does not crash and the events still reach the new
+// overlay window.
+TEST_F(OverscrollNavigationOverlayTest, OverlayWindowSwap) {
+  PerformBackNavigationViaSliderCallbacks();
+  aura::Window* first_overlay_window = GetOverlay()->window_.get();
+  EXPECT_TRUE(GetOverlay()->web_contents());
+  EXPECT_TRUE(first_overlay_window);
+
+  // At this stage, the overlay window is covering the web contents. Configure
+  // the animator of the overlay window for the test.
+  ui::ScopedAnimationDurationScaleMode normal_duration(
+      ui::ScopedAnimationDurationScaleMode::NORMAL_DURATION);
+  ui::LayerAnimator* animator = GetOverlay()->window_->layer()->GetAnimator();
+  animator->set_disable_timer_for_test(true);
+  ui::LayerAnimatorTestController test_controller(animator);
+
+  int overscroll_complete_distance =
+      root_window()->bounds().size().width() *
+          content::GetOverscrollConfig(
+              content::OVERSCROLL_CONFIG_HORIZ_THRESHOLD_COMPLETE) +
+      ui::GestureConfiguration::GetInstance()
+          ->max_touch_move_in_pixels_for_click() + 1;
+
+  // Start and complete a back navigation via a gesture.
+  ui::test::EventGenerator generator(root_window());
+  generator.GestureScrollSequence(gfx::Point(0, 0),
+                                  gfx::Point(overscroll_complete_distance, 0),
+                                  base::TimeDelta::FromMilliseconds(10), 10);

[mfomitchev, 2015/04/02 22:50:37]

nit: last 10 on a separate line

[Nina, 2015/04/07 14:02:08]

Done.

+
+  ui::ScopedLayerAnimationSettings settings(animator);
+  test_controller.StartThreadedAnimationsIfNeeded();
+
+  // The overlay window should now be being animated to the edge of the screen.

[mfomitchev, 2015/04/02 22:50:37]

We've removed animator->Step(), so there will be no movement in the animation?
But we do want the window to move a bit, don't we?

[Nina, 2015/04/07 14:02:08]

TBH it really is not necessary and only complicates the math for the next
gesture a little. Putting the animation "on hold" is enough.

+  EXPECT_EQ(GetOverlay()->window_.get(), first_overlay_window);
+
+  // The overlay window is halfway through, start another animation that will
+  // cancel the first one. The event that cancels the animation will go to
+  // the slide window, which will be used as the overlay window when the new
+  // overscroll starts.
+  int second_overscroll_start_distance = overscroll_complete_distance + 1;

[mfomitchev, 2015/04/02 22:50:37]

A comment explaining why we chose such a value for
second_overscroll_start_distance would be nice. It's pretty obscure..

[Nina, 2015/04/07 14:02:08]

If it was hard to write, it must be hard to read!
Just kidding, added some ascii art for it.

+  generator.GestureScrollSequence(
+      gfx::Point(second_overscroll_start_distance, 0),
+      gfx::Point(
+          second_overscroll_start_distance + overscroll_complete_distance, 0),
+      base::TimeDelta::FromMilliseconds(10), 10);
+  EXPECT_TRUE(GetOverlay()->window_.get());
+  // The overlay window should be a new window.
+  EXPECT_NE(GetOverlay()->window_.get(), first_overlay_window);
+
+  // Complete the animation.
+  GetOverlay()->window_->layer()->GetAnimator()->StopAnimating();
+  EXPECT_TRUE(GetOverlay()->window_.get());
+
+  // Load the page.
+  contents()->CommitPendingNavigation();
+  ReceivePaintUpdate();
+  EXPECT_FALSE(GetOverlay()->window_.get());
+  EXPECT_EQ(contents()->GetURL(), first_);
 }
 
 }  // namespace content