Bug 1273045 Part 2 - Update carets when scrolling in subframes without APZ. r?mtseng draft
authorTing-Yu Lin <tlin@mozilla.com>
Tue, 14 Mar 2017 17:50:03 +0800
changeset 498107 83e109ebb7ea8d54f01029f01384f9b42b94b18a
parent 498106 04ce7ba5114f8e7b5a8c61b760de30b6fbc730c1
child 549090 d66e9eea4186624796de21123db9316263ed137a
push id49117
push userbmo:tlin@mozilla.com
push dateTue, 14 Mar 2017 10:23:19 +0000
reviewersmtseng
bugs1273045
milestone55.0a1
Bug 1273045 Part 2 - Update carets when scrolling in subframes without 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
layout/base/AccessibleCaretManager.cpp
layout/base/AccessibleCaretManager.h
layout/base/gtest/TestAccessibleCaretEventHub.cpp
layout/base/gtest/TestAccessibleCaretManager.cpp
--- a/layout/base/AccessibleCaretEventHub.cpp
+++ b/layout/base/AccessibleCaretEventHub.cpp
@@ -244,16 +244,22 @@ class AccessibleCaretEventHub::ScrollSta
 public:
   virtual const char* Name() const override { return "ScrollState"; }
 
   virtual void OnScrollEnd(AccessibleCaretEventHub* aContext) override
   {
     aContext->SetState(aContext->PostScrollState());
   }
 
+  virtual void OnScrollPositionChanged(
+    AccessibleCaretEventHub* aContext) override
+  {
+    aContext->mManager->OnScrollPositionChanged();
+  }
+
   virtual void OnBlur(AccessibleCaretEventHub* aContext,
                       bool aIsLeavingDocument) override
   {
     aContext->mManager->OnBlur();
     if (aIsLeavingDocument) {
       aContext->SetState(aContext->NoActionState());
     }
   }
