Fixing race conditions in ui::content::WindowSlider which could cause the overscroll overlay to nev…

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"
 #include "content/common/view_messages.h"
+#include "content/public/browser/render_widget_host_view.h"
 #include "ui/aura/window.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 {
 
@@ skipping 193 matching lines @@
     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() {
+ui::Layer* OverscrollNavigationOverlay::OnSlideBackStartedCreateLayer() {
   if (!web_contents_->GetController().CanGoBack())
     return NULL;
   slide_direction_ = SLIDE_BACK;
   return CreateSlideLayer(-1);
 }
 
-ui::Layer* OverscrollNavigationOverlay::CreateFrontLayer() {
+ui::Layer* OverscrollNavigationOverlay::OnSlideForwardStartedCreateLayer() {
   if (!web_contents_->GetController().CanGoForward())
     return NULL;
   slide_direction_ = SLIDE_FRONT;
   return CreateSlideLayer(1);
 }
 
-void OverscrollNavigationOverlay::OnWindowSlideComplete() {
+void OverscrollNavigationOverlay::OnWindowFlingStarted() {
   if (slide_direction_ == SLIDE_UNKNOWN) {

[sadrul, 2014/03/19 20:21:18]

You can remove the braces

[mfomitchev, 2014/03/20 15:10:06]

Done.

-    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()));
-
-  SlideDirection direction = slide_direction_;
-  slide_direction_ = SLIDE_UNKNOWN;
-
   // Reset state and wait for the new navigation page to complete
   // loading/painting.
   StartObserving();
 
   // Perform the navigation.
-  if (direction == SLIDE_BACK)
+  if (slide_direction_ == SLIDE_BACK)
     web_contents_->GetController().GoBack();
-  else if (direction == SLIDE_FRONT)
+  else if (slide_direction_ == SLIDE_FRONT)
     web_contents_->GetController().GoForward();
   else
     NOTREACHED();
 
   NavigationEntry* pending_entry =
       web_contents_->GetController().GetPendingEntry();
   // Save id 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_id_ = pending_entry ? pending_entry->GetUniqueID() : 0;
 }
 
+void OverscrollNavigationOverlay::OnWindowFlingCompleted() {
+  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();
+  }
+}
+
 void OverscrollNavigationOverlay::OnWindowSlideAborted() {
+  // The overlay may have been kept because of an in-progress slide, so check
+  // if we need to dismiss it.

[sadrul, 2014/03/19 20:21:18]

I am not sure I understand this comment.

[mfomitchev, 2014/03/20 15:10:06]

Yeah, it's poorly worded. Basically I was just trying to say that we need to
check if we can remove the overlay here (because we may have loaded and painted
the page, but kept the overlay because IsSlideInProgress() returned true). I'll
just remove the comment, I think it should be pretty self explanatory.

   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.
@@ skipping 46 matching lines @@
 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