Refactor GestureNavigation to eliminate code redundancy

content/browser/web_contents/aura/overscroll_navigation_overlay.cc

Index: content/browser/web_contents/aura/overscroll_navigation_overlay.cc
diff --git a/content/browser/web_contents/aura/overscroll_navigation_overlay.cc b/content/browser/web_contents/aura/overscroll_navigation_overlay.cc
index 89aa4975f81a12b870ddd544c3dff67a96bc2561..39292ac10bb1d45473d2bf3991b12a07cad4550f 100644
--- a/content/browser/web_contents/aura/overscroll_navigation_overlay.cc
+++ b/content/browser/web_contents/aura/overscroll_navigation_overlay.cc
@@ -18,7 +18,6 @@
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/image/image_png_rep.h"
-#include "ui/gfx/image/image_skia.h"
 
 namespace content {
 namespace {
@@ -130,26 +129,29 @@ class OverlayDismissAnimator
 };
 
 OverscrollNavigationOverlay::OverscrollNavigationOverlay(
-    WebContentsImpl* web_contents)
+    WebContentsImpl* web_contents,
+    WebContentsViewAura* wcva)
     : web_contents_(web_contents),
       image_delegate_(NULL),
       loading_complete_(false),
       received_paint_update_(false),
-      slide_direction_(SLIDE_UNKNOWN) {
+      slide_direction_(SLIDE_UNKNOWN),
+      wcva_(wcva) {
 }
 
 OverscrollNavigationOverlay::~OverscrollNavigationOverlay() {
 }
 
 void OverscrollNavigationOverlay::StartObserving() {
-  loading_complete_ = false;
+  LOG(ERROR) << "Starting to observe";
+  /*loading_complete_ = false;
   received_paint_update_ = false;
-  overlay_dismiss_layer_.reset();
+  overlay_dismiss_layer_.reset();*/
   Observe(web_contents_);
 
-  // Make sure the overlay window is on top.
+  /*// Make sure the overlay window is on top.
   if (window_.get() && window_->parent())
-    window_->parent()->StackChildAtTop(window_.get());
+    window_->parent()->StackChildAtTop(window_.get());*/
 
   // Assumes the navigation has been initiated.
   NavigationEntry* pending_entry =
@@ -158,11 +160,13 @@ void OverscrollNavigationOverlay::StartObserving() {
   // Under some circumstances navigation can leave a null pending entry -
   // see comments in NavigationControllerImpl::NavigateToPendingEntry().
   pending_entry_url_ = pending_entry ? pending_entry->GetURL() : GURL();
+  LOG(ERROR) << "URL: " << pending_entry_url_.GetContent();
 }
 
 void OverscrollNavigationOverlay::SetOverlayWindow(
     scoped_ptr<aura::Window> window,
     aura_extra::ImageWindowDelegate* delegate) {
+  LOG(ERROR) << "Setting overlay window";
   window_ = window.Pass();
   if (window_.get() && window_->parent())
     window_->parent()->StackChildAtTop(window_.get());
@@ -178,7 +182,50 @@ void OverscrollNavigationOverlay::SetOverlayWindow(
   }
 }
 
+const gfx::Image OverscrollNavigationOverlay::GetImageForDirection(
+    Direction direction) {
+  const NavigationControllerImpl& controller = web_contents_->GetController();
+  const NavigationEntryImpl* entry = NavigationEntryImpl::FromNavigationEntry(
+      controller.GetEntryAtOffset(direction == FORWARD ? 1 : -1));
+
+  if (entry && entry->screenshot().get()) {
+    std::vector<gfx::ImagePNGRep> image_reps;
+    image_reps.push_back(gfx::ImagePNGRep(entry->screenshot(), 1.0f));
+    return gfx::Image(image_reps);
+  }
+  return gfx::Image();
+}
+
+scoped_ptr<ui::Layer> OverscrollNavigationOverlay::CreateLayerForDirection(
+    Direction direction) {
+  // TODO figure out if we want to keep only one layer delegate (or only one
+  // layer at all).
+  if (!layer_delegate_)
+    layer_delegate_.reset(new ImageLayerDelegate());
+  scoped_ptr<ui::Layer> layer(new ui::Layer(ui::LAYER_TEXTURED));
+  layer_delegate_->SetImage(GetImageForDirection(direction));
+  layer->set_delegate(layer_delegate_.get());
+  return layer.Pass();
+}
+
+OverscrollNavigationOverlay::Direction
+OverscrollNavigationOverlay::GetNavigationDirection(
+    const NavigationController& controller,
+    OverscrollMode mode) {
+  LOG(ERROR) << "Getting navigation direction";
+  if (mode == (base::i18n::IsRTL() ? OVERSCROLL_EAST : OVERSCROLL_WEST) &&
+      controller.CanGoForward()) {
+    return FORWARD;
+  }
+  if (mode == (base::i18n::IsRTL() ? OVERSCROLL_WEST : OVERSCROLL_EAST) &&
+      controller.CanGoBack()) {
+    return BACKWARD;
+  }
+  return NONE;
+}
+
 void OverscrollNavigationOverlay::StopObservingIfDone() {
+  LOG(ERROR) << "Stop observing if done";
   // Normally we dismiss the overlay once we receive a paint update, however
   // for in-page navigations DidFirstVisuallyNonEmptyPaint() does not get
   // called, and we rely on loading_complete_ for those cases.
@@ -186,16 +233,20 @@ void OverscrollNavigationOverlay::StopObservingIfDone() {
     return;
 
   // If a slide is in progress, then do not destroy the window or the slide.
-  if (window_slider_.get() && window_slider_->IsSlideInProgress())
-    return;
+  /*if (window_slider_.get() && window_slider_->IsSlideInProgress())
+    return;*/
 
-  // The layer to be animated by OverlayDismissAnimator
+  /*// The layer to be animated by OverlayDismissAnimator
   scoped_ptr<ui::Layer> overlay_dismiss_layer;
   if (overlay_dismiss_layer_)
     overlay_dismiss_layer = overlay_dismiss_layer_.Pass();
   else if (window_.get())
     overlay_dismiss_layer = window_->AcquireLayer();
+  LOG(ERROR) << "Resetting window slider";
+  */
   Observe(NULL);
+  /*
+  LOG(ERROR) << "Overscroll mode change";
   window_slider_.reset();
   window_.reset();
   image_delegate_ = NULL;
@@ -203,23 +254,18 @@ void OverscrollNavigationOverlay::StopObservingIfDone() {
     // OverlayDismissAnimator deletes overlay_dismiss_layer and itself when the
     // animation completes.
     (new OverlayDismissAnimator(overlay_dismiss_layer.Pass()))->Animate();
-  }
+  }*/
+  LOG(ERROR) << "Calling dismiss overlay on wcva";
+  wcva_->DismissOverlay();

[mfomitchev, 2015/02/13 20:50:21]

As implemented, OverscrollWindowAnimation::DismissOverlay() (which ends up
getting called here) is not really a directive to dismiss - it is a notification
that the page finished loading/painting, and then OverscrollWindowAnimation
(OWA) really decides when to dismiss based on gesture_completed_, etc. This will
become more complicated once we add support for the slider scenario. I am not
sure this will work well. 

What I envisioned instead was that OWA would use a combination of mechanisms
similar to WindowSlider::Delegate callbacks and IsSlideInProgress() to keep ONO
up to date with its state, and then all the logic for dismissing would be in
ONO. Perhaps there's a way to make this work differently - it's hard to tell now
because the slider use case is not implemented. I am skeptical though - with the
approach I am suggesting OWA is clearly only focused on animation/UI input
processing, while as implemented it would also have to contain the logic for
layer/window management and timely dismissal based on load/repaint state - this
just seems like unnecessarily mixing two separate roles in one class.

- If we  take the approach I am suggesting, we should probably also keep the
DismissAnimator in ONO.

 }
 
+// TODO delete these *Layer functions.
 ui::Layer* OverscrollNavigationOverlay::CreateSlideLayer(int offset) {
-  const NavigationControllerImpl& controller = web_contents_->GetController();
-  const NavigationEntryImpl* entry = NavigationEntryImpl::FromNavigationEntry(
-      controller.GetEntryAtOffset(offset));
-
-  gfx::Image image;
-  if (entry && entry->screenshot().get()) {
-    std::vector<gfx::ImagePNGRep> image_reps;
-    image_reps.push_back(gfx::ImagePNGRep(entry->screenshot(), 1.0f));
-    image = gfx::Image(image_reps);
-  }
   if (!layer_delegate_)
     layer_delegate_.reset(new ImageLayerDelegate());
-  layer_delegate_->SetImage(image);
+  // TODO fixme.
+  layer_delegate_->SetImage(
+      GetImageForDirection(offset == -1 ? BACKWARD : FORWARD));
 
   ui::Layer* layer = new ui::Layer(ui::LAYER_TEXTURED);
   layer->set_delegate(layer_delegate_.get());
@@ -296,6 +342,7 @@ void OverscrollNavigationOverlay::OnWindowSliderDestroyed() {
 }
 
 void OverscrollNavigationOverlay::DidFirstVisuallyNonEmptyPaint() {
+  LOG(ERROR) << "Did first visually non empty paint";
   NavigationEntry* visible_entry =
       web_contents_->GetController().GetVisibleEntry();
   if (pending_entry_url_.is_empty() ||
@@ -306,6 +353,7 @@ void OverscrollNavigationOverlay::DidFirstVisuallyNonEmptyPaint() {
 }
 
 void OverscrollNavigationOverlay::DidStopLoading(RenderViewHost* host) {
+  LOG(ERROR) << "Did stop loading";
   // Don't compare URLs in this case - it's possible they won't match if
   // a gesture-nav initiated navigation was interrupted by some other in-site
   // navigation ((e.g., from a script, or from a bookmark).