Refactor GestureNavigation to eliminate code redundancy

content/browser/web_contents/aura/overscroll_navigation_overlay.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 "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_layer_wrapper.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 {
 
 // Returns true if the entry's URL or any of the URLs in entry's redirect chain
 // match |url|.
 bool DoesEntryMatchURL(NavigationEntry* entry, const GURL& url) {
   if (!entry)
     return false;
   if (entry->GetURL() == url)
     return true;
   const std::vector<GURL>& redirect_chain = entry->GetRedirectChain();
   for (std::vector<GURL>::const_iterator it = redirect_chain.begin();
        it != redirect_chain.end();
        it++) {
     if (*it == url)
       return true;
   }
   return false;
 }
 
 }  // namespace
 
 // A LayerDelegate that paints an image for the layer.
+// TODO move to overscroll_window.h???
 class ImageLayerDelegate : public ui::LayerDelegate {
  public:
   ImageLayerDelegate() {}
 
   ~ImageLayerDelegate() override {}
 
   void SetImage(const gfx::Image& image) {
     image_ = image;
     image_size_ = image.AsImageSkia().size();
   }
@@ skipping 67 matching lines @@
 
  private:
   ~OverlayDismissAnimator() override {}
 
   scoped_ptr<ui::Layer> layer_;
 
   DISALLOW_COPY_AND_ASSIGN(OverlayDismissAnimator);
 };
 
 OverscrollNavigationOverlay::OverscrollNavigationOverlay(
-    WebContentsImpl* web_contents)
-    : web_contents_(web_contents),
-      image_delegate_(NULL),
+    WebContentsImpl* web_contents,
+    OverscrollWindowAnimation* owa,
+    aura::Window* web_contents_window)
+    : direction_(OverscrollWindowAnimation::NONE),
+      web_contents_(web_contents),
+      window_(nullptr),
       loading_complete_(false),
-      received_paint_update_(false),
-      slide_direction_(SLIDE_UNKNOWN) {
+      observing_(false),
+      owa_(owa),
+      web_contents_window_(web_contents_window) {
 }
 
 OverscrollNavigationOverlay::~OverscrollNavigationOverlay() {
 }
 
 void OverscrollNavigationOverlay::StartObserving() {
+  LOG(ERROR) << "Starting to observe";
   loading_complete_ = false;
-  received_paint_update_ = false;
-  overlay_dismiss_layer_.reset();
+  observing_ = true;
   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();
+
   // Save url of the pending entry to identify when it loads and paints later.
   // Under some circumstances navigation can leave a null pending entry -
   // see comments in NavigationControllerImpl::NavigateToPendingEntry().
   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.
-  if (window_slider_.get() && window_slider_->IsSlideInProgress())
-    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();
-  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();
-  }
-}
-
-ui::Layer* OverscrollNavigationOverlay::CreateSlideLayer(int offset) {
+  LOG(ERROR) << "URL: " << pending_entry_url_.GetContent();
+}
+
+void OverscrollNavigationOverlay::SetupOverlayWindow() {
+  LOG(ERROR) << "ONO: Setting overlay window";
+  window_.reset(new aura::Window(
+      new OverscrollWindowDelegate(owa_, GetImageForDirection(direction_))));
+  window_->SetType(ui::wm::WINDOW_TYPE_CONTROL);
+  window_->SetTransparent(true);
+  window_->Init(aura::WINDOW_LAYER_TEXTURED);
+  window_->layer()->SetMasksToBounds(false);
+  window_->SetName("OverscrollOverlay");
+  window_->layer()->SetLayerBrightness(-0.1f);

[mfomitchev, 2015/03/06 01:36:43]

It doesn't seem like we are animating the brightness anymore, so I think we can
just get rid of this

[Nina, 2015/03/09 15:54:52]

Done.

+  web_contents_window_->AddChild(window_.get());
+  window_->Show();
+}
+
+const gfx::Image OverscrollNavigationOverlay::GetImageForDirection(
+    OverscrollWindowAnimation::Direction direction) const {
   const NavigationControllerImpl& controller = web_contents_->GetController();
-  const NavigationEntryImpl* entry = controller.GetEntryAtOffset(offset);
-
-  gfx::Image image;
+  const NavigationEntryImpl* entry = NavigationEntryImpl::FromNavigationEntry(
+      controller.GetEntryAtOffset(
+          direction == OverscrollWindowAnimation::FORWARD ? 1 : -1));
+
+  LOG(ERROR) << "ONO: Image for entry - " << entry->GetTitle();
   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);
