Backed out changeset bc3e37b63def (bug 1246918) for fix frequent android c1 test failures
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Wed, 17 Feb 2016 14:35:10 +0100
changeset 320854 709f559b5406e8555cf84dd09bdc747b076f142c
parent 320853 7ced13d644be8b05f0de4beeaaa3458910eb2ad6
child 320855 0629918a09ae87808efdda432d7852371ba37db6
child 320871 5ce212e97896db95fea3a47a9676c5c7a3033fca
child 320924 428c9570954f13e57f4bf931ed847390499009a8
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1246918
milestone47.0a1
backs outbc3e37b63defca87d0de165fe167ef7f8a7db651
first release with
nightly linux32
709f559b5406 / 47.0a1 / 20160217062621 / files
nightly linux64
709f559b5406 / 47.0a1 / 20160217062621 / files
nightly mac
709f559b5406 / 47.0a1 / 20160217062621 / files
nightly win32
709f559b5406 / 47.0a1 / 20160217062621 / files
nightly win64
709f559b5406 / 47.0a1 / 20160217062621 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset bc3e37b63def (bug 1246918) for fix frequent android c1 test failures
layout/base/AccessibleCaretManager.cpp
layout/base/AccessibleCaretManager.h
layout/base/gtest/TestAccessibleCaretManager.cpp
mobile/android/app/mobile.js
--- a/layout/base/AccessibleCaretManager.cpp
+++ b/layout/base/AccessibleCaretManager.cpp
@@ -197,17 +197,17 @@ AccessibleCaretManager::UpdateCarets(Upd
   switch (mLastUpdateCaretMode) {
   case CaretMode::None:
     HideCarets();
     break;
   case CaretMode::Cursor:
     UpdateCaretsForCursorMode(aHint);
     break;
   case CaretMode::Selection:
-    UpdateCaretsForSelectionMode(aHint);
+    UpdateCaretsForSelectionMode();
     break;
   }
 }
 
 bool
 AccessibleCaretManager::IsCaretDisplayableInCursorMode(nsIFrame** aOutFrame,
                                                        int32_t* aOutOffset) const
 {
@@ -311,54 +311,45 @@ AccessibleCaretManager::UpdateCaretsForC
 
   if ((result != PositionChangedResult::NotChanged || oldSecondCaretVisible) &&
       !mActiveCaret) {
     DispatchCaretStateChangedEvent(CaretChangedReason::Updateposition);
   }
 }
 
 void
-AccessibleCaretManager::UpdateCaretsForSelectionMode(UpdateCaretsHint aHint)
+AccessibleCaretManager::UpdateCaretsForSelectionMode()
 {
   AC_LOG("%s: selection: %p", __FUNCTION__, GetSelection());
 
   int32_t startOffset = 0;
   nsIFrame* startFrame = FindFirstNodeWithFrame(false, &startOffset);
 
   int32_t endOffset = 0;
   nsIFrame* endFrame = FindFirstNodeWithFrame(true, &endOffset);
 
   if (!CompareTreePosition(startFrame, endFrame)) {
     // XXX: Do we really have to hide carets if this condition isn't satisfied?
     HideCarets();
     return;
   }
 
-  auto updateSingleCaret = [aHint](AccessibleCaret* aCaret, nsIFrame* aFrame,
-                                   int32_t aOffset) -> PositionChangedResult
+  auto updateSingleCaret = [](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:
-        switch (aHint) {
-          case UpdateCaretsHint::Default:
-            aCaret->SetAppearance(Appearance::Normal);
-            break;
-
-          case UpdateCaretsHint::RespectOldAppearance:
-            // Do nothing to preserve the appearance of the caret set by the
-            // caller.
-            break;
-        }
+        aCaret->SetAppearance(Appearance::Normal);
         break;
 
       case PositionChangedResult::Invisible:
         aCaret->SetAppearance(Appearance::NormalNotShown);
         break;
     }
     return result;
   };
@@ -369,21 +360,17 @@ AccessibleCaretManager::UpdateCaretsForS
     updateSingleCaret(mSecondCaret.get(), endFrame, endOffset);
 
   if (firstCaretResult == PositionChangedResult::Changed ||
       secondCaretResult == PositionChangedResult::Changed) {
     // Flush layout to make the carets intersection correct.
     FlushLayout();
   }
 
