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 a96b0d27ed66cd46912ebc7d55fdee0feb5e9f9c..96f9b458599374cef2d48bd97a2c5ea6591b38fa 100644
--- a/content/browser/web_contents/aura/overscroll_navigation_overlay.cc
+++ b/content/browser/web_contents/aura/overscroll_navigation_overlay.cc
@@ -4,21 +4,22 @@
 
 #include "content/browser/web_contents/aura/overscroll_navigation_overlay.h"
 
+#include <vector>
+
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/browser/web_contents/aura/overscroll_window_delegate.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "ui/aura/window.h"
-#include "ui/aura_extra/image_window_delegate.h"
 #include "ui/base/layout.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/layer_animation_observer.h"
 #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 {
@@ -42,51 +43,6 @@ bool DoesEntryMatchURL(NavigationEntry* entry, const GURL& url) {
 
 }  // namespace
 
-// A LayerDelegate that paints an image for the layer.
-class ImageLayerDelegate : public ui::LayerDelegate {
- public:
-  ImageLayerDelegate() {}
-
-  ~ImageLayerDelegate() override {}
-
-  void SetImage(const gfx::Image& image) {
-    image_ = image;
-    image_size_ = image.AsImageSkia().size();
-  }
-  const gfx::Image& image() const { return image_; }
-
- private:
-  // Overridden from ui::LayerDelegate:
-  void OnPaintLayer(gfx::Canvas* canvas) override {
-    if (image_.IsEmpty()) {
-      canvas->DrawColor(SK_ColorWHITE);
-    } else {
-      SkISize size = canvas->sk_canvas()->getDeviceSize();
-      if (size.width() != image_size_.width() ||
-          size.height() != image_size_.height()) {
-        canvas->DrawColor(SK_ColorWHITE);
-      }
-      canvas->DrawImageInt(image_.AsImageSkia(), 0, 0);
-    }
-  }
-
-  void OnDelegatedFrameDamage(const gfx::Rect& damage_rect_in_dip) override {}
-
-  // Called when the layer's device scale factor has changed.
-  void OnDeviceScaleFactorChanged(float device_scale_factor) override {}
-
-  // Invoked prior to the bounds changing. The returned closured is run after
-  // the bounds change.
-  base::Closure PrepareForLayerBoundsChange() override {
-    return base::Closure();
-  }
-
-  gfx::Image image_;
-  gfx::Size image_size_;
-
-  DISALLOW_COPY_AND_ASSIGN(ImageLayerDelegate);
-};
-
 // Responsible for fading out and deleting the layer of the overlay window.
 class OverlayDismissAnimator
     : public ui::LayerAnimationObserver {
@@ -130,12 +86,15 @@ class OverlayDismissAnimator
 };
 
 OverscrollNavigationOverlay::OverscrollNavigationOverlay(
-    WebContentsImpl* web_contents)
-    : web_contents_(web_contents),
-      image_delegate_(NULL),
+    WebContentsImpl* web_contents,
+    aura::Window* web_contents_window)
+    : direction_(NONE),
+      web_contents_(web_contents),
+      window_(nullptr),

[mfomitchev, 2015/03/26 02:41:38]

no need to init scoped_ptr to null

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

Removed.

       loading_complete_(false),
       received_paint_update_(false),
