Bug 1466208 - part 38: Create PresShell::EventHandler::PrepareToDispatchContextMenuEvent() r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 08 Mar 2019 23:41:30 +0000
changeset 521266 e0c39e6d8d2c77b51118d388515c37628277f2a6
parent 521265 5bc7479ef4a1d60e014bfe4759374d2d00e53ffb
child 521267 3c4b55694127237c3dd740d5ed055959c4d783fe
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1466208
milestone67.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 1466208 - part 38: Create PresShell::EventHandler::PrepareToDispatchContextMenuEvent() r=smaug The first switch statement of `PresShell::EventHandler::HandleEventInternal()` has 2 jobs: - Prepare something for specific event type. - Record the preparation time of some types of events to telemetry. This intermixed code is not easy to understand and somebody may add new preparation after recording them. So, even though the preparation time becomes worse a couple of nanoseconds, we should split those jobs. The patch moves the latter job into the new method. Differential Revision: https://phabricator.services.mozilla.com/D21339
layout/base/PresShell.cpp
layout/base/PresShell.h
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -7703,91 +7703,55 @@ nsresult PresShell::EventHandler::Handle
             //     even though it will be dispatched only into chrome.
             aEvent->PreventDefaultBeforeDispatch(CrossProcessForwarding::eStop);
             aEvent->mFlags.mOnlyChromeDispatch = true;
             if (aEvent->mMessage == eKeyUp) {
               Document::UnlockPointer();
             }
           }
         }
-        if (keyCode != NS_VK_ESCAPE && keyCode != NS_VK_SHIFT &&
+        // Allow keys other than ESC and modifiers be marked as a
+        // valid user input for triggering popup, fullscreen, and
+        // pointer lock.
+        isHandlingUserInput =
+            keyCode != NS_VK_ESCAPE && keyCode != NS_VK_SHIFT &&
             keyCode != NS_VK_CONTROL && keyCode != NS_VK_ALT &&
-            keyCode != NS_VK_WIN && keyCode != NS_VK_META) {
-          // Allow keys other than ESC and modifiers be marked as a
-          // valid user input for triggering popup, fullscreen, and
-          // pointer lock.
-          isHandlingUserInput = true;
-          GetPresContext()->RecordInteractionTime(
-              nsPresContext::InteractionType::eKeyInteraction,
-              aEvent->mTimeStamp);
-        }
-
-        Telemetry::AccumulateTimeDelta(
-            Telemetry::INPUT_EVENT_QUEUED_KEYBOARD_MS, aEvent->mTimeStamp);
+            keyCode != NS_VK_WIN && keyCode != NS_VK_META;
         break;
       }
       case eMouseDown:
       case eMouseUp:
-        Telemetry::AccumulateTimeDelta(Telemetry::INPUT_EVENT_QUEUED_CLICK_MS,
-                                       aEvent->mTimeStamp);
-        MOZ_FALLTHROUGH;
       case ePointerDown:
       case ePointerUp:
         isHandlingUserInput = true;
-        GetPresContext()->RecordInteractionTime(
-            nsPresContext::InteractionType::eClickInteraction,
-            aEvent->mTimeStamp);
         break;
 
       case eMouseMove:
-        if (aEvent->mFlags.mHandledByAPZ) {
-          Telemetry::AccumulateTimeDelta(
-              Telemetry::INPUT_EVENT_QUEUED_APZ_MOUSE_MOVE_MS,
-              aEvent->mTimeStamp);
-        }
-
         nsIPresShell::AllowMouseCapture(
             EventStateManager::GetActiveEventStateManager() == manager);
-
-        GetPresContext()->RecordInteractionTime(
-            nsPresContext::InteractionType::eMouseMoveInteraction,
-            aEvent->mTimeStamp);
         break;
 
       case eDrop: {
         nsCOMPtr<nsIDragSession> session = nsContentUtils::GetDragSession();
         if (session) {
           bool onlyChromeDrop = false;
           session->GetOnlyChromeDrop(&onlyChromeDrop);
           if (onlyChromeDrop) {
             aEvent->mFlags.mOnlyChromeDispatch = true;
           }
         }
         break;
       }
 
-      case eWheel:
-        if (aEvent->mFlags.mHandledByAPZ) {
-          Telemetry::AccumulateTimeDelta(
-              Telemetry::INPUT_EVENT_QUEUED_APZ_WHEEL_MS, aEvent->mTimeStamp);
-        }
-        break;
-
-      case eTouchMove:
-        if (aEvent->mFlags.mHandledByAPZ) {
-          Telemetry::AccumulateTimeDelta(
-              Telemetry::INPUT_EVENT_QUEUED_APZ_TOUCH_MOVE_MS,
-              aEvent->mTimeStamp);
-        }
-        break;
-
       default:
         break;
     }
 