-  if (aHint == UpdateCaretsHint::Default) {
-    // Only check for tilt carets with default update hint. Otherwise we might
-    // override the appearance set by the caller.
-    UpdateCaretsForTilt();
-  }
+  UpdateCaretsForTilt();
 
   if (!mActiveCaret) {
     DispatchCaretStateChangedEvent(CaretChangedReason::Updateposition);
   }
 }
 
 void
 AccessibleCaretManager::UpdateCaretsForTilt()
@@ -568,46 +555,40 @@ AccessibleCaretManager::SelectWordOrShor
   return rv;
 }
 
 void
 AccessibleCaretManager::OnScrollStart()
 {
   AC_LOG("%s", __FUNCTION__);
 
-  mFirstCaretAppearanceOnScrollStart = mFirstCaret->GetAppearance();
-  mSecondCaretAppearanceOnScrollStart = mSecondCaret->GetAppearance();
+  if (GetCaretMode() == CaretMode::Cursor) {
+    mFirstCaretAppearanceOnScrollStart = mFirstCaret->GetAppearance();
+  }
 
   // Hide the carets. (Extended visibility makes them "NormalNotShown").
   if (sCaretsExtendedVisibility) {
     DoNotShowCarets();
   } else {
     HideCarets();
   }
 }
 
 void
 AccessibleCaretManager::OnScrollEnd()
 {
   if (mLastUpdateCaretMode != GetCaretMode()) {
     return;
   }
 
-  mFirstCaret->SetAppearance(mFirstCaretAppearanceOnScrollStart);
-  mSecondCaret->SetAppearance(mSecondCaretAppearanceOnScrollStart);
-
-  // Flush layout to make the carets intersection correct since we turn the
-  // appearance of the carets from None or NormalNotShown into something
-  // visible.
-  FlushLayout();
-
   if (GetCaretMode() == CaretMode::Cursor) {
+    mFirstCaret->SetAppearance(mFirstCaretAppearanceOnScrollStart);
     if (!mFirstCaret->IsLogicallyVisible()) {
-      // If the caret is hidden (Appearance::None) due to timeout or blur, no
-      // need to update it.
+      // If the caret is hide (Appearance::None) due to timeout or blur, no need
+      // to update it.
       return;
     }
   }
 
   AC_LOG("%s: UpdateCarets()", __FUNCTION__);
   UpdateCarets();
 }
 
--- a/layout/base/AccessibleCaretManager.h
+++ b/layout/base/AccessibleCaretManager.h
@@ -130,17 +130,17 @@ protected:
   // Force hiding all carets regardless of the current selection status.
   void HideCarets();
 
   // Force carets to be "present" logically, but not visible. Allows ActionBar
   // to stay open when carets visibility is supressed during scroll.
   void DoNotShowCarets();
 
   void UpdateCaretsForCursorMode(UpdateCaretsHint aHint);
-  void UpdateCaretsForSelectionMode(UpdateCaretsHint aHint);
+  void UpdateCaretsForSelectionMode();
 
   // Provide haptic / touch feedback, primarily for select on longpress.
   void ProvideHapticFeedback();
 
   // Get the nearest enclosing focusable frame of aFrame.
   // @return focusable frame if there is any; nullptr otherwise.
   nsIFrame* GetFocusableFrame(nsIFrame* aFrame) const;
 
@@ -229,22 +229,20 @@ protected:
 
   // The timer for hiding the caret in cursor mode after timeout behind the
   // preference "layout.accessiblecaret.timeout_ms".
   nsCOMPtr<nsITimer> mCaretTimeoutTimer;
 
   // The caret mode since last update carets.
   CaretMode mLastUpdateCaretMode = CaretMode::None;
 
-  // Store the appearance of the carets when calling OnScrollStart() so that it
-  // can be restored in OnScrollEnd().
+  // Store the appearance of the first caret when calling OnScrollStart so that
+  // it can be restored in OnScrollEnd.
   AccessibleCaret::Appearance mFirstCaretAppearanceOnScrollStart =
                                  AccessibleCaret::Appearance::None;
-  AccessibleCaret::Appearance mSecondCaretAppearanceOnScrollStart =
-                                 AccessibleCaret::Appearance::None;
 
   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;
@@ -255,17 +253,17 @@ protected:
 
   // Preference to show caret in cursor mode when long tapping on an empty
   // content. This also changes the default update behavior in cursor mode,
   // which is based on the emptiness of the content, into something more
   // heuristic. See UpdateCaretsForCursorMode() for the details.
   static bool sCaretShownWhenLongTappingOnEmptyContent;
 
   // Android specific visibility extensions correct compatibility issues
