Bug 1273045 Part 2 - Update carets when scrolling in subframes withou…

…t APZ. r=mtseng

Override OnScrollPositionChanged() in ScrollState because we want to update
carets during scrolling in subframes without APZ.

Due to the observation in bug 1273045 comment 8, we do not distinguish
PositionChangedResult::NotChanged and PositionChangedResult::Changed.
Instead, we always update caret even if its position is not changed.

To avoid excessive CaretStateChangedEvents are dispatched in
OnScrollPositionChanged(), we add IsScrollStarted to distinguish whether
OnScrollStart() is called or not.

MozReview-Commit-ID: KNi9Mct4dSk

layout/base/AccessibleCaretEventHub.cpp

@@ -249,6 +249,12 @@ class AccessibleCaretEventHub::ScrollState
     aContext->SetState(aContext->PostScrollState());
   }
 
+  virtual void OnScrollPositionChanged(
+    AccessibleCaretEventHub* aContext) override
+  {
+    aContext->mManager->OnScrollPositionChanged();
+  }
+
   virtual void OnBlur(AccessibleCaretEventHub* aContext,
                       bool aIsLeavingDocument) override
   {

layout/base/AccessibleCaretManager.cpp

@@ -63,6 +63,7 @@ std::ostream& operator<<(std::ostream& aStream,
   switch (aHint) {
     AC_PROCESS_ENUM_TO_STREAM(UpdateCaretsHint::Default);
     AC_PROCESS_ENUM_TO_STREAM(UpdateCaretsHint::RespectOldAppearance);
+    AC_PROCESS_ENUM_TO_STREAM(UpdateCaretsHint::DispatchNoEvent);
   }
   return aStream;
 }
@@ -295,14 +296,10 @@ AccessibleCaretManager::UpdateCaretsForCursorMode(UpdateCaretsHintSet aHints)
     return;
   }
 
