Bug 1383816 - Adds Variant in implementation of FocusTarget and removes union; r=botond
authorJeff Hajewski <jeff.hajewski@gmail.com>
Sun, 03 Sep 2017 08:35:08 -0500
changeset 429322 a61e73b332e19c17ad424254aa1b6de6411b7a89
parent 429321 0e9b01271d17bbc4fb5e31071fd18f3d38e7279c
child 429323 7f209f4a6bf21c3c68e080c5e6305b8f64e6462c
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1383816
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1383816 - Adds Variant in implementation of FocusTarget and removes union; r=botond FocusTarget.h * Adds Variant property to FocusTarget * Adds RefLayerId typedef for uint64_t * Adds empty struct for the case where there is no focus target * Adds operator== for ScrollTargets struct * Updates mData from union to mozilla::variant * Removes FocusTargetType enum * Removes FocusTargetData union * Removes mType property FocusTarget.cpp * Updates methods using mData to use proper variant methods * Removes references to mType, instead using the appropriate variant methods MozReview-Commit-ID: BAarVxSGDtJ
gfx/layers/apz/src/FocusTarget.cpp
gfx/layers/apz/src/FocusTarget.h
--- a/gfx/layers/apz/src/FocusTarget.cpp
+++ b/gfx/layers/apz/src/FocusTarget.cpp
@@ -95,45 +95,44 @@ static bool
 IsEditableNode(nsINode* aNode)
 {
   return aNode && aNode->IsEditable();
 }
 
 FocusTarget::FocusTarget()
   : mSequenceNumber(0)
   , mFocusHasKeyEventListeners(false)
-  , mType(FocusTarget::eNone)
+  , mData(AsVariant(NoFocusTarget()))
 {
 }
 
 FocusTarget::FocusTarget(nsIPresShell* aRootPresShell,
                          uint64_t aFocusSequenceNumber)
   : mSequenceNumber(aFocusSequenceNumber)
   , mFocusHasKeyEventListeners(false)
