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..1a16a369ee5302672cce15fa7b952edaf558f17d 100644
--- a/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
+++ b/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
@@ -4,6 +4,7 @@
 
 #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"
@@ -15,22 +16,68 @@
 #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/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 +98,60 @@ 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();
+    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()),
+        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 +164,8 @@ 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 +183,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 aborted, no navigation is performed and the
+// state is restored.
+TEST_F(OverscrollNavigationOverlayTest, AbortNavigation) {
+  scoped_ptr<aura::Window> window = GetOverlay()->CreateBackWindow();
+  EXPECT_EQ(GetOverlay()->direction_, OverscrollNavigationOverlay::BACK);

[mfomitchev, 2015/03/31 22:16:31]

Might actually want to add this check to
PerformBackNavigationViaSliderCallbacks() as well

[Nina, 2015/04/01 21:49:05]

Done.

 
-  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()->OnOverscrollAborted();
+  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 aborted, tests that the
+// first one worked correctly.
+TEST_F(OverscrollNavigationOverlayTest, AbortAfterSuccessfulNavigation) {
   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()->OnOverscrollAborted();
+  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 +265,67 @@ 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);
+
+  // Start and complete a back navigation via a gesture.
+  ui::test::EventGenerator generator(root_window());
+  generator.GestureScrollSequence(
+      gfx::Point(10, 10),
+      gfx::Point(400, 10),
+      base::TimeDelta::FromMilliseconds(10),
+      10);
+
+  ui::ScopedLayerAnimationSettings settings(animator);
+  base::TimeDelta duration = settings.GetTransitionDuration();
+  test_controller.StartThreadedAnimationsIfNeeded();
+  base::TimeTicks start_time = gfx::FrameTime::Now();
+
+  // The overlay window should now be being animated to the edge of the screen.
+  animator->Step(start_time + duration / 8);
+  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 turned into the new overlay window

[mfomitchev, 2015/03/31 22:16:31]

which will be used as the overlay window when the new overscroll starts

[Nina, 2015/04/01 21:49:05]

Done.

+  // afterwards.
+  generator.GestureScrollSequence(
+      gfx::Point(550, 10),

[mfomitchev, 2015/03/31 22:16:31]

Avoid using "magic numbers" like this in tests. This applies to the previous
gesture as well, where we should probably use something like 10+threshold+1.

For this case I think it is a bigger problem - looks like you are somehow
relying on window being a certain width, and it's not clear immediately where is
that even declared. Best to use the width and the threshold here as well.

BTW, we should probably also DCHECK that threshold+1 is going to leave us within
the bounds.

[Nina, 2015/04/01 21:49:05]

Done.

+      gfx::Point(800, 10),
+      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