Removing listening for repaints (OnUpdateRect) from OverscrollNavigationOverlay.

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/image_window_delegate.h"
 #include "content/browser/web_contents/web_contents_impl.h"
@@ skipping 97 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(OverlayDismissAnimator);
 };
 
 OverscrollNavigationOverlay::OverscrollNavigationOverlay(
     WebContentsImpl* web_contents)
     : web_contents_(web_contents),
       image_delegate_(NULL),
       loading_complete_(false),
       received_paint_update_(false),
       pending_entry_id_(0),
-      slide_direction_(SLIDE_UNKNOWN),
-      need_paint_update_(true) {
+      slide_direction_(SLIDE_UNKNOWN) {
 }
 
 OverscrollNavigationOverlay::~OverscrollNavigationOverlay() {
 }
 
 void OverscrollNavigationOverlay::StartObserving() {
   loading_complete_ = false;
   received_paint_update_ = false;
+  overlay_dismiss_layer_.reset();
   pending_entry_id_ = 0;
   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();
@@ skipping 14 matching lines @@
   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::SetupForTesting() {
-  need_paint_update_ = false;
-}
-
 void OverscrollNavigationOverlay::StopObservingIfDone() {
-  if ((need_paint_update_ && !received_paint_update_)) {
+  // 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;
 
-  scoped_ptr<ui::Layer> layer;
-  if (window_.get())
-    layer = window_->AcquireLayer();
+  // 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();

[sadrul, 2014/05/27 16:48:29]

Does this work correctly?

From what I can see, |overlay_dismiss_layer_| is parented to |window_|'s layer.
If you destroy |window_| below in line 181, then |overlay_dismiss_layer| would
become un-parented, and the layer wouldn't show up. Is that not the case?

[mfomitchev, 2014/05/27 21:21:05]

It works. In this case overlay_dismiss_layer_ is the slider's layer. I don't
think window's layer is the parent of slider window's layer. It's parent is the
parent of window's layer. 
From SetupSlider():
ui::Layer* parent = owner_->layer()->parent();
..
parent->StackAbove(slider_.get(), owner_->layer());

+  else if (window_.get())
+    overlay_dismiss_layer = window_->AcquireLayer();
   Observe(NULL);
   window_slider_.reset();
   window_.reset();
   image_delegate_ = NULL;
-  if (layer.get()) {
-    // OverlayDismissAnimator deletes the layer and itself when the animation
-    // completes.
-    (new OverlayDismissAnimator(layer.Pass()))->Animate();
+  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) {
   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(),
         ui::GetImageScale(
             ui::GetScaleFactorForNativeView(window_.get()))));
     image = gfx::Image(image_reps);
   }
   if (!layer_delegate_)
     layer_delegate_.reset(new ImageLayerDelegate());
   layer_delegate_->SetImage(image);
 
   ui::Layer* layer = new ui::Layer(ui::LAYER_TEXTURED);
   layer->set_delegate(layer_delegate_.get());
   return layer;
 }
 
-void OverscrollNavigationOverlay::OnUpdateRect(
-    const ViewHostMsg_UpdateRect_Params& params) {
-  if (loading_complete_ &&
-      ViewHostMsg_UpdateRect_Flags::is_repaint_ack(params.flags)) {
-    NavigationEntry* visible_entry =
-        web_contents_->GetController().GetVisibleEntry();
-    int visible_entry_id = visible_entry ? visible_entry->GetUniqueID() : 0;
-    if (visible_entry_id == pending_entry_id_ || !pending_entry_id_) {
-      // This is a paint update after the page has been loaded. So do not wait
-      // for a 'first non-empty' paint update.
-      received_paint_update_ = true;
-      StopObservingIfDone();
-    }
-  }
-}
-
 ui::Layer* OverscrollNavigationOverlay::CreateBackLayer() {
   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;
@@ skipping 11 matching lines @@
   else if (slide_direction_ == SLIDE_FRONT)
     web_contents_->GetController().GoForward();
   else
     NOTREACHED();
 
   // Reset state and wait for the new navigation page to complete
   // loading/painting.
   StartObserving();
 }
 
-void OverscrollNavigationOverlay::OnWindowSlideCompleted() {
+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;
-
-  // Make sure the overlay layer is repainted before we dismiss it, otherwise
-  // OverlayDismissAnimator may end up showing the wrong screenshot during the
-  // fadeout animation.
-  if (received_paint_update_ && need_paint_update_) {
-    received_paint_update_ = false;
-    RenderWidgetHost* host =
-        web_contents_->GetRenderWidgetHostView()->GetRenderWidgetHost();
-    RenderViewHostImpl* view_host =
-        static_cast<RenderViewHostImpl*> (RenderViewHost::From(host));
-    view_host->ScheduleComposite();
-  } else if (!need_paint_update_) {
-    StopObservingIfDone();
-  }
+  // 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.
     WindowSlider* slider ALLOW_UNUSED = window_slider_.release();
     StopObservingIfDone();
   }
 }
 
-void OverscrollNavigationOverlay::DocumentOnLoadCompletedInMainFrame() {

[sadrul, 2014/05/27 16:48:29]

This is related to document loading. Why are we removing this? (IIRC this allows
removing the overlay earlier?)

[mfomitchev, 2014/05/27 21:21:05]

It was helping dismiss the page sooner when we required both a paint and a load,
but now that the logic changed I don't think we want this:
- for inter-page navigation we want to dismiss the overlay after the first paint
happens. If this method is somehow called before that (I couldn't make that
happen btw), we still want to wait until the paint. 
- For in-page navigation I don't see this called at all - I guess because the
document has already been loaded and isn't being reloaded again. I just see
DidStopLoading. 

So I didn't think there was any need to keep this.

-  // Use the last committed entry rather than the active one, in case a
-  // pending entry has been created.
-  int committed_entry_id =
-      web_contents_->GetController().GetLastCommittedEntry()->GetUniqueID();
-  // Consider the loading completed once the main frame has loaded.
-  if (committed_entry_id == pending_entry_id_ || !pending_entry_id_) {
-    loading_complete_ = true;
-    StopObservingIfDone();
-  }
-}
-
 void OverscrollNavigationOverlay::DidFirstVisuallyNonEmptyPaint() {
   int visible_entry_id =
       web_contents_->GetController().GetVisibleEntry()->GetUniqueID();
   if (visible_entry_id == pending_entry_id_ || !pending_entry_id_) {
     received_paint_update_ = true;
     StopObservingIfDone();
   }
 }
 
 void OverscrollNavigationOverlay::DidStopLoading(RenderViewHost* host) {
   // Use the last committed entry rather than the active one, in case a
   // pending entry has been created.
   int committed_entry_id =
       web_contents_->GetController().GetLastCommittedEntry()->GetUniqueID();
   if (committed_entry_id == pending_entry_id_ || !pending_entry_id_) {
     loading_complete_ = true;
-    if (!received_paint_update_ && need_paint_update_) {
-      // Force a repaint after the page is loaded.
-      RenderViewHostImpl* view = static_cast<RenderViewHostImpl*>(host);
-      view->ScheduleComposite();
-    }
     StopObservingIfDone();
   }
 }
 
-bool OverscrollNavigationOverlay::OnMessageReceived(
-    const IPC::Message& message) {
-  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  IPC_BEGIN_MESSAGE_MAP(OverscrollNavigationOverlay, message)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateRect, OnUpdateRect)
-  IPC_END_MESSAGE_MAP()
-  return false;
-}
-
 }  // namespace content