Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(175)

Unified Diff: third_party/WebKit/Source/core/events/TouchEvent.cpp

Issue 1864523004: Add UMA metric for tracking root level listeners for blocking touch. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/events/TouchEvent.cpp
diff --git a/third_party/WebKit/Source/core/events/TouchEvent.cpp b/third_party/WebKit/Source/core/events/TouchEvent.cpp
index 085c275c4bc8ed59f5f39a3945d3dc9e80d90be9..d8b89c9fcf55d8b3bb47d117d41105f6c375887c 100644
--- a/third_party/WebKit/Source/core/events/TouchEvent.cpp
+++ b/third_party/WebKit/Source/core/events/TouchEvent.cpp
@@ -28,13 +28,53 @@
#include "bindings/core/v8/DOMWrapperWorld.h"
#include "bindings/core/v8/ScriptState.h"
+#include "core/EventTypeNames.h"
#include "core/events/EventDispatcher.h"
#include "core/frame/FrameConsole.h"
+#include "core/frame/FrameView.h"
#include "core/frame/LocalDOMWindow.h"
+#include "core/html/HTMLElement.h"
tdresser 2016/04/07 12:22:07 Is this used?
dtapuska 2016/04/19 17:54:46 Done.
#include "core/inspector/ConsoleMessage.h"
+#include "platform/Histogram.h"
namespace blink {
+namespace {
+
+const size_t kRootScrollerOffset = 4;
tdresser 2016/04/07 12:22:07 Add a comment indicating what these are offsets in
dtapuska 2016/04/19 17:54:46 Done.
+const size_t kScrollableDocumentOffset = 2;
+const size_t kHandledOffset = 1;
+
+enum ListenerHistogram {
+ NonRootScrollerNonScrollableNotHandled, // Non-root-scroller, non-scrollable document, not handled.
+ NonRootScrollerNonScrollableHandled, // Non-root-scroller, non-scrollable document, handled application.
+ NonRootScrollerScrollableDocumentNotHandled, // Non-root-scroller, scrollable document, not handled.
+ NonRootScrollerScrollableDocumentHandled, // Non-root-scroller, scrollable document, handled application.
+ RootScrollerNonScrollableNotHandled, // Root-scroller, non-scrollable document, not handled.
+ RootScrollerNonScrollableHandled, // Root-scroller, non-scrollable document, handled.
+ RootScrollerScrollableDocumentNotHandled, // Root-scroller, scrollable document, not handled.
+ RootScrollerScrollableDocumentHandled, // Root-scroller, scrollable document, handled.
+ ListenerHistogramMax,
+};
+
+ListenerHistogram toListenerHistogramValue(const Node& node, DispatchEventResult dispatchResult)
+{
+ int result = 0;
+ if (node.isDocumentNode() || static_cast<Node*>(node.document().documentElement()) == &node || static_cast<Node*>(node.document().body()) == &node) {
tdresser 2016/04/07 12:22:08 We need to check |window| too, don't we? Add a co
dtapuska 2016/04/19 17:54:47 Done.
+ result += kRootScrollerOffset;
+ }
+ FrameView* view = node.document().view();
+ if (view && view->isScrollable())
+ result += kScrollableDocumentOffset;
+
+ if (dispatchResult != DispatchEventResult::NotCanceled) {
tdresser 2016/04/07 12:22:07 Let's be consistent about whether or not we use {}
dtapuska 2016/04/19 17:54:46 Done.
+ result += kHandledOffset;
+ }
+ return (ListenerHistogram)result;
tdresser 2016/04/07 12:22:08 static_cast
dtapuska 2016/04/19 17:54:47 Done.
+}
+
+} // namespace
+
TouchEvent::TouchEvent()
{
}
@@ -42,11 +82,15 @@ TouchEvent::TouchEvent()
TouchEvent::TouchEvent(TouchList* touches, TouchList* targetTouches,
TouchList* changedTouches, const AtomicString& type,
AbstractView* view,
- PlatformEvent::Modifiers modifiers, bool cancelable, bool causesScrollingIfUncanceled,
+ PlatformEvent::Modifiers modifiers, bool cancelable, bool causesScrollingIfUncanceled, bool firstTouchMoveAfterTouchStart,
double platformTimeStamp)
// Pass a sourceCapabilities including the ability to fire touchevents when creating this touchevent, which is always created from input device capabilities from EventHandler.
- : UIEventWithKeyState(type, true, cancelable, view, 0, modifiers, platformTimeStamp, InputDeviceCapabilities::firesTouchEventsSourceCapabilities()),
- m_touches(touches), m_targetTouches(targetTouches), m_changedTouches(changedTouches), m_causesScrollingIfUncanceled(causesScrollingIfUncanceled)
+ : UIEventWithKeyState(type, true, cancelable, view, 0, modifiers, platformTimeStamp, InputDeviceCapabilities::firesTouchEventsSourceCapabilities())
+ , m_touches(touches)
+ , m_targetTouches(targetTouches)
+ , m_changedTouches(changedTouches)
+ , m_causesScrollingIfUncanceled(causesScrollingIfUncanceled)
+ , m_firstTouchMoveAfterTouchStart(firstTouchMoveAfterTouchStart)
{
}
@@ -139,8 +183,17 @@ TouchEvent& TouchEventDispatchMediator::event() const
DispatchEventResult TouchEventDispatchMediator::dispatchEvent(EventDispatcher& dispatcher) const
{
- event().eventPath().adjustForTouchEvent(event());
- return dispatcher.dispatch();
+ TouchEvent& touchEvent = event();
+ touchEvent.eventPath().adjustForTouchEvent(touchEvent);
+ DispatchEventResult result = dispatcher.dispatch();
+
+ // Only report for top level documents with a single touch on
+ // touch-start or the first touch-move.
+ if (touchEvent.cancelable() && touchEvent.touches() && touchEvent.touches()->length() == 1 && !dispatcher.node().document().ownerElement() && (touchEvent.type() == EventTypeNames::touchstart || (touchEvent.type() == EventTypeNames::touchmove && touchEvent.firstTouchMoveAfterTouchStart()))) {
tdresser 2016/04/07 12:22:08 This condition is pretty illegible. Maybe split th
dtapuska 2016/04/19 17:54:47 Done.
+ DEFINE_STATIC_LOCAL(EnumerationHistogram, rootDocumentListenerHistogram, ("Event.Listeners.TouchBlockingOnRootElements", ListenerHistogramMax));
+ rootDocumentListenerHistogram.count(toListenerHistogramValue(dispatcher.node(), result));
+ }
+ return result;
}
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698