+  , mData(AsVariant(NoFocusTarget()))
 {
   MOZ_ASSERT(aRootPresShell);
   MOZ_ASSERT(NS_IsMainThread());
 
   // Key events can be retargeted to a child PresShell when there is an iframe
   nsCOMPtr<nsIPresShell> presShell = GetRetargetEventPresShell(aRootPresShell);
 
   if (!presShell) {
     FT_LOG("Creating nil target with seq=%" PRIu64 " (can't find retargeted presshell)\n",
            aFocusSequenceNumber);
 
-    mType = FocusTarget::eNone;
     return;
   }
 
   nsCOMPtr<nsIDocument> document = presShell->GetDocument();
   if (!document) {
     FT_LOG("Creating nil target with seq=%" PRIu64 " (no document)\n",
            aFocusSequenceNumber);
 
-    mType = FocusTarget::eNone;
     return;
   }
 
   // Find the focused content and use it to determine whether there are key event
   // listeners or whether key events will be targeted at a different process
   // through a remote browser.
   nsCOMPtr<nsIContent> focusedContent = presShell->GetFocusedContentInOurWindow();
   nsCOMPtr<nsIContent> keyEventTarget = focusedContent;
@@ -155,96 +154,81 @@ FocusTarget::FocusTarget(nsIPresShell* a
   // Check if the key event target is content editable or if the document
   // is in design mode.
   if (IsEditableNode(keyEventTarget) ||
       IsEditableNode(document)) {
     FT_LOG("Creating nil target with seq=%" PRIu64 ", kl=%d (disabling for editable node)\n",
            aFocusSequenceNumber,
            static_cast<int>(mFocusHasKeyEventListeners));
 
-    mType = FocusTarget::eNone;
     return;
   }
 
   // Check if the key event target is a remote browser
   if (TabParent* browserParent = TabParent::GetFrom(keyEventTarget)) {
     RenderFrameParent* rfp = browserParent->GetRenderFrame();
 
     // The globally focused element for scrolling is in a remote layer tree
     if (rfp) {
       FT_LOG("Creating reflayer target with seq=%" PRIu64 ", kl=%d, lt=%" PRIu64 "\n",
              aFocusSequenceNumber,
              mFocusHasKeyEventListeners,
              rfp->GetLayersId());
 
-      mType = FocusTarget::eRefLayer;
-      mData.mRefLayerId = rfp->GetLayersId();
+      mData = AsVariant<RefLayerId>(rfp->GetLayersId());
       return;
     }
 
     FT_LOG("Creating nil target with seq=%" PRIu64 ", kl=%d (remote browser missing layers id)\n",
            aFocusSequenceNumber,
            mFocusHasKeyEventListeners);
 
-    mType = FocusTarget::eNone;
     return;
   }
 
   // The content to scroll is either the focused element or the focus node of
   // the selection. It's difficult to determine if an element is an interactive
   // element requiring async keyboard scrolling to be disabled. So we only
   // allow async key scrolling based on the selection, which doesn't have
   // this problem and is more common.
   if (focusedContent) {
     FT_LOG("Creating nil target with seq=%" PRIu64 ", kl=%d (disabling for focusing an element)\n",
            aFocusSequenceNumber,
            mFocusHasKeyEventListeners);
 
-    mType = FocusTarget::eNone;
     return;
   }
 
   nsCOMPtr<nsIContent> selectedContent = presShell->GetSelectedContentForScrolling();
 
   // Gather the scrollable frames that would be scrolled in each direction
   // for this scroll target
   nsIScrollableFrame* horizontal =
     presShell->GetScrollableFrameToScrollForContent(selectedContent.get(),
                                                     nsIPresShell::eHorizontal);
   nsIScrollableFrame* vertical =
     presShell->GetScrollableFrameToScrollForContent(selectedContent.get(),
                                                     nsIPresShell::eVertical);
 
   // We might have the globally focused element for scrolling. Gather a ViewID for
   // the horizontal and vertical scroll targets of this element.
-  mType = FocusTarget::eScrollLayer;
-  mData.mScrollTargets.mHorizontal =
-    nsLayoutUtils::FindIDForScrollableFrame(horizontal);
-  mData.mScrollTargets.mVertical =
-    nsLayoutUtils::FindIDForScrollableFrame(vertical);
+  ScrollTargets target;
+  target.mHorizontal =  nsLayoutUtils::FindIDForScrollableFrame(horizontal);
+  target.mVertical = nsLayoutUtils::FindIDForScrollableFrame(vertical);
+  mData = AsVariant(target);
 
   FT_LOG("Creating scroll target with seq=%" PRIu64 ", kl=%d, h=%" PRIu64 ", v=%" PRIu64 "\n",
          aFocusSequenceNumber,
          mFocusHasKeyEventListeners,
-         mData.mScrollTargets.mHorizontal,
-         mData.mScrollTargets.mVertical);
+         target.mHorizontal,
+         target.mVertical);
 }
 
 bool
 FocusTarget::operator==(const FocusTarget& aRhs) const
 {
-  if (mSequenceNumber != aRhs.mSequenceNumber ||
-      mFocusHasKeyEventListeners != aRhs.mFocusHasKeyEventListeners ||
-      mType != aRhs.mType) {
-    return false;
-  }
-
-  if (mType == FocusTarget::eRefLayer) {
-      return mData.mRefLayerId == aRhs.mData.mRefLayerId;
-  } else if (mType == FocusTarget::eScrollLayer) {
-      return mData.mScrollTargets.mHorizontal == aRhs.mData.mScrollTargets.mHorizontal &&
-             mData.mScrollTargets.mVertical == aRhs.mData.mScrollTargets.mVertical;
-  }
-  return true;
+  return mSequenceNumber == aRhs.mSequenceNumber &&
+         mFocusHasKeyEventListeners == aRhs.mFocusHasKeyEventListeners &&
+         mData == aRhs.mData;
 }
 
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/apz/src/FocusTarget.h
+++ b/gfx/layers/apz/src/FocusTarget.h
@@ -5,16 +5,17 @@
 
 #ifndef mozilla_layers_FocusTarget_h
 #define mozilla_layers_FocusTarget_h
 
 #include <stdint.h> // for int32_t, uint32_t
 
 #include "FrameMetrics.h"        // for FrameMetrics::ViewID
 #include "mozilla/DefineEnum.h"  // for MOZ_DEFINE_ENUM
+#include "mozilla/Variant.h"     // for Variant
 
 class nsIPresShell;
 
 namespace mozilla {
 namespace layers {
 
 /**
  * This class is used for communicating information about the currently focused
@@ -24,29 +25,33 @@ namespace layers {
  */
 class FocusTarget final
 {
 public:
   struct ScrollTargets
   {
     FrameMetrics::ViewID mHorizontal;
     FrameMetrics::ViewID mVertical;
+
+    bool operator==(const ScrollTargets& aRhs) const
+    {
+      return mHorizontal == aRhs.mHorizontal &&
+             mVertical == aRhs.mVertical;
+    }
   };
 
-  MOZ_DEFINE_ENUM_AT_CLASS_SCOPE(
-    FocusTargetType, (
-      eNone,
-      eRefLayer,
-      eScrollLayer
-  ));
+  typedef uint64_t RefLayerId;
 
-  union FocusTargetData
-  {
-    uint64_t      mRefLayerId;
-    ScrollTargets mScrollTargets;
+  // We need this to represent the case where mData has no focus target data
+  // because we can't have an empty variant
+  struct NoFocusTarget {
+    bool operator==(const NoFocusTarget& aRhs) const
+    {
+     return true;
+    }
   };
 
   FocusTarget();
 
   /**
    * Construct a focus target for the specified top level PresShell
    */
   FocusTarget(nsIPresShell* aRootPresShell,
@@ -57,16 +62,15 @@ public:
 public:
   // The content sequence number recorded at the time of this class's creation
   uint64_t mSequenceNumber;
 
   // Whether there are keydown, keypress, or keyup event listeners
   // in the event target chain of the focused element
   bool mFocusHasKeyEventListeners;
 
-  FocusTargetType mType;
-  FocusTargetData mData;
+  mozilla::Variant<RefLayerId, ScrollTargets, NoFocusTarget> mData;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif // mozilla_layers_FocusTarget_h