+    RecordEventPreparationPerformance(aEvent);
+
     if (!mPresShell->mTouchManager.PreHandleEvent(
             aEvent, aEventStatus, touchIsNew, isHandlingUserInput,
             mPresShell->mCurrentEventContent)) {
       return NS_OK;
     }
   }
 
   // If we cannot open context menu even though eContextMenu is fired, we
@@ -7947,16 +7911,85 @@ bool PresShell::EventHandler::PrepareToD
   // of not cancelable.
   if (mouseEvent->IsShift()) {
     aEvent->mFlags.mOnlyChromeDispatch = true;
     aEvent->mFlags.mRetargetToNonNativeAnonymous = true;
   }
   return true;
 }
 
+void PresShell::EventHandler::RecordEventPreparationPerformance(
+    const WidgetEvent* aEvent) {
+  MOZ_ASSERT(aEvent);
+
+  switch (aEvent->mMessage) {
+    case eKeyPress:
+    case eKeyDown:
+    case eKeyUp:
+      switch (aEvent->AsKeyboardEvent()->mKeyCode) {
+        case NS_VK_ESCAPE:
+        case NS_VK_SHIFT:
+        case NS_VK_CONTROL:
+        case NS_VK_ALT:
+        case NS_VK_WIN:
+        case NS_VK_META:
+          break;
+        default:
+          GetPresContext()->RecordInteractionTime(
+              nsPresContext::InteractionType::eKeyInteraction,
+              aEvent->mTimeStamp);
+          break;
+      }
+      Telemetry::AccumulateTimeDelta(Telemetry::INPUT_EVENT_QUEUED_KEYBOARD_MS,
+                                     aEvent->mTimeStamp);
+      return;
+
+    case eMouseDown:
+    case eMouseUp:
+      Telemetry::AccumulateTimeDelta(Telemetry::INPUT_EVENT_QUEUED_CLICK_MS,
+                                     aEvent->mTimeStamp);
+      MOZ_FALLTHROUGH;
+    case ePointerDown:
+    case ePointerUp:
+      GetPresContext()->RecordInteractionTime(
+          nsPresContext::InteractionType::eClickInteraction,
+          aEvent->mTimeStamp);
+      return;
+
+    case eMouseMove:
+      if (aEvent->mFlags.mHandledByAPZ) {
+        Telemetry::AccumulateTimeDelta(
+            Telemetry::INPUT_EVENT_QUEUED_APZ_MOUSE_MOVE_MS,
+            aEvent->mTimeStamp);
+      }
+      GetPresContext()->RecordInteractionTime(
+          nsPresContext::InteractionType::eMouseMoveInteraction,
+          aEvent->mTimeStamp);
+      return;
+
+    case eWheel:
+      if (aEvent->mFlags.mHandledByAPZ) {
+        Telemetry::AccumulateTimeDelta(
+            Telemetry::INPUT_EVENT_QUEUED_APZ_WHEEL_MS, aEvent->mTimeStamp);
+      }
+      return;
+
+    case eTouchMove:
+      if (aEvent->mFlags.mHandledByAPZ) {
+        Telemetry::AccumulateTimeDelta(
+            Telemetry::INPUT_EVENT_QUEUED_APZ_TOUCH_MOVE_MS,
+            aEvent->mTimeStamp);
+      }
+      return;
+
+    default:
+      return;
+  }
+}
+
 void PresShell::EventHandler::RecordEventHandlingResponsePerformance(
     const WidgetEvent* aEvent) {
   if (!Telemetry::CanRecordBase() || aEvent->mTimeStamp.IsNull() ||
       aEvent->mTimeStamp <= mPresShell->mLastOSWake ||
       !aEvent->AsInputEvent()) {
     return;
   }
 
--- a/layout/base/PresShell.h
+++ b/layout/base/PresShell.h
@@ -1064,16 +1064,25 @@ class PresShell final : public nsIPresSh
 
      private:
       const EventHandler& mEventHandler;
       const WidgetEvent* mEvent;
       TimeStamp mHandlingStartTime;
     };
 
     /**
+     * RecordEventPreparationPerformance() records event preparation performance
+     * with telemetry.
+     *
+     * @param aEvent            The handling event which we've finished
+     *                          preparing something to dispatch.
+     */
+    void RecordEventPreparationPerformance(const WidgetEvent* aEvent);
+
+    /**
      * RecordEventHandlingResponsePerformance() records event handling response
      * performance with telemetry.
      *
      * @param aEvent            The handled event.
      */
     void RecordEventHandlingResponsePerformance(const WidgetEvent* aEvent);
 
     /**