Skip to content

Commit

Permalink
Add a feature flag and about_flags entry to disable Mutation Events
Browse files Browse the repository at this point in the history
Mutation events are being deprecated, and this feature allows testing
with the events completely disabled.

Bug: 1446498
Change-Id: Ia5584320f82b7eb1c7643b36a5edd47d4419b56e
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4547496
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1147523}
  • Loading branch information
Mason Freed authored and Chromium LUCI CQ committed May 22, 2023
1 parent 62e4362 commit e8de880
Show file tree
Hide file tree
Showing 17 changed files with 238 additions and 49 deletions.
3 changes: 3 additions & 0 deletions chrome/browser/about_flags.cc
Expand Up @@ -5431,6 +5431,9 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kSecurePaymentConfirmationDebugName,
flag_descriptions::kSecurePaymentConfirmationDebugDescription, kOsAll,
FEATURE_VALUE_TYPE(features::kSecurePaymentConfirmationDebug)},
{"mutation-events", flag_descriptions::kMutationEventsName,
flag_descriptions::kMutationEventsDescription, kOsAll,
FEATURE_VALUE_TYPE(blink::features::kMutationEvents)},
{"fill-on-account-select", flag_descriptions::kFillOnAccountSelectName,
flag_descriptions::kFillOnAccountSelectDescription, kOsAll,
FEATURE_VALUE_TYPE(password_manager::features::kFillOnAccountSelect)},
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/flag-metadata.json
Expand Up @@ -5159,6 +5159,11 @@
"owners": [ "christianxu", "stkhapugin", "bling-flags@google.com"],
"expiry_milestone": 120
},
{
"name": "mutation-events",
"owners": [ "masonf" ],
"expiry_milestone": 127
},
{
"name": "mute-notification-snooze-action",
"owners": [ "knollr@chromium.org", "peter@chromium.org" ],
Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/flag_descriptions.cc
Expand Up @@ -1572,6 +1572,14 @@ const char kFillingAcrossGroupedSitesDescription[] =
"This flag enables password filling across grouped websites. Information "
"about website groups is provided by the affiliation service.";

const char kMutationEventsName[] =
"Enable (deprecated) synchronous mutation events";
const char kMutationEventsDescription[] =
"Mutation Events are a deprecated set of events which cause performance "
"issues. Disabling this feature turns off Mutation Events. NOTE: Disabling "
"these events can cause breakage on some sites that are still reliant on "
"these deprecated features.";

const char kFillOnAccountSelectName[] = "Fill passwords on account selection";
const char kFillOnAccountSelectDescription[] =
"Filling of passwords when an account is explicitly selected by the user "
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/flag_descriptions.h
Expand Up @@ -877,6 +877,9 @@ extern const char kFillingAcrossAffiliatedWebsitesDescription[];
extern const char kFillingAcrossGroupedSitesName[];
extern const char kFillingAcrossGroupedSitesDescription[];

extern const char kMutationEventsName[];
extern const char kMutationEventsDescription[];

extern const char kFillOnAccountSelectName[];
extern const char kFillOnAccountSelectDescription[];

Expand Down
10 changes: 7 additions & 3 deletions third_party/blink/renderer/core/dom/document.cc
Expand Up @@ -5728,8 +5728,13 @@ Event* Document::createEvent(ScriptState* script_state,

void Document::AddMutationEventListenerTypeIfEnabled(
ListenerType listener_type) {
if (ContextFeatures::MutationEventsEnabled(this))
AddListenerType(listener_type);
// Mutation events can be disabled by the embedder via a ContextFeatures
// switch, or via the runtime enabled feature.
if (!ContextFeatures::MutationEventsEnabled(this) ||
!RuntimeEnabledFeatures::MutationEventsEnabled()) {
return;
}
AddListenerType(listener_type);
}

void Document::AddListenerTypeIfNeeded(const AtomicString& event_type,
Expand All @@ -5738,7 +5743,6 @@ void Document::AddListenerTypeIfNeeded(const AtomicString& event_type,
ListenerType listener_type;
if (event_util::IsDOMMutationEventType(event_type, mutation_event_feature,
listener_type)) {
UseCounter::Count(*this, mutation_event_feature);
AddMutationEventListenerTypeIfEnabled(listener_type);
} else if (event_type == event_type_names::kWebkitAnimationStart ||
event_type == event_type_names::kAnimationstart) {
Expand Down
7 changes: 7 additions & 0 deletions third_party/blink/renderer/core/dom/document.h
Expand Up @@ -1125,9 +1125,16 @@ class CORE_EXPORT Document : public ContainerNode,
kScrollListener = 1 << 14,
kLoadListenerAtCapturePhaseOrAtStyleElement = 1 << 15,
// 0 bits remaining
kDOMMutationEventListener =
kDOMSubtreeModifiedListener | kDOMNodeInsertedListener |
kDOMNodeRemovedListener | kDOMNodeRemovedFromDocumentListener |
kDOMNodeInsertedIntoDocumentListener |
kDOMCharacterDataModifiedListener,
};

bool HasListenerType(ListenerType listener_type) const {
DCHECK(RuntimeEnabledFeatures::MutationEventsEnabled() ||
!(listener_types_ & kDOMMutationEventListener));
return (listener_types_ & listener_type);
}
void AddListenerTypeIfNeeded(const AtomicString& event_type, EventTarget&);
Expand Down
22 changes: 19 additions & 3 deletions third_party/blink/renderer/core/dom/events/event_dispatcher.cc
Expand Up @@ -32,6 +32,7 @@
#include "third_party/blink/public/common/input/web_keyboard_event.h"
#include "third_party/blink/public/web/web_local_frame_client.h"
#include "third_party/blink/renderer/core/accessibility/ax_object_cache.h"
#include "third_party/blink/renderer/core/dom/context_features.h"
#include "third_party/blink/renderer/core/dom/element.h"
#include "third_party/blink/renderer/core/dom/events/event_dispatch_forbidden_scope.h"
#include "third_party/blink/renderer/core/dom/events/event_dispatch_result.h"
Expand All @@ -41,6 +42,7 @@
#include "third_party/blink/renderer/core/dom/events/window_event_context.h"
#include "third_party/blink/renderer/core/dom/node.h"
#include "third_party/blink/renderer/core/editing/editor.h"
#include "third_party/blink/renderer/core/event_type_names.h"
#include "third_party/blink/renderer/core/events/keyboard_event.h"
#include "third_party/blink/renderer/core/events/mouse_event.h"
#include "third_party/blink/renderer/core/events/simulated_event_util.h"
Expand All @@ -61,7 +63,6 @@
#include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
#include "third_party/blink/renderer/platform/keyboard_codes.h"
#include "ui/events/keycodes/dom/keycode_converter.h"

namespace blink {

DispatchEventResult EventDispatcher::DispatchEvent(Node& node, Event& event) {
Expand Down Expand Up @@ -187,7 +188,8 @@ DispatchEventResult EventDispatcher::Dispatch() {
return DispatchEventResult::kNotCanceled;
}
std::unique_ptr<EventTiming> eventTiming;
LocalFrame* frame = node_->GetDocument().GetFrame();
auto& document = node_->GetDocument();
LocalFrame* frame = document.GetFrame();
LocalDOMWindow* window = nullptr;
if (frame) {
window = frame->DomWindow();
Expand Down Expand Up @@ -218,10 +220,24 @@ DispatchEventResult EventDispatcher::Dispatch() {
DCHECK(!frame->GetAdTracker() || !frame->GetAdTracker()->IsAdScriptInStack(
AdTracker::StackType::kBottomAndTop));
if (frame->IsAdFrame()) {
UseCounter::Count(node_->GetDocument(), WebFeature::kAdClick);
UseCounter::Count(document, WebFeature::kAdClick);
}
}

#if DCHECK_IS_ON()
// If Mutation Events are disabled, we should never dispatch trusted ones.
if (event_->isTrusted() &&
(event_->type() == event_type_names::kDOMCharacterDataModified ||
event_->type() == event_type_names::kDOMSubtreeModified ||
event_->type() == event_type_names::kDOMNodeInserted ||
event_->type() == event_type_names::kDOMNodeInsertedIntoDocument ||
event_->type() == event_type_names::kDOMNodeRemoved ||
event_->type() == event_type_names::kDOMNodeRemovedFromDocument)) {
DCHECK(RuntimeEnabledFeatures::MutationEventsEnabled());
DCHECK(ContextFeatures::MutationEventsEnabled(&document));
}
#endif

// 6. Let isActivationEvent be true, if event is a MouseEvent object and
// event's type attribute is "click", and false otherwise.
//
Expand Down
88 changes: 47 additions & 41 deletions third_party/blink/renderer/core/dom/events/event_target.cc
Expand Up @@ -42,6 +42,7 @@
#include "third_party/blink/renderer/bindings/core/v8/v8_union_boolean_eventlisteneroptions.h"
#include "third_party/blink/renderer/core/dom/abort_signal.h"
#include "third_party/blink/renderer/core/dom/abort_signal_registry.h"
#include "third_party/blink/renderer/core/dom/context_features.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_dispatch_forbidden_scope.h"
Expand All @@ -63,6 +64,7 @@
#include "third_party/blink/renderer/platform/bindings/v8_dom_activity_logger.h"
#include "third_party/blink/renderer/platform/instrumentation/histogram.h"
#include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
#include "third_party/blink/renderer/platform/wtf/assertions.h"
#include "third_party/blink/renderer/platform/wtf/functional.h"
#include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
Expand Down Expand Up @@ -513,50 +515,54 @@ bool EventTarget::AddEventListenerInternal(
void EventTarget::AddedEventListener(
const AtomicString& event_type,
RegisteredEventListener& registered_listener) {
if (const LocalDOMWindow* executing_window = ExecutingWindow()) {
if (Document* document = executing_window->document()) {
if (event_type == event_type_names::kAuxclick) {
UseCounter::Count(*document, WebFeature::kAuxclickAddListenerCount);
} else if (event_type == event_type_names::kAppinstalled) {
UseCounter::Count(*document, WebFeature::kAppInstalledEventAddListener);
} else if (event_util::IsPointerEventType(event_type)) {
UseCounter::Count(*document, WebFeature::kPointerEventAddListenerCount);
} else if (event_type == event_type_names::kSlotchange) {
UseCounter::Count(*document, WebFeature::kSlotChangeEventAddListener);
} else if (event_type == event_type_names::kBeforematch) {
UseCounter::Count(*document, WebFeature::kBeforematchHandlerRegistered);
} else if (event_type ==
event_type_names::kContentvisibilityautostatechange) {
UseCounter::Count(
*document,
WebFeature::kContentVisibilityAutoStateChangeHandlerRegistered);
} else if (event_type == event_type_names::kScrollend) {
UseCounter::Count(*document, WebFeature::kScrollend);
}
const LocalDOMWindow* executing_window = ExecutingWindow();
Document* document =
executing_window ? executing_window->document() : nullptr;
if (document) {
if (event_type == event_type_names::kAuxclick) {
UseCounter::Count(*document, WebFeature::kAuxclickAddListenerCount);
} else if (event_type == event_type_names::kAppinstalled) {
UseCounter::Count(*document, WebFeature::kAppInstalledEventAddListener);
} else if (event_util::IsPointerEventType(event_type)) {
UseCounter::Count(*document, WebFeature::kPointerEventAddListenerCount);
} else if (event_type == event_type_names::kSlotchange) {
UseCounter::Count(*document, WebFeature::kSlotChangeEventAddListener);
} else if (event_type == event_type_names::kBeforematch) {
UseCounter::Count(*document, WebFeature::kBeforematchHandlerRegistered);
} else if (event_type ==
event_type_names::kContentvisibilityautostatechange) {
UseCounter::Count(
*document,
WebFeature::kContentVisibilityAutoStateChangeHandlerRegistered);
} else if (event_type == event_type_names::kScrollend) {
UseCounter::Count(*document, WebFeature::kScrollend);
}
}

WebFeature mutation_event_feature;
Document::ListenerType listener_type;
if (event_util::IsDOMMutationEventType(event_type, mutation_event_feature,
listener_type)) {
if (ExecutionContext* context = GetExecutionContext()) {
String message_text = String::Format(
"Listener added for a synchronous '%s' DOM Mutation Event. "
"This event type is deprecated "
"(https://w3c.github.io/uievents/#legacy-event-types) "
"and work is underway to remove it from this browser. Usage of this "
"event listener will cause performance issues today, and represents "
"a risk of future incompatibility. Consider using MutationObserver "
"instead.",
event_type.GetString().Utf8().c_str());
PerformanceMonitor::ReportGenericViolation(
context, PerformanceMonitor::kDiscouragedAPIUse, message_text,
base::TimeDelta(), nullptr);
context->AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
mojom::blink::ConsoleMessageSource::kDeprecation,
mojom::blink::ConsoleMessageLevel::kWarning, message_text));
Deprecation::CountDeprecation(context, mutation_event_feature);
if (RuntimeEnabledFeatures::MutationEventsEnabled() &&
(!document || ContextFeatures::MutationEventsEnabled(document))) {
WebFeature mutation_event_feature;
Document::ListenerType listener_type;
if (event_util::IsDOMMutationEventType(event_type, mutation_event_feature,
listener_type)) {
if (ExecutionContext* context = GetExecutionContext()) {
String message_text = String::Format(
"Listener added for a synchronous '%s' DOM Mutation Event. "
"This event type is deprecated "
"(https://w3c.github.io/uievents/#legacy-event-types) "
"and work is underway to remove it from this browser. Usage of "
"this event listener will cause performance issues today, and "
"represents a risk of future incompatibility. Consider using "
"MutationObserver instead.",
event_type.GetString().Utf8().c_str());
PerformanceMonitor::ReportGenericViolation(
context, PerformanceMonitor::kDiscouragedAPIUse, message_text,
base::TimeDelta(), nullptr);
context->AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
mojom::blink::ConsoleMessageSource::kDeprecation,
mojom::blink::ConsoleMessageLevel::kWarning, message_text));
Deprecation::CountDeprecation(context, mutation_event_feature);
}
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions third_party/blink/renderer/core/events/event_util.cc
Expand Up @@ -39,6 +39,11 @@ bool IsPointerEventType(const AtomicString& event_type) {
event_type == event_type_names::kPointerup;
}

bool IsDOMMutationEventType(const AtomicString& event_type) {
WebFeature web_feature;
Document::ListenerType listener_type;
return IsDOMMutationEventType(event_type, web_feature, listener_type);
}
bool IsDOMMutationEventType(const AtomicString& event_type,
WebFeature& web_feature,
Document::ListenerType& listener_type) {
Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/core/events/event_util.h
Expand Up @@ -27,6 +27,7 @@ CORE_EXPORT bool IsPointerEventType(const AtomicString& event_type);
bool IsDOMMutationEventType(const AtomicString& event_type,
WebFeature& web_feature,
Document::ListenerType& listener_type);
bool IsDOMMutationEventType(const AtomicString& event_type);

} // namespace event_util

Expand Down
3 changes: 2 additions & 1 deletion third_party/blink/renderer/core/events/mutation_event.idl
Expand Up @@ -20,7 +20,8 @@
// https://w3c.github.io/uievents/#interface-MutationEvent

[
Exposed=Window
Exposed=Window,
RuntimeEnabled=MutationEvents
] interface MutationEvent : Event {
// attrChangeType
const unsigned short MODIFICATION = 1;
Expand Down
Expand Up @@ -2321,7 +2321,6 @@
base_feature: "none",
},
{

name: "MobileLayoutTheme",
base_feature: "none",
},
Expand All @@ -2337,6 +2336,12 @@
status: "test",
base_feature: "none",
},
// crbug.com/1446498: This feature is being used for the deprecation of
// Mutation Events.
{
name: "MutationEvents",
status: "stable",
},
{
name: "NavigateEventCancelableTraversals",
status: "stable",
Expand Down
7 changes: 7 additions & 0 deletions third_party/blink/web_tests/VirtualTestSuites
Expand Up @@ -1300,6 +1300,13 @@
"--enable-blink-features=SerializeViewTransitionStateInSPA"],
"expires": "Feb 1, 2024"
},
{
"prefix": "mutation-events-disabled",
"platforms": ["Linux"],
"bases": [],
"args": ["--disable-blink-features=MutationEvents"],
"expires": "Sep 1, 2024"
},
{
"prefix": "popover-hint-disabled",
"platforms": ["Linux", "Mac", "Win"],
Expand Down
@@ -0,0 +1,53 @@
<!DOCTYPE html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>

<div id=target>foo</div>

<script>
test(() => {
const target = document.getElementById('target');

// From web_feature.mojom
const kDOMSubtreeModifiedEvent = 143;
const kDOMNodeInsertedEvent = 144;
const kDOMNodeRemovedEvent = 145;
const kDOMNodeRemovedFromDocumentEvent = 146;
const kDOMNodeInsertedIntoDocumentEvent = 147;
const kDOMCharacterDataModifiedEvent = 148;

const events = [
{counter: kDOMSubtreeModifiedEvent, event: 'DOMSubtreeModified'},
{counter: kDOMNodeInsertedEvent, event: 'DOMNodeInserted'},
{counter: kDOMNodeRemovedEvent, event: 'DOMNodeRemoved'},
{counter: kDOMNodeRemovedFromDocumentEvent, event: 'DOMNodeRemovedFromDocument'},
{counter: kDOMNodeInsertedIntoDocumentEvent, event: 'DOMNodeInsertedIntoDocument'},
{counter: kDOMCharacterDataModifiedEvent, event: 'DOMCharacterDataModified'},
];

const expectedCounters = new Set();
function assertUseCounted(msg) {
events.forEach(evt => {
assert_equals(internals.isUseCounted(document, evt.counter), expectedCounters.has(evt.counter), evt.event + ', ' + msg);
});
}
function triggerThemAll() {
target.remove();
document.body.appendChild(target);
target.setAttribute('foo','bar');
target.removeAttribute('foo');
target.firstChild.textContent = "bar";
}

assertUseCounted('initial');
triggerThemAll();
assertUseCounted('before event listeners attached');
events.forEach(evt => {
target.addEventListener(evt.event,() => {});
expectedCounters.add(evt.counter);
assertUseCounted('after adding ' + evt.event + ' listener, before firing');
triggerThemAll();
assertUseCounted('after adding ' + evt.event + ' listener, after firing (no difference)');
});
}, 'Use of Mutation Events is use counted.');
</script>
@@ -0,0 +1,4 @@
# Overview

This suite tests the case where Mutation Events are disabled via
`--disable-blink-features=MutationEvents`.

0 comments on commit e8de880

Please sign in to comment.