-      slide_direction_(SLIDE_UNKNOWN) {
+      owa_(new OverscrollWindowAnimation(this)),
+      web_contents_window_(web_contents_window) {
 }
 
 OverscrollNavigationOverlay::~OverscrollNavigationOverlay() {
@@ -144,13 +103,8 @@ OverscrollNavigationOverlay::~OverscrollNavigationOverlay() {
 void OverscrollNavigationOverlay::StartObserving() {
   loading_complete_ = false;
   received_paint_update_ = false;
-  overlay_dismiss_layer_.reset();
   Observe(web_contents_);
 
-  // Make sure the overlay window is on top.
-  if (window_.get() && window_->parent())
-    window_->parent()->StackChildAtTop(window_.get());
-
   // Assumes the navigation has been initiated.
   NavigationEntry* pending_entry =
       web_contents_->GetController().GetPendingEntry();
@@ -160,140 +114,120 @@ void OverscrollNavigationOverlay::StartObserving() {
   pending_entry_url_ = pending_entry ? pending_entry->GetURL() : GURL();
 }
 
-void OverscrollNavigationOverlay::SetOverlayWindow(
-    scoped_ptr<aura::Window> window,
-    aura_extra::ImageWindowDelegate* delegate) {
-  window_ = window.Pass();
-  if (window_.get() && window_->parent())
-    window_->parent()->StackChildAtTop(window_.get());
-  image_delegate_ = delegate;
-
-  if (window_.get() && delegate->has_image()) {
-    window_slider_.reset(new WindowSlider(this,
-                                          window_->parent(),
-                                          window_.get()));
-    slide_direction_ = SLIDE_UNKNOWN;
-  } else {
-    window_slider_.reset();
-  }
-}
-
 void OverscrollNavigationOverlay::StopObservingIfDone() {
   // 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.
-  if (!received_paint_update_ && !loading_complete_)
-    return;
-
   // If a slide is in progress, then do not destroy the window or the slide.

[mfomitchev, 2015/03/26 02:41:39]

"slide" is not clear

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

Done.

-  if (window_slider_.get() && window_slider_->IsSlideInProgress())
+  if (!window_ || !(loading_complete_ || received_paint_update_) ||

[mfomitchev, 2015/03/26 02:41:38]

Do need both !window_ and owa_->is_active()?

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

Yes. For the first overscroll case, window_ == nullptr && owa_->is_active()
after OnOverscrollCompleting() but before OnOverscrollCompleted() => we want to
return early (no need to )

If the first overscroll is aborted, window_ == nullptr && !owa_->is_active() =>
we want to return early.

If the first overscroll completes successfully and another one is started before
the page loads, window_ != nullptr && owa_->is_active => return early.

For all the successful overscroll cases where we want to dismiss the overlay, we
hold a reference to the window_ and owa is not active.

+      owa_->is_active()) {
     return;
-
-  // 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();
-  Observe(NULL);
-  window_slider_.reset();
+  }
+  // OverlayDismissAnimator deletes the dismiss layer and itself when the
+  // animation completes.
+  scoped_ptr<ui::Layer> dismiss_layer = window_->AcquireLayer();
   window_.reset();
-  image_delegate_ = NULL;
-  if (overlay_dismiss_layer.get()) {
-    // OverlayDismissAnimator deletes overlay_dismiss_layer and itself when the
-    // animation completes.
-    (new OverlayDismissAnimator(overlay_dismiss_layer.Pass()))->Animate();
+  (new OverlayDismissAnimator(dismiss_layer.Pass()))->Animate();
+  Observe(NULL);
+  received_paint_update_ = false;
+  loading_complete_ = false;
+}
+
+scoped_ptr<aura::Window> OverscrollNavigationOverlay::CreateSlideWindow() {
+  OverscrollWindowDelegate* overscroll_delegate = new OverscrollWindowDelegate(
+      owa_.get(), GetImageForDirection(direction_));
+  scoped_ptr<aura::Window> window(new aura::Window(overscroll_delegate));
+  window->SetTransparent(true);
+  window->Init(aura::WINDOW_LAYER_TEXTURED);
+  window->layer()->SetMasksToBounds(false);
+  window->SetName("OverscrollOverlay");
+  web_contents_window_->AddChild(window.get());
+  aura::Window* target_window = GetTargetWindow();

[mfomitchev, 2015/03/26 02:41:39]

event_window? top_window? owner_window?

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

Event window sgtm

+  if (target_window) {

[mfomitchev, 2015/03/26 02:41:38]

Let's find a way to get rid of this if as we discussed. For now maybe add a
TODO.

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

Found and done

+    if (direction_ == FORWARD)
+      web_contents_window_->StackChildAbove(window.get(), target_window);
+    else
+      web_contents_window_->StackChildBelow(window.get(), target_window);
+    target_window->SetCapture();

[mfomitchev, 2015/03/26 02:41:39]

A short comment here would be nice

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

Done.

   }
+  window->Show();
+  return window.Pass();
 }
 
-ui::Layer* OverscrollNavigationOverlay::CreateSlideLayer(int offset) {
+const gfx::Image OverscrollNavigationOverlay::GetImageForDirection(
+    NavigationDirection direction) const {
   const NavigationControllerImpl& controller = web_contents_->GetController();
-  const NavigationEntryImpl* entry = controller.GetEntryAtOffset(offset);
+  const NavigationEntryImpl* entry = NavigationEntryImpl::FromNavigationEntry(
+      controller.GetEntryAtOffset(direction == FORWARD ? 1 : -1));
 
-  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);
+    return gfx::Image(image_reps);
   }
-  if (!layer_delegate_)
-    layer_delegate_.reset(new ImageLayerDelegate());
-  layer_delegate_->SetImage(image);
+  return gfx::Image();
+}
 
-  ui::Layer* layer = new ui::Layer(ui::LAYER_TEXTURED);
-  layer->set_delegate(layer_delegate_.get());
-  return layer;
+scoped_ptr<aura::Window> OverscrollNavigationOverlay::CreateFrontWindow() {
+  if (!web_contents_->GetController().CanGoForward())
+    return nullptr;
+  direction_ = FORWARD;
+  return CreateSlideWindow();
 }
 
-ui::Layer* OverscrollNavigationOverlay::CreateBackLayer() {
+scoped_ptr<aura::Window> OverscrollNavigationOverlay::CreateBackWindow() {
   if (!web_contents_->GetController().CanGoBack())
-    return NULL;
-  slide_direction_ = SLIDE_BACK;
-  return CreateSlideLayer(-1);
+    return nullptr;
+  direction_ = BACK;
+  return CreateSlideWindow();
 }
 
-ui::Layer* OverscrollNavigationOverlay::CreateFrontLayer() {
-  if (!web_contents_->GetController().CanGoForward())
-    return NULL;
-  slide_direction_ = SLIDE_FRONT;
-  return CreateSlideLayer(1);
+aura::Window* OverscrollNavigationOverlay::GetTargetWindow() const {
+  if (window_)
+    return window_.get();
+  return web_contents_->GetContentNativeView();
 }
 
-void OverscrollNavigationOverlay::OnWindowSlideCompleting() {
-  if (slide_direction_ == SLIDE_UNKNOWN)
-    return;
-
-  // Perform the navigation.
-  if (slide_direction_ == SLIDE_BACK)
-    web_contents_->GetController().GoBack();
-  else if (slide_direction_ == SLIDE_FRONT)
+void OverscrollNavigationOverlay::OnOverscrollCompleting() {
+  aura::Window* target_window = GetTargetWindow();
+  if (target_window)
+    target_window->ReleaseCapture();
+  // We start the navigation as soon as we know the overscroll gesture is
+  // completing.
+  DCHECK(direction_ != NONE);
+  // Make sure we can navigate first, as other factors can trigger a navigation
+  // during an overscroll gesture and navigating without history produces a
+  // crash.
+  if (direction_ == FORWARD && web_contents_->GetController().CanGoForward())

[mfomitchev, 2015/03/26 02:41:38]

We should probably change this logic, since it looks like we won't be able to
fix the screenshot issue in time (is there a crbug for that btw?) Lets do that,
add a TODO, and reference the crbug in a TODO.

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

Looks like we were able to fix it by avoiding clipping. I'll double check with
vollick@ though.

     web_contents_->GetController().GoForward();
-  else
-    NOTREACHED();
-
-  // Reset state and wait for the new navigation page to complete
-  // loading/painting.
+  if (direction_ == BACK && web_contents_->GetController().CanGoBack())
+    web_contents_->GetController().GoBack();
   StartObserving();
 }
 
-void OverscrollNavigationOverlay::OnWindowSlideCompleted(
-    scoped_ptr<ui::Layer> layer) {
-  if (slide_direction_ == SLIDE_UNKNOWN) {
-    window_slider_.reset();
-    StopObservingIfDone();
-    return;
-  }
-
-  // Change the image used for the overlay window.
-  image_delegate_->SetImage(layer_delegate_->image());
-  window_->layer()->SetTransform(gfx::Transform());
-  window_->SchedulePaintInRect(gfx::Rect(window_->bounds().size()));
-  slide_direction_ = SLIDE_UNKNOWN;
-  // We may end up dismissing the overlay before it has a chance to repaint, so
-  // set the slider layer to be the one animated by OverlayDismissAnimator.
-  if (layer.get())
-    overlay_dismiss_layer_ = layer.Pass();
+void OverscrollNavigationOverlay::OnOverscrollCompleted(
+    scoped_ptr<aura::Window> window) {
+  window_ = window.Pass();
+  // Make sure the overlay window is on top.
+  web_contents_window_->StackChildAtTop(window_.get());
+  // Reset the position of the contents window.
+  if (web_contents_->GetContentNativeView())
+    web_contents_->GetContentNativeView()->SetTransform(gfx::Transform());

[mfomitchev, 2015/03/26 02:41:39]

Can we avoid doing this for the slider case? Perhaps we could just do that on
the GetTargetWindow() (before we assign window.Pass to window_ though, otherwise
it won't work)

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

Done.

+  // Reset the position of the overlay window.
+  window_->SetBounds(gfx::Rect(web_contents_window_->bounds().size()));
+  window_->SetTransform(gfx::Transform());
+  direction_ = NONE;
   StopObservingIfDone();
 }
 
-void OverscrollNavigationOverlay::OnWindowSlideAborted() {
+void OverscrollNavigationOverlay::OnOverscrollAborted() {
+  aura::Window* target_window = GetTargetWindow();
+  if (target_window)
+    target_window->ReleaseCapture();
+  direction_ = NONE;
   StopObservingIfDone();
 }
 
-void OverscrollNavigationOverlay::OnWindowSliderDestroyed() {
-  // We only want to take an action here if WindowSlider is being destroyed
-  // outside of OverscrollNavigationOverlay. If window_slider_.get() is NULL,
-  // then OverscrollNavigationOverlay is the one destroying WindowSlider, and
-  // we don't need to do anything.
-  // This check prevents StopObservingIfDone() being called multiple times
-  // (including recursively) for a single event.
-  if (window_slider_.get()) {
-    // The slider has just been destroyed. Release the ownership.
-    ignore_result(window_slider_.release());
-    StopObservingIfDone();
-  }
-}
-
 void OverscrollNavigationOverlay::DidFirstVisuallyNonEmptyPaint() {
   NavigationEntry* visible_entry =
       web_contents_->GetController().GetVisibleEntry();
@@ -307,7 +241,7 @@ void OverscrollNavigationOverlay::DidFirstVisuallyNonEmptyPaint() {
 void OverscrollNavigationOverlay::DidStopLoading(RenderViewHost* host) {
   // 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).
+  // navigation (e.g., from a script, or from a bookmark).
   loading_complete_ = true;
   StopObservingIfDone();
 }