Bug 1383816 - Updates FocusState to use variant matcher; r?botond draft
authorJeff Hajewski <jeff.hajewski@gmail.com>
Sat, 02 Sep 2017 14:05:16 -0500
changeset 661708 973750414eba2a6eb86d4eb5b47d1543d49dfdc6
parent 661707 6ca1c580ddeaee6ed9ce039906aa1d5b02649357
child 661709 f4ff720f709a5e51a9f15fce2a2efbd641b741f8
push id78858
push userjeff.hajewski@gmail.com
push dateFri, 08 Sep 2017 22:55:28 +0000
reviewersbotond
bugs1383816
milestone57.0a1
Bug 1383816 - Updates FocusState to use variant matcher; r?botond Updates FocusState::Update to use a matcher on the the mData variant. This replaces a switch statement that was used to unpack the mData union, prior to making mData a variant. FocusTargetDataMatcher has three match methods, which are largely unmodified from the three cases of the original switch statement. The constructor takes two arguments: a reference to |this| (FocusState) and a copy of the |sequenceNumber|. |sequenceNumber| was added to the constructor to avoid passing in a reference to |target|, since we only needed the |sequenceNumber| property from |target|. MozReview-Commit-ID: FkjQm8oGysM
gfx/layers/apz/src/FocusState.cpp
--- a/gfx/layers/apz/src/FocusState.cpp
+++ b/gfx/layers/apz/src/FocusState.cpp
@@ -69,63 +69,77 @@ FocusState::Update(uint64_t aRootLayerTr
       return;
     }
 
     const FocusTarget& target = currentNode->second;
 
     // Accumulate event listener flags on the path to the focus target
     mFocusHasKeyEventListeners |= target.mFocusHasKeyEventListeners;
 
-    switch (target.mType) {
-      case FocusTarget::eRefLayer: {
+    // Match on the data stored in mData
+    // The match functions return true or false depending on whether the
+    // enclosing method, FocusState::Update, should return or continue to the
+    // next iteration of the while loop, respectively.
+    struct FocusTargetDataMatcher {
+
+      FocusState& mFocusState;
+      const uint64_t mSequenceNumber;
+
+      bool match(const FocusTarget::NoFocusTarget& aNoFocusTarget) {
+        FS_LOG("Setting target to nil (reached a nil target)\n");
+
+        // Mark what sequence number this target has for debugging purposes so
+        // we can always accurately report on whether we are stale or not
+        mFocusState.mLastContentProcessedEvent = mSequenceNumber;
+        return true;
+      }
+
+      bool match(const FocusTarget::RefLayerId aRefLayerId) {
         // Guard against infinite loops
-        MOZ_ASSERT(mFocusLayersId != target.mData.mRefLayerId);
-        if (mFocusLayersId == target.mData.mRefLayerId) {
+        MOZ_ASSERT(mFocusState.mFocusLayersId != aRefLayerId);
+        if (mFocusState.mFocusLayersId == aRefLayerId) {
           FS_LOG("Setting target to nil (bailing out of infinite loop, lt=%" PRIu64 ")\n",
-                 mFocusLayersId);
-          return;
+                 mFocusState.mFocusLayersId);
+          return true;
         }
 
-        FS_LOG("Looking for target in lt=%" PRIu64 "\n", target.mData.mRefLayerId);
+        FS_LOG("Looking for target in lt=%" PRIu64 "\n", aRefLayerId);
 
         // The focus target is in a child layer tree
-        mFocusLayersId = target.mData.mRefLayerId;
-        break;
+        mFocusState.mFocusLayersId = aRefLayerId;
+        return false;
       }
-      case FocusTarget::eScrollLayer: {
+
+      bool match(const FocusTarget::ScrollTargets& aScrollTargets) {
         FS_LOG("Setting target to h=%" PRIu64 ", v=%" PRIu64 ", and seq=%" PRIu64 "\n",
-               target.mData.mScrollTargets.mHorizontal,
-               target.mData.mScrollTargets.mVertical,
-               target.mSequenceNumber);
+               aScrollTargets.mHorizontal,
+               aScrollTargets.mVertical,
+               mSequenceNumber);
 
         // This is the global focus target
-        mFocusHorizontalTarget = target.mData.mScrollTargets.mHorizontal;
-        mFocusVerticalTarget = target.mData.mScrollTargets.mVertical;
+        mFocusState.mFocusHorizontalTarget = aScrollTargets.mHorizontal;
+        mFocusState.mFocusVerticalTarget = aScrollTargets.mVertical;
 
         // Mark what sequence number this target has so we can determine whether
         // it is stale or not
-        mLastContentProcessedEvent = target.mSequenceNumber;
+        mFocusState.mLastContentProcessedEvent = mSequenceNumber;
 
         // If this focus state was just created and content has experienced more
         // events then us, then assume we were recreated and sync focus sequence
         // numbers.
-        if (mLastAPZProcessedEvent == 1 &&
-            mLastContentProcessedEvent > mLastAPZProcessedEvent) {
-          mLastAPZProcessedEvent = mLastContentProcessedEvent;
+        if (mFocusState.mLastAPZProcessedEvent == 1 &&
+            mFocusState.mLastContentProcessedEvent > mFocusState.mLastAPZProcessedEvent) {
+          mFocusState.mLastAPZProcessedEvent = mFocusState.mLastContentProcessedEvent;
         }
-        return;
+        return true;
       }
-      case FocusTarget::eNone: {
-        FS_LOG("Setting target to nil (reached a nil target)\n");
+    }; // struct FocusTargetDataMatcher
 
-        // Mark what sequence number this target has for debugging purposes so
-        // we can always accurately report on whether we are stale or not
-        mLastContentProcessedEvent = target.mSequenceNumber;
-        return;
-      }
+    if (target.mData.match(FocusTargetDataMatcher{*this, target.mSequenceNumber})) {
+      return;
     }
   }
 }
 
 std::unordered_set<uint64_t>
 FocusState::GetFocusTargetLayerIds() const
 {
   std::unordered_set<uint64_t> layersIds;