Refactor GestureNavigation to eliminate code redundancy

content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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/test/mock_render_process_host.h"
 #include "content/test/test_render_frame_host.h"
 #include "content/test/test_render_view_host.h"
 #include "content/test/test_web_contents.h"
 #include "ui/aura/test/test_windows.h"
 #include "ui/aura/window.h"
 #include "ui/aura_extra/image_window_delegate.h"
 #include "ui/gfx/codec/png_codec.h"
 
 namespace content {
 
 class OverscrollNavigationOverlayTest : public RenderViewHostImplTestHarness {
  public:
-  OverscrollNavigationOverlayTest() {}
+  OverscrollNavigationOverlayTest()
+      : first_("https://www.google.com"),
+        second_("http://www.chromium.org"),
+        third_("https://www.kernel.org/") {}
+
   ~OverscrollNavigationOverlayTest() override {}
 
   gfx::Image CreateDummyScreenshot() {
     SkBitmap bitmap;
     bitmap.allocN32Pixels(1, 1);
     bitmap.eraseColor(SK_ColorWHITE);
     return gfx::Image::CreateFrom1xBitmap(bitmap);
   }
 
   void SetDummyScreenshotOnNavEntry(NavigationEntry* entry) {
     SkBitmap bitmap;
     bitmap.allocN32Pixels(1, 1);
     bitmap.eraseColor(SK_ColorWHITE);
     std::vector<unsigned char> png_data;
     gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, true, &png_data);
     scoped_refptr<base::RefCountedBytes> png_bytes =
         base::RefCountedBytes::TakeVector(&png_data);
     NavigationEntryImpl* entry_impl =
         NavigationEntryImpl::FromNavigationEntry(entry);
     entry_impl->SetScreenshotPNGData(png_bytes);
   }
 
   void ReceivePaintUpdate() {
     FrameHostMsg_DidFirstVisuallyNonEmptyPaint msg(
         main_test_rfh()->GetRoutingID());
     RenderViewHostTester::TestOnMessageReceived(test_rvh(), msg);
   }
 
   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();
+    GetOverlay()->OnOverscrollCompleted(window.Pass());
   }
 
+  // Tests URLs.
+  const GURL first_;
+  const GURL second_;
+  const GURL third_;
+
  protected:
   // RenderViewHostImplTestHarness:
   void SetUp() override {
     RenderViewHostImplTestHarness::SetUp();
 
-    const GURL first("https://www.google.com");
-    contents()->NavigateAndCommit(first);
+    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());
+    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;
     memset(&params, 0, sizeof(params));
     params.view_size = gfx::Size(10, 10);
     ViewHostMsg_UpdateRect rect(test_rvh()->GetRoutingID(), params);
     RenderViewHostTester::TestOnMessageReceived(test_rvh(), rect);
 
     // Reset pending flags for size/paint.
     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(), contents()->GetNativeView()));
   }
 
   void TearDown() override {
     overlay_.reset();
     RenderViewHostImplTestHarness::TearDown();
   }
 
   OverscrollNavigationOverlay* GetOverlay() {
     return overlay_.get();
   }
 
  private:
   scoped_ptr<OverscrollNavigationOverlay> overlay_;
 
   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());
-
-  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 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());
 }
 
-TEST_F(OverscrollNavigationOverlayTest, LoadUpdateWithoutNonEmptyPaint) {
-  GetOverlay()->image_delegate_->SetImage(CreateDummyScreenshot());
-  process()->sink().ClearMessages();
-
-  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());
+// 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, MultiNavigation_PaintUpdate) {
-  GetOverlay()->image_delegate_->SetImage(CreateDummyScreenshot());
-  SetDummyScreenshotOnNavEntry(controller().GetEntryAtOffset(-1));
+// 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_NE(GetOverlay()->direction_, OverscrollNavigationOverlay::NONE);

[mfomitchev, 2015/03/26 15:36:00]

Why don't we test that the direction is BACK instead of testing it's not NONE?
Applies to other tests as well.

[Nina, 2015/03/27 17:52:35]

Done.

 
+  GetOverlay()->OnOverscrollAborted();
+  // Make sure that if we started a navigation (we shouldn't), we commit it.
+  EXPECT_FALSE(contents()->cross_navigation_pending());

[mfomitchev, 2015/03/26 15:36:00]

Also confirm direction is NONE?

[Nina, 2015/03/27 17:52:35]

Done.

+}
+
+// Tests that if a second navigation is aborted before, the first one still

[mfomitchev, 2015/03/26 15:36:00]

"a second navigation is aborted before" is not clear

[Nina, 2015/03/27 17:52:35]

Done.

+// happens.
+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_NE(GetOverlay()->direction_, OverscrollNavigationOverlay::NONE);
 
+  GetOverlay()->OnOverscrollAborted();
+  EXPECT_EQ(GetOverlay()->direction_, OverscrollNavigationOverlay::NONE);
+
+  EXPECT_TRUE(contents()->cross_navigation_pending());
+  contents()->CommitPendingNavigation();
+  EXPECT_EQ(contents()->GetURL(), second_);
+}
+
+// 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(), second_);
 }
 
-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
+  // (which can happen if a new navigation is performed while while a GestureNav
+  // navigation is in progress).
+  contents()->TestSetIsLoading(true);
+  contents()->TestSetIsLoading(false);
+  EXPECT_FALSE(GetOverlay()->web_contents());
+  contents()->CommitPendingNavigation();
+  EXPECT_EQ(contents()->GetURL(), second_);
+}
+
+// Tests that an overscroll gesture followed after another one before the first
+// one finishes loading works with a paint update.
+TEST_F(OverscrollNavigationOverlayTest, MultiNavigation_PaintUpdate) {
+  PerformBackNavigationViaSliderCallbacks();

[mfomitchev, 2015/03/26 15:36:00]

Do you think we still need the multinavigation tests? It was a different flow
for the second navigation before since the slider callbacks were only used for
the second navigation. Now it's the same callbacks, so I am not sure these tests
are still useful.

[Nina, 2015/03/27 17:52:35]

Yeah... it does not make that much sense to have these. The codepath is the
same. Removed.

+  EXPECT_TRUE(GetOverlay()->web_contents());
+  PerformBackNavigationViaSliderCallbacks();
+  EXPECT_TRUE(GetOverlay()->web_contents());
+  ReceivePaintUpdate();
+
+  // Paint updates until the navigation is committed typically represent updates
+  // 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 - we should no
+  // longer be observing.
+  EXPECT_FALSE(GetOverlay()->web_contents());
+  EXPECT_EQ(contents()->GetURL(), first_);
+}
+
+// Tests that an overscroll gesture followed after another one before the first
+// one finishes loading works with a loading update.
+TEST_F(OverscrollNavigationOverlayTest, MultiNavigation_LoadingUpdate) {
+  PerformBackNavigationViaSliderCallbacks();
+  EXPECT_TRUE(GetOverlay()->web_contents());
+  PerformBackNavigationViaSliderCallbacks();
+  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
   // (which can happen if a new navigation is performed while while a GestureNav
   // 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(), first_);
 }
 
 }  // namespace content