--- a/layout/base/AccessibleCaretManager.cpp
+++ b/layout/base/AccessibleCaretManager.cpp
@@ -58,16 +58,17 @@ operator<<(std::ostream& aStream,
 
 std::ostream& operator<<(std::ostream& aStream,
                          const AccessibleCaretManager::UpdateCaretsHint& aHint)
 {
   using UpdateCaretsHint = AccessibleCaretManager::UpdateCaretsHint;
   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;
 }
 #undef AC_PROCESS_ENUM_TO_STREAM
 
 /* static */ bool
 AccessibleCaretManager::sSelectionBarEnabled = false;
 /* static */ bool
@@ -290,24 +291,20 @@ AccessibleCaretManager::UpdateCaretsForC
 
   int32_t offset = 0;
   nsIFrame* frame = nullptr;
   if (!IsCaretDisplayableInCursorMode(&frame, &offset)) {
     HideCarets();
     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))) {
           mFirstCaret->SetAppearance(Appearance::Normal);
         } else if (sCaretShownWhenLongTappingOnEmptyContent) {
           if (mFirstCaret->IsLogicallyVisible()) {
             // Possible cases are: 1) SelectWordOrShortcut() sets the
             // appearance to Normal. 2) When the caret is out of viewport and
@@ -337,17 +334,17 @@ AccessibleCaretManager::UpdateCaretsForC
       break;
   }
 
   mFirstCaret->SetSelectionBarEnabled(false);
   mSecondCaret->SetAppearance(Appearance::None);
 
   LaunchCaretTimeoutTimer();
 
-  if ((result != PositionChangedResult::NotChanged || oldSecondCaretVisible) &&
+  if (!aHints.contains(UpdateCaretsHint::DispatchNoEvent) &&
       !mActiveCaret) {
     DispatchCaretStateChangedEvent(CaretChangedReason::Updateposition);
   }
 }
 
 void
 AccessibleCaretManager::UpdateCaretsForSelectionMode(UpdateCaretsHintSet aHints)
 {
@@ -370,19 +367,16 @@ AccessibleCaretManager::UpdateCaretsForS
   auto updateSingleCaret = [aHints](AccessibleCaret* aCaret, nsIFrame* aFrame,
                                     int32_t aOffset) -> PositionChangedResult
   {
     PositionChangedResult result = aCaret->SetPosition(aFrame, aOffset);
     aCaret->SetSelectionBarEnabled(sSelectionBarEnabled);
 
     switch (result) {
       case PositionChangedResult::NotChanged:
-        // Do nothing
-        break;
-
       case PositionChangedResult::Changed:
         if (aHints == UpdateCaretsHint::Default) {
           aCaret->SetAppearance(Appearance::Normal);
         } else if (aHints.contains(UpdateCaretsHint::RespectOldAppearance)) {
           // Do nothing to preserve the appearance of the caret set by the
           // caller.
         }
         break;
@@ -413,17 +407,18 @@ AccessibleCaretManager::UpdateCaretsForS
     // override the appearance set by the caller.
     if (sCaretsAlwaysTilt) {
       UpdateCaretsForAlwaysTilt(startFrame, endFrame);
     } else {
       UpdateCaretsForOverlappingTilt();
     }
   }
 
-  if (!mActiveCaret) {
+  if (!aHints.contains(UpdateCaretsHint::DispatchNoEvent) &&
+      !mActiveCaret) {
     DispatchCaretStateChangedEvent(CaretChangedReason::Updateposition);
   }
 }
 
 bool
 AccessibleCaretManager::UpdateCaretsForOverlappingTilt()
 {
   if (!mFirstCaret->IsVisuallyVisible() || !mSecondCaret->IsVisuallyVisible()) {
@@ -653,16 +648,18 @@ AccessibleCaretManager::SelectWordOrShor
   return rv;
 }
 
 void
 AccessibleCaretManager::OnScrollStart()
 {
   AC_LOG("%s", __FUNCTION__);
 
+  mIsScrollStarted = true;
+
   if (!sCaretsAlwaysShowWhenScrolling) {
     // Backup the appearance so that we can restore them after the scrolling
     // ends.
     mFirstCaretAppearanceOnScrollStart = mFirstCaret->GetAppearance();
     mSecondCaretAppearanceOnScrollStart = mSecondCaret->GetAppearance();
     HideCarets();
     return;
   }
@@ -676,16 +673,18 @@ AccessibleCaretManager::OnScrollStart()
 
 void
 AccessibleCaretManager::OnScrollEnd()
 {
   if (mLastUpdateCaretMode != GetCaretMode()) {
     return;
   }
 
+  mIsScrollStarted = false;
+
   if (!sCaretsAlwaysShowWhenScrolling) {
     // Restore the appearance which is saved before the scrolling is started.
     mFirstCaret->SetAppearance(mFirstCaretAppearanceOnScrollStart);
     mSecondCaret->SetAppearance(mSecondCaretAppearanceOnScrollStart);
   }
 
   if (GetCaretMode() == CaretMode::Cursor) {
     if (!mFirstCaret->IsLogicallyVisible()) {
@@ -710,18 +709,27 @@ AccessibleCaretManager::OnScrollEnd()
 void
 AccessibleCaretManager::OnScrollPositionChanged()
 {
   if (mLastUpdateCaretMode != GetCaretMode()) {
     return;
   }
 
   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);
+    }
   }
 }
 
 void
 AccessibleCaretManager::OnReflow()
 {
   if (mLastUpdateCaretMode != GetCaretMode()) {
     return;
--- a/layout/base/AccessibleCaretManager.h
+++ b/layout/base/AccessibleCaretManager.h
@@ -125,17 +125,21 @@ protected:
 
   enum class UpdateCaretsHint : uint8_t {
     // Update everything including appearance and position.
     Default,
 
     // 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>;
 
   friend std::ostream& operator<<(std::ostream& aStream,
                                   const UpdateCaretsHint& aResult);
 
   // Update carets based on current selection status. This function will flush
@@ -291,16 +295,19 @@ protected:
                                  AccessibleCaret::Appearance::None;
 
   // The last input source that the event hub saw. We use this to decide whether
   // or not show the carets when the selection is updated, as we want to hide
   // the carets for mouse-triggered selection changes but show them for other
   // 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
   // front or end of the content. To avoid this, we need to deflate the content
   // boundary by 61 app units, which is 1 pixel + 1 app unit as defined in
   // AppUnit.h.
   static const int32_t kBoundaryAppUnits = 61;
 
--- a/layout/base/gtest/TestAccessibleCaretEventHub.cpp
+++ b/layout/base/gtest/TestAccessibleCaretEventHub.cpp
@@ -733,27 +733,27 @@ AccessibleCaretEventHubTester::TestEvent
 
   check.Call("4");
 
   // Simulate scroll end fired by timer.
   MockAccessibleCaretEventHub::FireScrollEnd(nullptr, mHub);
   EXPECT_EQ(mHub->GetState(), MockAccessibleCaretEventHub::NoActionState());
 }
 
-TEST_F(AccessibleCaretEventHubTester, TestNoEventAsyncPanZoomScroll)
+TEST_F(AccessibleCaretEventHubTester, TestAsyncPanZoomScroll)
 {
   MockFunction<void(::std::string aCheckPointName)> check;
   {
     InSequence dummy;
 
     EXPECT_CALL(check, Call("1"));
     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());
   }
 
   check.Call("1");
 
   mHub->AsyncPanZoomStarted();
--- a/layout/base/gtest/TestAccessibleCaretManager.cpp
+++ b/layout/base/gtest/TestAccessibleCaretManager.cpp
@@ -448,16 +448,19 @@ TEST_F(AccessibleCaretManagerTester, Tes
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("updatecarets"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   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"));
 
     // After scroll ended, first caret is visible and second caret is out of
     // scroll port.
     EXPECT_CALL(mManager.SecondCaret(), SetPosition(_, _))
       .WillOnce(Return(PositionChangedResult::Invisible));
@@ -465,16 +468,19 @@ TEST_F(AccessibleCaretManagerTester, Tes
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("scrollend1"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   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"));
 
     // After the scroll ended, both carets are visible.
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                   CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("scrollend2"));
@@ -485,31 +491,41 @@ TEST_F(AccessibleCaretManagerTester, Tes
   EXPECT_EQ(SecondCaretAppearance(), Appearance::Right);
   check.Call("updatecarets");
 
   mManager.OnScrollStart();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
   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);
   check.Call("reflow1");
 
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Left);
   EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
   check.Call("scrollend1");
 
   mManager.OnScrollStart();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Left);
   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);
   check.Call("reflow2");
 
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Left);
   EXPECT_EQ(SecondCaretAppearance(), Appearance::Right);
@@ -790,21 +806,23 @@ TEST_F(AccessibleCaretManagerTester,
   check.Call("longtap scrollstart1");
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
   check.Call("longtap scrollend1");
 
   // 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");
 }
 
 } // namespace mozilla