-  bool oldSecondCaretVisible = mSecondCaret->IsLogicallyVisible();
   PositionChangedResult result = mFirstCaret->SetPosition(frame, offset);
 
   switch (result) {
     case PositionChangedResult::NotChanged:
-      // Do nothing
-      break;
-
     case PositionChangedResult::Changed:
       if (aHints == UpdateCaretsHint::Default) {
         if (HasNonEmptyTextContent(GetEditingHostForFrame(frame))) {
@@ -342,7 +339,7 @@ AccessibleCaretManager::UpdateCaretsForCursorMode(UpdateCaretsHintSet aHints)
 
   LaunchCaretTimeoutTimer();
 
-  if ((result != PositionChangedResult::NotChanged || oldSecondCaretVisible) &&
+  if (!aHints.contains(UpdateCaretsHint::DispatchNoEvent) &&
       !mActiveCaret) {
     DispatchCaretStateChangedEvent(CaretChangedReason::Updateposition);
   }
@@ -375,9 +372,6 @@ AccessibleCaretManager::UpdateCaretsForSelectionMode(UpdateCaretsHintSet aHints)
 
     switch (result) {
       case PositionChangedResult::NotChanged:
-        // Do nothing
-        break;
-
       case PositionChangedResult::Changed:
         if (aHints == UpdateCaretsHint::Default) {
           aCaret->SetAppearance(Appearance::Normal);
@@ -418,7 +412,8 @@ AccessibleCaretManager::UpdateCaretsForSelectionMode(UpdateCaretsHintSet aHints)
     }
   }
 
-  if (!mActiveCaret) {
+  if (!aHints.contains(UpdateCaretsHint::DispatchNoEvent) &&
+      !mActiveCaret) {
     DispatchCaretStateChangedEvent(CaretChangedReason::Updateposition);
   }
 }
@@ -658,6 +653,8 @@ AccessibleCaretManager::OnScrollStart()
 {
   AC_LOG("%s", __FUNCTION__);
 
+  mIsScrollStarted = true;
+
   if (!sCaretsAlwaysShowWhenScrolling) {
     // Backup the appearance so that we can restore them after the scrolling
     // ends.
@@ -681,6 +678,8 @@ AccessibleCaretManager::OnScrollEnd()
     return;
   }
 
+  mIsScrollStarted = false;
+
   if (!sCaretsAlwaysShowWhenScrolling) {
     // Restore the appearance which is saved before the scrolling is started.
     mFirstCaret->SetAppearance(mFirstCaretAppearanceOnScrollStart);
@@ -715,8 +714,17 @@ AccessibleCaretManager::OnScrollPositionChanged()
   }
 
   if (mFirstCaret->IsLogicallyVisible() || mSecondCaret->IsLogicallyVisible()) {
-    AC_LOG("%s: UpdateCarets(RespectOldAppearance)", __FUNCTION__);
-    UpdateCarets(UpdateCaretsHint::RespectOldAppearance);
+    if (mIsScrollStarted) {
+      // We don't want extra CaretStateChangedEvents dispatched when user is
+      // scrolling the page.
+      AC_LOG("%s: UpdateCarets(RespectOldAppearance | DispatchNoEvent)",
+             __FUNCTION__);
+      UpdateCarets({ UpdateCaretsHint::RespectOldAppearance,
+                     UpdateCaretsHint::DispatchNoEvent });
+    } else {
+      AC_LOG("%s: UpdateCarets(RespectOldAppearance)", __FUNCTION__);
+      UpdateCarets(UpdateCaretsHint::RespectOldAppearance);
+    }
   }
 }
 

layout/base/AccessibleCaretManager.h

@@ -130,7 +130,11 @@ class AccessibleCaretManager
     // Update everything while respecting the old appearance. For example, if
     // the caret in cursor mode is hidden due to timeout, do not change its
     // appearance to Normal.
-    RespectOldAppearance
+    RespectOldAppearance,
+
+    // No CaretStateChangedEvent will be dispatched in the end of
+    // UpdateCarets().
+    DispatchNoEvent,
   };
 
   using UpdateCaretsHintSet = mozilla::EnumSet<UpdateCaretsHint>;
@@ -296,6 +300,9 @@ class AccessibleCaretManager
   // input types such as touch.
   uint16_t mLastInputSource = nsIDOMMouseEvent::MOZ_SOURCE_UNKNOWN;
 
+  // Set to true in OnScrollStart() and set to false in OnScrollEnd().
+  bool mIsScrollStarted = false;
+
   static const int32_t kAutoScrollTimerDelay = 30;
 
   // Clicking on the boundary of input or textarea will move the caret to the

layout/base/gtest/TestAccessibleCaretEventHub.cpp

@@ -738,7 +738,7 @@ AccessibleCaretEventHubTester::TestEventDrivenAsyncPanZoomScroll(
   EXPECT_EQ(mHub->GetState(), MockAccessibleCaretEventHub::NoActionState());
 }
 
-TEST_F(AccessibleCaretEventHubTester, TestNoEventAsyncPanZoomScroll)
+TEST_F(AccessibleCaretEventHubTester, TestAsyncPanZoomScroll)
 {
   MockFunction<void(::std::string aCheckPointName)> check;
   {
@@ -748,7 +748,7 @@ TEST_F(AccessibleCaretEventHubTester, TestNoEventAsyncPanZoomScroll)
     EXPECT_CALL(*mHub->GetMockAccessibleCaretManager(), OnScrollStart());
 
     EXPECT_CALL(*mHub->GetMockAccessibleCaretManager(),
-                OnScrollPositionChanged()).Times(0);
+                OnScrollPositionChanged()).Times(2);
 
     EXPECT_CALL(check, Call("2"));
     EXPECT_CALL(*mHub->GetMockAccessibleCaretManager(), OnScrollEnd());

layout/base/gtest/TestAccessibleCaretManager.cpp

@@ -453,6 +453,9 @@ TEST_F(AccessibleCaretManagerTester, TestScrollInSelectionModeWithAlwaysTiltPref
                   CaretChangedReason::Scroll));
     EXPECT_CALL(check, Call("scrollstart1"));
 
+    EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(_)).Times(0);
+    EXPECT_CALL(check, Call("scrollPositionChanged1"));
+
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("reflow1"));
@@ -470,6 +473,9 @@ TEST_F(AccessibleCaretManagerTester, TestScrollInSelectionModeWithAlwaysTiltPref
                   CaretChangedReason::Scroll));
     EXPECT_CALL(check, Call("scrollstart2"));
 
+    EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(_)).Times(0);
+    EXPECT_CALL(check, Call("scrollPositionChanged2"));
+
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("reflow2"));
@@ -490,6 +496,11 @@ TEST_F(AccessibleCaretManagerTester, TestScrollInSelectionModeWithAlwaysTiltPref
   EXPECT_EQ(SecondCaretAppearance(), Appearance::Right);
   check.Call("scrollstart1");
 
+  mManager.OnScrollPositionChanged();
+  EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
+  EXPECT_EQ(SecondCaretAppearance(), Appearance::Right);
+  check.Call("scrollPositionChanged1");
+
   mManager.OnReflow();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
   EXPECT_EQ(SecondCaretAppearance(), Appearance::Right);
@@ -505,6 +516,11 @@ TEST_F(AccessibleCaretManagerTester, TestScrollInSelectionModeWithAlwaysTiltPref
   EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
   check.Call("scrollstart2");
 
+  mManager.OnScrollPositionChanged();
+  EXPECT_EQ(FirstCaretAppearance(), Appearance::Left);
+  EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
+  check.Call("scrollPositionChanged2");
+
   mManager.OnReflow();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Left);
   EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
@@ -795,13 +811,15 @@ TEST_F(AccessibleCaretManagerTester,
   // Scroll the caret into the viewport.
   mManager.OnScrollStart();
   check.Call("longtap scrollstart2");
+  mManager.OnScrollPositionChanged();
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
   check.Call("longtap scrollend2");
 
   // Scroll the caret within the viewport.
   mManager.OnScrollStart();
   check.Call("longtap scrollstart3");
+  mManager.OnScrollPositionChanged();
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
   check.Call("longtap scrollend3");