Removing listening for repaints (OnUpdateRect) from OverscrollNavigationOverlay.

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 1a17d9056ac6b4976259fc5013f0342564766d16..2954f63210e22c0558609d7a49ce1a555b75ecc9 100644
--- a/content/browser/web_contents/aura/overscroll_navigation_overlay.cc
+++ b/content/browser/web_contents/aura/overscroll_navigation_overlay.cc
@@ -115,8 +115,7 @@ OverscrollNavigationOverlay::OverscrollNavigationOverlay(
       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() {
@@ -125,6 +124,7 @@ OverscrollNavigationOverlay::~OverscrollNavigationOverlay() {
 void OverscrollNavigationOverlay::StartObserving() {
   loading_complete_ = false;
   received_paint_update_ = false;
+  overlay_dismiss_layer_.reset();
   pending_entry_id_ = 0;
   Observe(web_contents_);
 
@@ -159,30 +159,31 @@ void OverscrollNavigationOverlay::SetOverlayWindow(
   }
 }
 
-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();
   }
 }
 
@@ -208,22 +209,6 @@ ui::Layer* OverscrollNavigationOverlay::CreateSlideLayer(int offset) {
   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;
@@ -255,7 +240,8 @@ void OverscrollNavigationOverlay::OnWindowSlideCompleting() {
   StartObserving();
 }
 
-void OverscrollNavigationOverlay::OnWindowSlideCompleted() {
+void OverscrollNavigationOverlay::OnWindowSlideCompleted(
+    scoped_ptr<ui::Layer> layer) {
   if (slide_direction_ == SLIDE_UNKNOWN) {
     window_slider_.reset();
     StopObservingIfDone();
@@ -267,20 +253,11 @@ void OverscrollNavigationOverlay::OnWindowSlideCompleted() {
   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() {
@@ -301,18 +278,6 @@ void OverscrollNavigationOverlay::OnWindowSliderDestroyed() {
   }
 }
 
-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();
@@ -329,22 +294,8 @@ void OverscrollNavigationOverlay::DidStopLoading(RenderViewHost* host) {
       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