-  // with ActionBar visibility during page scroll.
+  // with caret-drag and ActionBar visibility during page scroll.
   static bool sCaretsExtendedVisibility;
 
   // By default, javascript content selection changes closes AccessibleCarets and
   // UI interactions. Optionally, we can try to maintain the active UI, keeping
   // carets and ActionBar available.
   static bool sCaretsScriptUpdates;
 
   // AccessibleCaret pref for haptic feedback behaviour on longPress.
--- a/layout/base/gtest/TestAccessibleCaretManager.cpp
+++ b/layout/base/gtest/TestAccessibleCaretManager.cpp
@@ -55,17 +55,16 @@ public:
 
   class MockAccessibleCaretManager : public AccessibleCaretManager
   {
   public:
     using CaretMode = AccessibleCaretManager::CaretMode;
     using AccessibleCaretManager::UpdateCarets;
     using AccessibleCaretManager::HideCarets;
     using AccessibleCaretManager::sCaretShownWhenLongTappingOnEmptyContent;
-    using AccessibleCaretManager::sCaretsExtendedVisibility;
 
     MockAccessibleCaretManager()
       : AccessibleCaretManager(nullptr)
     {
       mFirstCaret = MakeUnique<MockAccessibleCaret>();
       mSecondCaret = MakeUnique<MockAccessibleCaret>();
     }
 
@@ -340,166 +339,52 @@ TEST_F(AccessibleCaretManagerTester, Tes
     .WillRepeatedly(Return(CaretMode::Selection));
 
   MockFunction<void(std::string aCheckPointName)> check;
   {
     InSequence dummy;
 
     // Initially, first caret is out of scrollport, and second caret is visible.
     EXPECT_CALL(mManager.FirstCaret(), SetPosition(_, _))
-      .WillOnce(Return(PositionChangedResult::Invisible));
+      .WillRepeatedly(Return(PositionChangedResult::Invisible));
+    EXPECT_CALL(mManager.SecondCaret(), SetPosition(_, _))
+      .WillRepeatedly(Return(PositionChangedResult::Changed));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Updateposition));
+                  CaretChangedReason::Updateposition)).Times(1);
     EXPECT_CALL(check, Call("updatecarets"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Visibilitychange));
-    EXPECT_CALL(check, Call("scrollstart1"));
+                  CaretChangedReason::Visibilitychange)).Times(1);
+    EXPECT_CALL(check, Call("scrollstart"));
 
     // After scroll ended, first caret is visible and second caret is out of
     // scroll port.
+    EXPECT_CALL(mManager.FirstCaret(), SetPosition(_, _))
+      .WillRepeatedly(Return(PositionChangedResult::Changed));
     EXPECT_CALL(mManager.SecondCaret(), SetPosition(_, _))
-      .WillOnce(Return(PositionChangedResult::Invisible));
-
-    EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Updateposition));
-    EXPECT_CALL(check, Call("scrollend1"));
+      .WillRepeatedly(Return(PositionChangedResult::Invisible));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Visibilitychange));
-    EXPECT_CALL(check, Call("scrollstart2"));
-
-    // After the scroll ended, both carets are visible.
-    EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Updateposition));
-    EXPECT_CALL(check, Call("scrollend2"));
+                  CaretChangedReason::Updateposition)).Times(1);
   }
 
   mManager.UpdateCarets();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
   EXPECT_EQ(SecondCaretAppearance(), Appearance::Normal);
   check.Call("updatecarets");
 
   mManager.OnScrollStart();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
   EXPECT_EQ(SecondCaretAppearance(), Appearance::None);
-  check.Call("scrollstart1");
-
-  mManager.OnReflow();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::None);
+  check.Call("scrollstart");
 
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
   EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
