Call capture event listeners in capturing phase at shadow hosts

Chromestatus entry is here: https://www.chromestatus.com/feature/5636327009681408 Per the discussion of whatwg/dom#685, Blink will try to align the event dispatch behavior with other browsers; Call capture event listeners in capturing phase at shadow hosts. So far, Blink and WebKit call capture event listeners in *bubbling* phase, instead of *capturing* phase, at shadow hosts. Other browsers: - Safari: Will try to change the behavior in the next Safari Technical Preview. - Firefox: Already implemented the new behavior - Edge: Strong public support for the new behavior. This change is guard by CallCaptureListenersAtCapturePhaseAtShadowHosts flag, which is disabled at this moment, to confirm that this CL doesn't cause any behavior change when the flag is disabled. This CL adds a wpt for new behavior, which is now marked as [Failure] in Blink. After this CL lands, I will flip the flag in a follow-up CL, with rebasing a very few existing tests. BUG=883650 Change-Id: I29938840aed4f3430d8b749cd4843176b8668b5d Reviewed-on: https://chromium-review.googlesource.com/1212255 Commit-Queue: Hayato Ito <hayato@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Cr-Commit-Position: refs/heads/master@{#591939}
chromium · Sep 18, 2018 · da8cf24 · da8cf24
1 parent 9a8aacf
commit da8cf24
Show file tree

Hide file tree

Showing 10 changed files with 156 additions and 18 deletions.
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations
@@ -1947,6 +1947,7 @@ crbug.com/404597 fast/forms/long-text-in-input.html [ Skip ]
 # Web Components related tests (Shadow DOM, Custom Elements) failures.
 crbug.com/392771 external/wpt/shadow-dom/untriaged/styles/test-003.html [ Failure ]
 crbug.com/869308 shadow-dom/imperative-api.html [ Failure ]
+crbug.com/883650 external/wpt/shadow-dom/event-dispatch-order.tentative.html [ Failure ]
 
 crbug.com/552494 virtual/prefer_compositing_to_lcd_text/scrollbars/overflow-scrollbar-combinations.html [ Pass Failure ]
 

diff --git a/third_party/WebKit/LayoutTests/external/wpt/shadow-dom/event-dispatch-order.tentative.html b/third_party/WebKit/LayoutTests/external/wpt/shadow-dom/event-dispatch-order.tentative.html
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<title>Shadow DOM: event dispatch order for capture and non-capture listerns at a shadow host</title>
+<meta name="author" title="Hayato Ito" href="mailto:hayato@google.com">
+<link rel="help" href="https://github.com/whatwg/dom/issues/685">
+<link rel="help" href="https://github.com/whatwg/dom/pull/686">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="resources/shadow-dom.js"></script>
+<div id=host>
+  <template id=shadowroot data-mode=open>
+    <div id=target></div>
+  </template>
+</div>
+<script>
+test(() => {
+  let nodes = createTestTree(host);
+  let log = dispatchEventWithLog(nodes, nodes.target,
+                                 new Event('my-event', { bubbles: true, composed: true }),
+                                 { capture: true });
+  let path = ['target', 'shadowroot', 'host'];
+  assert_event_path_equals(log,
+                           [['host', 'host', null, path, 'capture'],
+                            ['shadowroot', 'target', null, path, 'capture'],
+                            ['target', 'target', null, path, 'capture'],
+                            ['target', 'target', null, path, 'non-capture'],
+                            ['shadowroot', 'target', null, path, 'non-capture'],
+                            ['host', 'host', null, path, 'non-capture'],
+                           ]);
+}, 'Event dispatch order: capture listerns should be called in capturing phase at a shadow host');
+</script>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/shadow-dom/resources/shadow-dom.js b/third_party/WebKit/LayoutTests/external/wpt/shadow-dom/resources/shadow-dom.js
@@ -53,7 +53,9 @@ function createTestTree(node) {
   return ids;
 }
 