+  }
+  return gfx::Image();
+}
+
+scoped_ptr<ui::Layer> OverscrollNavigationOverlay::CreateSlideLayer() {
+  LOG(ERROR) << "ONO: Creating slide layer";
   if (!layer_delegate_)
     layer_delegate_.reset(new ImageLayerDelegate());
-  layer_delegate_->SetImage(image);
-
-  ui::Layer* layer = new ui::Layer(ui::LAYER_TEXTURED);
+  layer_delegate_->SetImage(GetImageForDirection(direction_));
+  scoped_ptr<ui::Layer> layer(new ui::Layer(ui::LAYER_TEXTURED));
   layer->set_delegate(layer_delegate_.get());
-  return layer;
-}
-
-ui::Layer* OverscrollNavigationOverlay::CreateBackLayer() {
+  ui::Layer* parent = window_->layer()->parent();
+  parent->Add(layer.get());
+  if (direction_ == OverscrollWindowAnimation::FORWARD)

[mfomitchev, 2015/03/05 23:37:06]

I think somewhere here we should have LTR/RTL logic?

[Nina, 2015/03/09 15:54:52]

FORWARD and BACKWARD refer to history direction, not screen direction, so RTL
should have already been taken care of where this is set. Regardless of
direction, if we're going FORWARD we want to stack above the existing layer, and
vice versa.

[mfomitchev, 2015/03/10 19:25:58]

Ok, cool

+    parent->StackAbove(layer.get(), window_->layer());
+  else
+    parent->StackBelow(layer.get(), window_->layer());
+  gfx::Rect bounds = gfx::Rect(layer->parent()->bounds().size());
+  layer->SetBounds(bounds);
+  return layer.Pass();
+}
+
+scoped_ptr<OverscrollLayerWrapper> OverscrollNavigationOverlay::CreateLayer() {

[mfomitchev, 2015/03/05 23:37:05]

Method needs to be renamed - we are creating a wrapper (around window or layer),
not a layer

[Nina, 2015/03/09 15:54:52]

Agreed, had a TODO on the .h file.

+  LOG(ERROR) << "ONO: CreateLayer";
+  if (window_) {
+    LOG(ERROR) << "We have a window, returning slide layer";
+    return scoped_ptr<OverscrollLayerWrapper>(
+        new OverscrollLayerWrapper(CreateSlideLayer()));
+  }
+  SetupOverlayWindow();

[mfomitchev, 2015/03/05 23:37:06]

SetupOverlayWindow() should not inititalize window_. At this point ONO doesn't
own the overlay window - it is creating it for OWA, so it shouldn't set window_
to the overlay window, even intermittently for the duration of CreateLayer() -
it just makes the state/ownership confusing. window_ should only be set to the
overlay window in the "slide state".

[Nina, 2015/03/09 15:54:52]

Done.

+  if (direction_ == OverscrollWindowAnimation::FORWARD) {
+    web_contents_window_->StackChildAbove(
+        window_.get(), web_contents_->GetContentNativeView());
+  } else {
+    web_contents_window_->StackChildBelow(
+        window_.get(), web_contents_->GetContentNativeView());
+  }
+  return scoped_ptr<OverscrollLayerWrapper>(
+      new OverscrollLayerWrapper(window_.Pass()));
+}
+
+scoped_ptr<OverscrollLayerWrapper>
+OverscrollNavigationOverlay::CreateFrontLayer() {
+  LOG(ERROR) << "ONO: Create front layer";
+  if (!web_contents_->GetController().CanGoForward())
+    return nullptr;
+  direction_ = OverscrollWindowAnimation::FORWARD;
+  return CreateLayer();
+}
+
+scoped_ptr<OverscrollLayerWrapper>
+OverscrollNavigationOverlay::CreateBackLayer() {
+  LOG(ERROR) << "ONO: Create back layer";
   if (!web_contents_->GetController().CanGoBack())
-    return NULL;
-  slide_direction_ = SLIDE_BACK;
-  return CreateSlideLayer(-1);
-}
-
-ui::Layer* OverscrollNavigationOverlay::CreateFrontLayer() {
-  if (!web_contents_->GetController().CanGoForward())
-    return NULL;
-  slide_direction_ = SLIDE_FRONT;
-  return CreateSlideLayer(1);
-}
-
-void OverscrollNavigationOverlay::OnWindowSlideCompleting() {
-  if (slide_direction_ == SLIDE_UNKNOWN)
+    return nullptr;
+  direction_ = OverscrollWindowAnimation::BACKWARD;
+  return CreateLayer();
+}
+
+void OverscrollNavigationOverlay::OnOverscrollCompleted(
+    scoped_ptr<OverscrollLayerWrapper> layer_wrapper) {
+  LOG(ERROR) << "ONO: OnOverscrollCompleted";
+
+  // The overscroll gesture has been cancelled.
+  if (!observing_ && !loading_complete_) {

[mfomitchev, 2015/03/05 23:37:05]

This seems like the only place we use observing_. So if we add a separate
callback for cancelled overscroll - we should be able to get rid of observing_.

[Nina, 2015/03/09 15:54:52]

Done.

+    LOG(ERROR) << "Not observing, the gesture has been cancelled";
+    LOG(ERROR) << "Observing:        " << observing_;
+    LOG(ERROR) << "Loading complete: " << loading_complete_;
+    window_.reset();
     return;
-
-  // Perform the navigation.
-  if (slide_direction_ == SLIDE_BACK)
-    web_contents_->GetController().GoBack();
-  else if (slide_direction_ == SLIDE_FRONT)
+  }
+
+  // TODO if the first iteration captures a dismiss layer and the second a
+  // window_, this code might not work.
+  if (layer_wrapper->has_window())
+    window_ = layer_wrapper->AcquireWindow();
+  else
+    dismiss_layer_ = layer_wrapper->AcquireLayer();
+
+  // Make sure the overlay window is on top.
+  web_contents_window_->StackChildAtTop(window_.get());
+  //dismiss_layer_ = layer_wrapper->AcquireLayer();
+  owa_->set_live_window(window_.get());
+
+  // Reset the position of the contents window.
+  web_contents_->GetContentNativeView()->SetTransform(gfx::Transform());

[mfomitchev, 2015/03/06 01:36:43]

These three instructions should only be done for the overlay case
(layer_wrapper->has_window()), but not the slider case. Take a look at the old
OnWindowSlideCompleted() implementation to see what should be done for the
slider case

[Nina, 2015/03/09 15:54:52]

Done.
Agreed. This was not working at all.

+
+  StopObservingIfDone();
+}
+
+void OverscrollNavigationOverlay::OnOverscrollCompleting() {
+  // We start the navigation as soon as we know the overscroll gesture is
+  // completing.
+  LOG(ERROR) << "ONO: Starting navigation";
+  if (direction_ == OverscrollWindowAnimation::FORWARD)
     web_contents_->GetController().GoForward();
   else
-    NOTREACHED();
-
-  // Reset state and wait for the new navigation page to complete
-  // loading/painting.
+    web_contents_->GetController().GoBack();
   StartObserving();
 }
 
