Bug 1383816 - Adds Variant in implementation of FocusTarget and removes union; r?botond draft
authorJeff Hajewski <jeff.hajewski@gmail.com>
Sun, 03 Sep 2017 08:35:08 -0500
changeset 661707 6ca1c580ddeaee6ed9ce039906aa1d5b02649357
parent 661706 69fd247df26427afe8921b4732f00ef663647221
child 661708 973750414eba2a6eb86d4eb5b47d1543d49dfdc6
push id78858
push userjeff.hajewski@gmail.com
push dateFri, 08 Sep 2017 22:55:28 +0000
reviewersbotond
bugs1383816
milestone57.0a1
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