-function dispatchEventWithLog(nodes, target, event) {
+// TODO: Refactor this so that only interested results are recorded.
+// Callers of this function would not be interested in every results.
+function dispatchEventWithLog(nodes, target, event, options) {
 
   function labelFor(e) {
     return e.id || e.tagName;
@@ -70,15 +72,40 @@ function dispatchEventWithLog(nodes, target, event) {
       if (!id)
         continue;
       attachedNodes.push(node);
-      node.addEventListener(event.type, (e) => {
+      if (options && options.capture) {
+        // Record [currentTarget, target, relatedTarget, composedPath(), 'capture' | 'non-capture']
+        // TODO: Support registering listeners in different orders.
+        // e.g. Register a non-capture listener at first, then register a capture listener.
+        node.addEventListener(event.type, (e) => {
+          log.push([id,
+                    labelFor(e.target),
+                    e.relatedTarget ? labelFor(e.relatedTarget) : null,
+                    e.composedPath().map((n) => {
+                      return labelFor(n);
+                    }),
+                    'capture']);
+        }, true);
+        node.addEventListener(event.type, (e) => {
+          log.push([id,
+                    labelFor(e.target),
+                    e.relatedTarget ? labelFor(e.relatedTarget) : null,
+                    e.composedPath().map((n) => {
+                      return labelFor(n);
+                    }),
+                    'non-capture']);
+        });
+      } else {
         // Record [currentTarget, target, relatedTarget, composedPath()]
-        log.push([id,
-                  labelFor(e.target),
-                  e.relatedTarget ? labelFor(e.relatedTarget) : null,
-                  e.composedPath().map((n) => {
-                    return labelFor(n);
-                  })]);
-      });
+        node.addEventListener(event.type, (e) => {
+          log.push([id,
+                    labelFor(e.target),
+                    e.relatedTarget ? labelFor(e.relatedTarget) : null,
+                    e.composedPath().map((n) => {
+                      return labelFor(n);
+                    })]
+                  );
+        });
+      }
     }
   }
   target.dispatchEvent(event);
@@ -122,9 +149,13 @@ function dispatchUAEventWithLog(nodes, target, eventType, callback) {
 function assert_event_path_equals(actual, expected) {
   assert_equals(actual.length, expected.length);
   for (let i = 0; i < actual.length; ++i) {
+    assert_equals(actual[i].length, expected[i].length);
     assert_equals(actual[i][0], expected[i][0], 'currentTarget at ' + i + ' should be same');
     assert_equals(actual[i][1], expected[i][1], 'target at ' + i + ' should be same');
     assert_equals(actual[i][2], expected[i][2], 'relatedTarget at ' + i + ' should be same');
     assert_array_equals(actual[i][3], expected[i][3], 'composedPath at ' + i + ' should be same');
+    if (actual[i][4]) {
+      assert_equals(actual[i][4], expected[i][4], 'listener type should be same at ' + i);
+    }
   }
 }
diff --git a/third_party/blink/renderer/core/dom/events/event.cc b/third_party/blink/renderer/core/dom/events/event.cc
@@ -100,6 +100,8 @@ Event::Event(const AtomicString& event_type,
       executed_listener_or_default_action_(false),
       prevent_default_called_on_uncancelable_event_(false),
       legacy_did_listeners_throw_flag_(false),
+      fire_only_capture_listeners_at_target_(false),
+      fire_only_non_capture_listeners_at_target_(false),
       handling_passive_(PassiveMode::kNotPassiveDefault),
       event_phase_(0),
       current_target_(nullptr),

diff --git a/third_party/blink/renderer/core/dom/events/event.h b/third_party/blink/renderer/core/dom/events/event.h
@@ -136,6 +136,27 @@ class CORE_EXPORT Event : public ScriptWrappable {
   unsigned short eventPhase() const { return event_phase_; }
   void SetEventPhase(unsigned short event_phase) { event_phase_ = event_phase; }
 
+  void SetFireOnlyCaptureListenersAtTarget(
+      bool fire_only_capture_listeners_at_target) {
+    DCHECK_EQ(event_phase_, kAtTarget);
+    fire_only_capture_listeners_at_target_ =
+        fire_only_capture_listeners_at_target;
+  }
+
+  void SetFireOnlyNonCaptureListenersAtTarget(
+      bool fire_only_non_capture_listeners_at_target) {
+    DCHECK_EQ(event_phase_, kAtTarget);
+    fire_only_non_capture_listeners_at_target_ =
+        fire_only_non_capture_listeners_at_target;
+  }
+
+  bool FireOnlyCaptureListenersAtTarget() const {
+    return fire_only_capture_listeners_at_target_;
+  }
+  bool FireOnlyNonCaptureListenersAtTarget() const {
+    return fire_only_non_capture_listeners_at_target_;
+  }
+
   bool bubbles() const { return bubbles_; }
   bool cancelable() const { return cancelable_; }
   bool composed() const { return composed_; }
@@ -332,6 +353,11 @@ class CORE_EXPORT Event : public ScriptWrappable {
   // https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke
   unsigned legacy_did_listeners_throw_flag_ : 1;
 
+  // This fields are effective only when
+  // CallCaptureListenersAtCapturePhaseAtShadowHosts runtime flag is enabled.
+  unsigned fire_only_capture_listeners_at_target_ : 1;
+  unsigned fire_only_non_capture_listeners_at_target_ : 1;
+
   PassiveMode handling_passive_;
   unsigned short event_phase_;
   Member<EventTarget> current_target_;

diff --git a/third_party/blink/renderer/core/dom/events/event_dispatcher.cc b/third_party/blink/renderer/core/dom/events/event_dispatcher.cc
@@ -226,9 +226,18 @@ inline EventDispatchContinuation EventDispatcher::DispatchEventAtCapturing() {
 
   for (size_t i = event_->GetEventPath().size() - 1; i > 0; --i) {
     const NodeEventContext& event_context = event_->GetEventPath()[i];
-    if (event_context.CurrentTargetSameAsTarget())
-      continue;
-    event_context.HandleLocalEvents(*event_);
+    if (event_context.CurrentTargetSameAsTarget()) {
+      if (!RuntimeEnabledFeatures::
+              CallCaptureListenersAtCapturePhaseAtShadowHostsEnabled())
+        continue;
+      event_->SetEventPhase(Event::kAtTarget);
+      event_->SetFireOnlyCaptureListenersAtTarget(true);
+      event_context.HandleLocalEvents(*event_);
+      event_->SetFireOnlyCaptureListenersAtTarget(false);
+    } else {
+      event_->SetEventPhase(Event::kCapturingPhase);
+      event_context.HandleLocalEvents(*event_);
+    }
     if (event_->PropagationStopped())
       return kDoneDispatching;
   }
@@ -249,13 +258,22 @@ inline void EventDispatcher::DispatchEventAtBubbling() {
   for (size_t i = 1; i < size; ++i) {
     const NodeEventContext& event_context = event_->GetEventPath()[i];
     if (event_context.CurrentTargetSameAsTarget()) {
+      // TODO(hayato): Need to check cancelBubble() also here?
       event_->SetEventPhase(Event::kAtTarget);
+      if (RuntimeEnabledFeatures::
+              CallCaptureListenersAtCapturePhaseAtShadowHostsEnabled()) {
+        event_->SetFireOnlyNonCaptureListenersAtTarget(true);
+        event_context.HandleLocalEvents(*event_);
+        event_->SetFireOnlyNonCaptureListenersAtTarget(false);
+      } else {
+        event_context.HandleLocalEvents(*event_);
+      }
     } else if (event_->bubbles() && !event_->cancelBubble()) {
       event_->SetEventPhase(Event::kBubblingPhase);
+      event_context.HandleLocalEvents(*event_);
     } else {
       continue;
     }
-    event_context.HandleLocalEvents(*event_);
     if (event_->PropagationStopped())
       return;
   }

diff --git a/third_party/blink/renderer/core/dom/events/event_target.cc b/third_party/blink/renderer/core/dom/events/event_target.cc
@@ -816,11 +816,7 @@ bool EventTarget::FireEventListeners(Event& event,
     // EventTarget::removeEventListener.
     ++i;
 
-    if (event.eventPhase() == Event::kCapturingPhase &&
-        !registered_listener.Capture())
-      continue;
-    if (event.eventPhase() == Event::kBubblingPhase &&
-        registered_listener.Capture())
+    if (!registered_listener.ShouldFire(event))
       continue;
 
     EventListener* listener = registered_listener.Callback();

diff --git a/third_party/blink/renderer/core/events/registered_event_listener.cc b/third_party/blink/renderer/core/events/registered_event_listener.cc
@@ -25,7 +25,9 @@
 #include "third_party/blink/renderer/core/events/registered_event_listener.h"
 
 #include "third_party/blink/renderer/core/dom/events/add_event_listener_options_resolved.h"
+#include "third_party/blink/renderer/core/dom/events/event.h"
 #include "third_party/blink/renderer/core/dom/events/event_listener.h"
+#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
 
 namespace blink {
 
@@ -80,6 +82,32 @@ bool RegisteredEventListener::Matches(
          static_cast<bool>(use_capture_) == options.capture();
 }
 
+bool RegisteredEventListener::ShouldFire(const Event& event) const {
+  if (RuntimeEnabledFeatures::
+          CallCaptureListenersAtCapturePhaseAtShadowHostsEnabled()) {
+    if (event.FireOnlyCaptureListenersAtTarget()) {
+      DCHECK_EQ(event.eventPhase(), Event::kAtTarget);
+      return Capture();
+    }
+    if (event.FireOnlyNonCaptureListenersAtTarget()) {
+      DCHECK_EQ(event.eventPhase(), Event::kAtTarget);
+      return !Capture();
+    }
+    if (event.eventPhase() == Event::kCapturingPhase)
+      return Capture();
+    if (event.eventPhase() == Event::kBubblingPhase)
+      return !Capture();
+    return true;
+  }
+  DCHECK(!event.FireOnlyCaptureListenersAtTarget());
+  DCHECK(!event.FireOnlyNonCaptureListenersAtTarget());
+  if (event.eventPhase() == Event::kCapturingPhase)
+    return Capture();
+  if (event.eventPhase() == Event::kBubblingPhase)
+    return !Capture();
+  return true;
+}
+
 bool RegisteredEventListener::operator==(
     const RegisteredEventListener& other) const {
   // Equality is soley based on the listener and useCapture flags.

diff --git a/third_party/blink/renderer/core/events/registered_event_listener.h b/third_party/blink/renderer/core/events/registered_event_listener.h
@@ -32,6 +32,7 @@
 namespace blink {
 
 class AddEventListenerOptionsResolved;
+class Event;
 class EventListener;
 class EventListenerOptions;
 
@@ -79,6 +80,8 @@ class RegisteredEventListener final {
   bool Matches(const EventListener* listener,
                const EventListenerOptions& options) const;
 
+  bool ShouldFire(const Event&) const;
+
   bool operator==(const RegisteredEventListener& other) const;
 
  private:

diff --git a/third_party/blink/renderer/platform/runtime_enabled_features.json5 b/third_party/blink/renderer/platform/runtime_enabled_features.json5
@@ -157,6 +157,9 @@
       name: "CacheStyleSheetWithMediaQueries",
       status: "experimental",
     },
+    {
+      name: "CallCaptureListenersAtCapturePhaseAtShadowHosts",
+    },
     {
       name: "Canvas2dContextLostRestored",
       status: "experimental",