-  check.Call("scrollend1");
-
-  mManager.OnScrollStart();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::None);
-  check.Call("scrollstart2");
-
-  mManager.OnReflow();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::None);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::None);
-
-  mManager.OnScrollEnd();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::Normal);
-  check.Call("scrollend2");
-}
-
-TEST_F(AccessibleCaretManagerTester,
-       TestScrollInSelectionModeWithExtendedVisibility)
-{
-  EXPECT_CALL(mManager, GetCaretMode())
-    .WillRepeatedly(Return(CaretMode::Selection));
-
-  MockFunction<void(std::string aCheckPointName)> check;
-  {
-    InSequence dummy;
-
-    // Initially, first caret is out of scrollport, and second caret is visible.
-    EXPECT_CALL(mManager.FirstCaret(), SetPosition(_, _))
-      .WillOnce(Return(PositionChangedResult::Invisible));
-
-    EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Updateposition));
-    EXPECT_CALL(check, Call("updatecarets"));
-
-    EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Visibilitychange));
-    EXPECT_CALL(check, Call("scrollstart1"));
-
-    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));
-
-    EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Updateposition));
-    EXPECT_CALL(check, Call("scrollend1"));
-
-    EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
-                  CaretChangedReason::Visibilitychange));
-    EXPECT_CALL(check, Call("scrollstart2"));
-
-    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"));
-  }
-
-  AutoRestore<bool> savePref(
-    MockAccessibleCaretManager::sCaretsExtendedVisibility);
-  MockAccessibleCaretManager::sCaretsExtendedVisibility = true;
-
-  mManager.UpdateCarets();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::Normal);
-  check.Call("updatecarets");
-
-  mManager.OnScrollStart();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
-  check.Call("scrollstart1");
-
-  mManager.OnReflow();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
-  check.Call("reflow1");
-
-  mManager.OnScrollEnd();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
-  check.Call("scrollend1");
-
-  mManager.OnScrollStart();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
-  check.Call("scrollstart2");
-
-  mManager.OnReflow();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::NormalNotShown);
-  check.Call("reflow2");
-
-  mManager.OnScrollEnd();
-  EXPECT_EQ(FirstCaretAppearance(), Appearance::Normal);
-  EXPECT_EQ(SecondCaretAppearance(), Appearance::Normal);
-  check.Call("scrollend2");
 }
 
 TEST_F(AccessibleCaretManagerTester, TestScrollInCursorModeWhenLogicallyVisible)
 {
   EXPECT_CALL(mManager, GetCaretMode())
     .WillRepeatedly(Return(CaretMode::Cursor));
 
   EXPECT_CALL(mManager, HasNonEmptyTextContent(_))
@@ -647,17 +532,17 @@ TEST_F(AccessibleCaretManagerTester, Tes
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                    CaretChangedReason::Visibilitychange));
     EXPECT_CALL(check, Call("scrollstart3"));
 
     EXPECT_CALL(mManager, DispatchCaretStateChangedEvent(
                    CaretChangedReason::Updateposition));
     EXPECT_CALL(check, Call("scrollend3"));
-  }
+}
 
   // Simulate a single tap on an empty content.
   mManager.UpdateCarets();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
   check.Call("updatecarets");
 
   // Scroll the caret to be out of the viewport.
   mManager.OnScrollStart();
@@ -676,16 +561,17 @@ TEST_F(AccessibleCaretManagerTester, Tes
   // Scroll the caret within the viewport.
   mManager.OnScrollStart();
   check.Call("scrollstart3");
   mManager.OnScrollEnd();
   EXPECT_EQ(FirstCaretAppearance(), Appearance::NormalNotShown);
   check.Call("scrollend3");
 }
 
+
 TEST_F(AccessibleCaretManagerTester,
        TestScrollInCursorModeOnEmptyContentWithSpecialPreference)
 {
   EXPECT_CALL(mManager, GetCaretMode())
     .WillRepeatedly(Return(CaretMode::Cursor));
 
   EXPECT_CALL(mManager, HasNonEmptyTextContent(_))
     .WillRepeatedly(Return(false));
--- a/mobile/android/app/mobile.js
+++ b/mobile/android/app/mobile.js
@@ -950,18 +950,18 @@ pref("layout.accessiblecaret.caret_shown
 
 // Android needs persistent carets and actionbar. Turn off the caret timeout.
 pref("layout.accessiblecaret.timeout_ms", 0);
 
 // Android generates long tap (mouse) events.
 pref("layout.accessiblecaret.use_long_tap_injector", false);
 
 // AccessibleCarets behaviour is extended to support Android specific
-// requirements to hide carets while maintaining ActionBar visiblity during page
-// scroll.
+// requirements during caret-drag, tapping into empty inputs, and to
+// hide carets while maintaining ActionBar visiblity during page scroll.
 pref("layout.accessiblecaret.extendedvisibility", true);
 
 // Selection change notifications generated by Javascript changes
 // update active AccessibleCarets / UI interactions.
 pref("layout.accessiblecaret.allow_script_change_updates", true);
 
 // Optionally provide haptic feedback on longPress selection events.
 pref("layout.accessiblecaret.hapticfeedback", true);