Check auto scroll not canceled before schedule animation

This issue is caused by AutoscrollForSelection call UpdateSelectionForMouseDrag may cancel auto scroll by layout. In this patch we check auto scroll state before schedule animation. Bug: 781455 Change-Id: I904ae86aaaa4694a5ce56f9daca7daf778d8a6c8 Reviewed-on: https://chromium-review.googlesource.com/776622 Commit-Queue: Jianpeng Chao <chaopeng@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#517711}
marcosholgado · Nov 18, 2017 · b536038 · b536038
1 parent eaa4b88
commit b536038
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 2 deletions.
diff --git a/third_party/WebKit/Source/core/BUILD.gn b/third_party/WebKit/Source/core/BUILD.gn
@@ -1675,6 +1675,7 @@ jumbo_source_set("unit_tests") {
     "loader/resource/MockImageResourceObserver.h",
     "loader/resource/MultipartImageResourceParserTest.cpp",
     "origin_trials/OriginTrialContextTest.cpp",
+    "page/AutoscrollControllerTest.cpp",
     "page/ChromeClientImplTest.cpp",
     "page/ChromeClientTest.cpp",
     "page/DragControllerTest.cpp",

diff --git a/third_party/WebKit/Source/core/page/AutoscrollController.cpp b/third_party/WebKit/Source/core/page/AutoscrollController.cpp
@@ -333,8 +333,14 @@ void AutoscrollController::Animate(double) {
         return;
       }
       event_handler.UpdateSelectionForMouseDrag();
-      ScheduleMainThreadAnimation();
-      autoscroll_layout_object_->Autoscroll(selection_point);
+
+      // UpdateSelectionForMouseDrag may call layout to cancel auto scroll
+      // animation.
+      if (autoscroll_type_ != kNoAutoscroll) {
+        DCHECK(autoscroll_layout_object_);
+        ScheduleMainThreadAnimation();
+        autoscroll_layout_object_->Autoscroll(selection_point);
+      }
       break;
     case kNoAutoscroll:
     case kAutoscrollForMiddleClick:
@@ -347,4 +353,8 @@ void AutoscrollController::ScheduleMainThreadAnimation() {
       autoscroll_layout_object_->GetFrame()->View());
 }
 
+bool AutoscrollController::IsAutoscrolling() const {
+  return (autoscroll_type_ != kNoAutoscroll);
+}
+
 }  // namespace blink
diff --git a/third_party/WebKit/Source/core/page/AutoscrollController.h b/third_party/WebKit/Source/core/page/AutoscrollController.h
@@ -26,6 +26,7 @@
 #ifndef AutoscrollController_h
 #define AutoscrollController_h
 
+#include "base/gtest_prod_util.h"
 #include "core/CoreExport.h"
 #include "platform/geometry/FloatPoint.h"
 #include "platform/geometry/FloatSize.h"
@@ -97,6 +98,9 @@ class CORE_EXPORT AutoscrollController final
  private:
   explicit AutoscrollController(Page&);
 
+  // For test.
+  bool IsAutoscrolling() const;
+
   Member<Page> page_;
   AutoscrollType autoscroll_type_ = kNoAutoscroll;
 
@@ -111,6 +115,9 @@ class CORE_EXPORT AutoscrollController final
   FloatPoint middle_click_autoscroll_start_pos_global_;
   FloatSize last_velocity_;
   MiddleClickMode middle_click_mode_ = kMiddleClickInitial;
+
+  FRIEND_TEST_ALL_PREFIXES(AutoscrollControllerTest,
+                           CrashWhenLayoutStopAnimationBeforeScheduleAnimation);
 };
 
 }  // namespace blink

diff --git a/third_party/WebKit/Source/core/page/AutoscrollControllerTest.cpp b/third_party/WebKit/Source/core/page/AutoscrollControllerTest.cpp
@@ -0,0 +1,81 @@
+// Copyright 2017 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 "core/dom/Element.h"
+#include "core/input/EventHandler.h"
+#include "core/page/AutoscrollController.h"
+#include "core/page/Page.h"
+#include "core/testing/sim/SimDisplayItemList.h"
+#include "core/testing/sim/SimRequest.h"
+#include "core/testing/sim/SimTest.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blink {
+
+class AutoscrollControllerTest : public SimTest {
+ public:
+  AutoscrollController& GetAutoscrollController() {
+    return WebView().GetPage()->GetAutoscrollController();
+  }
+};
+
+// Ensure Autoscroll not crash by layout called in UpdateSelectionForMouseDrag.
+TEST_F(AutoscrollControllerTest,
+       CrashWhenLayoutStopAnimationBeforeScheduleAnimation) {
+  WebView().Resize(WebSize(800, 600));
+  WebView().SetBaseBackgroundColorOverride(SK_ColorTRANSPARENT);
+  SimRequest request("https://example.com/test.html", "text/html");
+  LoadURL("https://example.com/test.html");
+  request.Complete(R"HTML(
+    <!DOCTYPE html>
+    <style>
+      #scrollable {
+        overflow: auto;
+        width: 10px;
+        height: 10px;
+      }
+    </style>
+    <div id='scrollable'>
+      <p id='p'>Some text here for selection autoscroll.</p>
+      <p>Some text here for selection autoscroll.</p>
+      <p>Some text here for selection autoscroll.</p>
+      <p>Some text here for selection autoscroll.</p>
+      <p>Some text here for selection autoscroll.</p>
+      <p>Some text here for selection autoscroll.</p>
+      <p>Some text here for selection autoscroll.</p>
+      <p>Some text here for selection autoscroll.</p>
+    </div>
+  )HTML");
+
+  Compositor().BeginFrame();
+
+  AutoscrollController& controller = GetAutoscrollController();
+  Document& document = GetDocument();
+
+  Element* scrollable = document.getElementById("scrollable");
+  DCHECK(scrollable);
+  DCHECK(scrollable->GetLayoutObject());
+
+  WebMouseEvent event(WebInputEvent::kMouseDown, WebFloatPoint(5, 5),
+                      WebFloatPoint(5, 5), WebPointerProperties::Button::kLeft,
+                      0, WebInputEvent::Modifiers::kLeftButtonDown,
+                      TimeTicks::Now().InSeconds());
+  event.SetFrameScale(1);
+
+  GetDocument().GetFrame()->GetEventHandler().HandleMousePressEvent(event);
+
+  controller.StartAutoscrollForSelection(scrollable->GetLayoutObject());
+
+  DCHECK(controller.IsAutoscrolling());
+
+  // Hide scrollable here will cause UpdateSelectionForMouseDrag stop animation.
+  scrollable->SetInlineStyleProperty(CSSPropertyDisplay, CSSValueNone);
+
+  // BeginFrame will call AutoscrollController::Animate.
+  Compositor().BeginFrame();
+
+  EXPECT_FALSE(controller.IsAutoscrolling());
+}
+
+}  // namespace blink