-void OverscrollNavigationOverlay::OnWindowSlideCompleted(
-    scoped_ptr<ui::Layer> layer) {
-  if (slide_direction_ == SLIDE_UNKNOWN) {
-    window_slider_.reset();
-    StopObservingIfDone();
+void OverscrollNavigationOverlay::FadeOutOverscrollWindow() {
+  LOG(ERROR) << "ONO: FadeOutOverscrollWindow";
+  // TODO maybe we do not need this check.
+  if (!dismiss_layer_ && !window_)
     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();
-  StopObservingIfDone();
-}
-
-void OverscrollNavigationOverlay::OnWindowSlideAborted() {
-  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();
-  }
+  if (!dismiss_layer_)
+    dismiss_layer_ = window_->AcquireLayer();
+  window_.reset();
+  aura::Window* contents = web_contents_->GetContentNativeView();
+  contents->layer()->SetLayerBrightness(1.f);
+  {
+    ui::ScopedLayerAnimationSettings settings(contents->layer()->GetAnimator());
+    settings.SetPreemptionStrategy(
+        ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
+    settings.SetTweenType(gfx::Tween::EASE_OUT);
+    contents->layer()->SetLayerBrightness(0.f);
+  }
+  (new OverlayDismissAnimator(dismiss_layer_.Pass()))->Animate();
+}
+
+void OverscrollNavigationOverlay::StopObservingIfDone() {

[mfomitchev, 2015/03/06 01:36:43]

Order of the methods should be the same as in .h file. It's also best not to
change the order unless there's a good reason - makes it easier to see the diff
when reviewing.

[Nina, 2015/03/09 15:54:52]

Done. I also rearranged some of the methods in a way that IMHO makes more sense.

+  LOG(ERROR) << "ONO: Stop observing if done";
+  if (!loading_complete_ || owa_->is_active()) {
+    LOG(ERROR) << "Returning early";
+    return;
+  }
+  FadeOutOverscrollWindow();
+  Observe(NULL);
+  observing_ = false;
+  loading_complete_ = false;
+  owa_->set_live_window(web_contents_window_);

[mfomitchev, 2015/03/06 01:36:43]

It's pretty unfortunate that ONO has to do this - it should have no business
setting up the relationship between web_contents_window_ and owa. Unfortunately
I don't see a good way for WCVA to do this.
Having two separate instances of OWA would solve this though. Is there any good
reason not to do it?

[Nina, 2015/03/09 15:54:52]

Yes
No. Done!

 }
 
 void OverscrollNavigationOverlay::DidFirstVisuallyNonEmptyPaint() {
+  LOG(ERROR) << "Did first visually non empty paint";
   NavigationEntry* visible_entry =
       web_contents_->GetController().GetVisibleEntry();
   if (pending_entry_url_.is_empty() ||
       DoesEntryMatchURL(visible_entry, pending_entry_url_)) {
-    received_paint_update_ = true;
+    loading_complete_ = true;
     StopObservingIfDone();
   }
 }
 
 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).
   loading_complete_ = true;
   StopObservingIfDone();
 }
 
 }  // namespace content