v8bindings: Remove custom bindings of window.event

Currently, window.event is implemented as a private symbol property
stored on the Window JavaScript object itself. While doing so may have
simplified memory management when the code was first written versus
storing the event in C++, it does not anymore with Oilpan and unified
heap. This also has no performance benefits, since each call to event()
has had to jump through multiple C++/JS boundaries.

The custom setter for Window.event was previous removed in r538391. This
CL removes the custom getter.

Bug: 236420
Change-Id: I67e862dda8c5a0248436ea90553fbf0b9b37aca9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225505
Commit-Queue: Timothy Gu <timothygu@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774170}

third_party/blink/renderer/bindings/IDLExtendedAttributes.md

@@ -640,13 +640,13 @@ Usage:
 
 For methods all calls are logged, and by default for attributes all access (calls to getter or setter) are logged, but this can be restricted to just read (getter) or just write (setter).
 
-### [CallWith] _(m, a)_, [SetterCallWith] _(a)_, [ConstructorCallWith] _(i)_
+### [CallWith] _(m, a)_, [GetterCallWith] _(a)_, [SetterCallWith] _(a)_, [ConstructorCallWith] _(i)_
 
 Summary: `[CallWith]` indicates that the bindings code calls the Blink implementation with additional information.
 
 Each value changes the signature of the Blink methods by adding an additional parameter to the head of the parameter list, such as `ScriptState*` for `[CallWith=ScriptState]`.
 
-`[SetterCallWith]` applies to attributes, and only affects the signature of the setter.
+`[GetterCallWith]` and `[SetterCallWith]` apply to attributes, and only affects the signature of the getter and setter, respectively.
 
 #### [CallWith=ScriptState] _(m, a*)_
 

third_party/blink/renderer/bindings/IDLExtendedAttributes.txt

@@ -58,6 +58,7 @@ EnforceRange
 Exposed=*
 FeaturePolicy=*
 FlexibleArrayBufferView
+GetterCallWith=ScriptState
 Global=*
 HighEntropy
 HTMLConstructor

third_party/blink/renderer/bindings/core/v8/custom/v8_window_custom.cc

@@ -100,42 +100,6 @@ void V8Window::LocationAttributeGetterCustom(
   V8SetReturnValue(info, wrapper);
 }
 
-void V8Window::EventAttributeGetterCustom(
-    const v8::FunctionCallbackInfo<v8::Value>& info) {
-  LocalDOMWindow* impl = To<LocalDOMWindow>(V8Window::ToImpl(info.Holder()));
-  v8::Isolate* isolate = info.GetIsolate();
-  ExceptionState exception_state(isolate, ExceptionState::kGetterContext,
-                                 "Window", "event");
-  if (!BindingSecurity::ShouldAllowAccessTo(CurrentDOMWindow(isolate), impl,
-                                            exception_state)) {
-    return;
-  }
-
-  v8::Local<v8::Value> js_event;
-  if (!V8PrivateProperty::GetSymbol(isolate, kPrivatePropertyGlobalEvent)
-           .GetOrUndefined(info.Holder())
-           .ToLocal(&js_event)) {
-    return;
-  }
-
-  // Track usage of window.event when the event's target is inside V0 shadow
-  // tree.
-  // TODO(yukishiino): Make window.event [Replaceable] and simplify the
-  // following IsWrapper/ToImplWithTypeCheck hack.
-  if (V8DOMWrapper::IsWrapper(isolate, js_event)) {
-    if (Event* event = V8Event::ToImplWithTypeCheck(isolate, js_event)) {
-      if (event->target()) {
-        Node* target_node = event->target()->ToNode();
-        if (target_node && target_node->IsInV0ShadowTree()) {
-          UseCounter::Count(CurrentExecutionContext(isolate),
-                            WebFeature::kWindowEventInV0ShadowTree);
-        }
-      }
-    }
-  }
-  V8SetReturnValue(info, js_event);
-}
-
 void V8Window::FrameElementAttributeGetterCustom(
     const v8::FunctionCallbackInfo<v8::Value>& info) {
   LocalDOMWindow* impl = To<LocalDOMWindow>(V8Window::ToImpl(info.Holder()));

third_party/blink/renderer/bindings/core/v8/js_based_event_listener.cc

@@ -16,9 +16,6 @@
 
 namespace blink {
 
-// extern
-const V8PrivateProperty::SymbolKey kPrivatePropertyGlobalEvent;
-
 JSBasedEventListener::JSBasedEventListener() {
   if (IsMainThread()) {
     InstanceCounters::IncrementCounter(
@@ -123,25 +120,23 @@ void JSBasedEventListener::Invoke(
 
   // Step 6: Let |global| be listener callback’s associated Realm’s global
   // object.
-  v8::Local<v8::Object> global =
-      script_state_of_listener->GetContext()->Global();
-
-  // Step 8: If global is a Window object, then:
-  // Set currentEvent to global’s current event.
-  // If tuple’s item-in-shadow-tree is false, then set global’s current event to
-  // event.
-  V8PrivateProperty::Symbol event_symbol =
-      V8PrivateProperty::GetSymbol(isolate, kPrivatePropertyGlobalEvent);
-  ExecutionContext* execution_context_of_listener =
-      ExecutionContext::From(script_state_of_listener);
-  v8::Local<v8::Value> current_event;
-  if (execution_context_of_listener->IsDocument()) {
-    current_event = event_symbol.GetOrUndefined(global).ToLocalChecked();
-    // Expose the event object as |window.event|, except when the event's target
-    // is in a V1 shadow tree.
+  LocalDOMWindow* window =
+      ToLocalDOMWindow(script_state_of_listener->GetContext());
+
+  // Step 7: Let |current_event| be undefined.
+  Event* current_event = nullptr;
+
+  // Step 8: If |global| is a Window object, then:
+  if (window) {
+    // Step 8-1: Set |current_event| to |global|’s current event.
+    current_event = window->CurrentEvent();
+
+    // Step 8-2: If |struct|’s invocation-target-in-shadow-tree is false (i.e.,
+    // event's target is in a V1 shadow tree), then set |global|’s current
+    // event to event.
     Node* target_node = event->target()->ToNode();
     if (!(target_node && target_node->IsInV1ShadowTree()))
-      event_symbol.Set(global, js_event);
+      window->SetCurrentEvent(event);
   }
 
   {
@@ -158,17 +153,12 @@ void JSBasedEventListener::Invoke(
       // Step 10-2: Set legacyOutputDidListenersThrowFlag if given.
       event->LegacySetDidListenersThrowFlag();
     }
-
-    // |event_symbol.Set(global, current_event)| cannot and does not have to be
-    // performed when the isolate is terminating.
-    if (isolate->IsExecutionTerminating())
-      return;
   }
 
   // Step 12: If |global| is a Window object, then set |global|’s current event
   // to |current_event|.
-  if (execution_context_of_listener->IsDocument())
-    event_symbol.Set(global, current_event);
+  if (window)
+    window->SetCurrentEvent(current_event);
 }
 
 std::unique_ptr<SourceLocation> JSBasedEventListener::GetSourceLocation(

third_party/blink/renderer/bindings/core/v8/js_based_event_listener.h

@@ -91,8 +91,6 @@ struct DowncastTraits<JSBasedEventListener> {
   }
 };
 
-extern const V8PrivateProperty::SymbolKey kPrivatePropertyGlobalEvent;
-
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_JS_BASED_EVENT_LISTENER_H_

third_party/blink/renderer/bindings/scripts/bind_gen/interface.py

@@ -576,6 +576,7 @@ def _make_blink_api_call(code_node,
     ext_attrs = cg_context.member_like.extended_attributes
 
     values = ext_attrs.values_of("CallWith") + (
+        ext_attrs.values_of("GetterCallWith") if cg_context.attribute_get else
         ext_attrs.values_of("SetterCallWith") if cg_context.attribute_set else
         ())
     if "Isolate" in values:

third_party/blink/renderer/bindings/scripts/v8_attributes.py

@@ -442,6 +442,9 @@ def getter_context(interface, attribute, context):
             cpp_value=cpp_value,
             creation_context='holder',
             extended_attributes=extended_attributes),
+        'is_getter_call_with_script_state':
+        has_extended_attribute_value(attribute, 'GetterCallWith',
+                                     'ScriptState'),
         'v8_set_return_value_for_main_world':
         v8_set_return_value_statement(for_main_world=True),
         'v8_set_return_value':
@@ -455,10 +458,9 @@ def getter_expression(interface, attribute, context):
                                              extra_arguments)
     getter_name = scoped_name(interface, attribute, this_getter_base_name)
 
-    arguments = []
-    arguments.extend(
-        v8_utilities.call_with_arguments(
-            attribute.extended_attributes.get('CallWith')))
+    arguments = v8_utilities.call_with_arguments(
+        attribute.extended_attributes.get('GetterCallWith')
+        or attribute.extended_attributes.get('CallWith'))
     # Members of IDL partial interface definitions are implemented in C++ as
     # static member functions, which for instance members (non-static members)
     # take *impl as their first argument

third_party/blink/renderer/bindings/templates/attributes.cc.tmpl

@@ -122,7 +122,8 @@ const v8::FunctionCallbackInfo<v8::Value>& info
   {% endif %}
   {% endif %}
 
-  {% if attribute.is_call_with_script_state %}
+  {% if attribute.is_call_with_script_state or
+        attribute.is_getter_call_with_script_state %}
   {% if attribute.is_static %}
   ScriptState* script_state = ScriptState::ForCurrentRealm(info);
   {% else %}
@@ -455,7 +456,7 @@ static void {{attribute.camel_case_name}}AttributeSetter{{world_suffix}}(
   {% endif %}
 
   {% if attribute.is_call_with_script_state or
-  	attribute.is_setter_call_with_script_state %}
+        attribute.is_setter_call_with_script_state %}
   {% if attribute.is_static %}
   ScriptState* script_state = ScriptState::ForCurrentRealm(info);
   {% else %}

third_party/blink/renderer/core/frame/local_dom_window.cc

@@ -39,6 +39,7 @@
 #include "third_party/blink/renderer/bindings/core/v8/binding_security.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_controller.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
+#include "third_party/blink/renderer/bindings/core/v8/script_value.h"
 #include "third_party/blink/renderer/bindings/core/v8/source_location.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_scroll_to_options.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_void_function.h"
@@ -116,6 +117,7 @@
 #include "third_party/blink/renderer/core/trustedtypes/trusted_types_util.h"
 #include "third_party/blink/renderer/platform/bindings/exception_messages.h"
 #include "third_party/blink/renderer/platform/bindings/microtask.h"
+#include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h"
 #include "third_party/blink/renderer/platform/scheduler/public/dummy_schedulers.h"
@@ -123,6 +125,7 @@
 #include "third_party/blink/renderer/platform/timer.h"
 #include "third_party/blink/renderer/platform/weborigin/security_origin.h"
 #include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
+#include "v8/include/v8.h"
 
 namespace blink {
 
@@ -240,6 +243,34 @@ void LocalDOMWindow::AcceptLanguagesChanged() {
   DispatchEvent(*Event::Create(event_type_names::kLanguagechange));
 }
 
+ScriptValue LocalDOMWindow::event(ScriptState* script_state) const {
+  // If current event is null, return undefined.
+  if (!current_event_) {
+    return ScriptValue(script_state->GetIsolate(),
+                       ToV8(ToV8UndefinedGenerator(), script_state));
+  }
+
+  // Track usage of window.event when the event's target is inside V0 shadow
+  // tree.
+  if (current_event_->target()) {
+    Node* target_node = current_event_->target()->ToNode();
+    if (target_node && target_node->IsInV0ShadowTree()) {
+      UseCounter::Count(document(), WebFeature::kWindowEventInV0ShadowTree);
+    }
+  }
+
+  return ScriptValue(script_state->GetIsolate(),
+                     ToV8(CurrentEvent(), script_state));
+}
+
+Event* LocalDOMWindow::CurrentEvent() const {
+  return current_event_.Get();
+}
+
+void LocalDOMWindow::SetCurrentEvent(Event* new_event) {
+  current_event_ = new_event;
+}
+
 TrustedTypePolicyFactory* LocalDOMWindow::trustedTypes() const {
   if (!trusted_types_) {
     trusted_types_ =
@@ -1905,6 +1936,7 @@ void LocalDOMWindow::Trace(Visitor* visitor) const {
   visitor->Trace(application_cache_);
   visitor->Trace(visualViewport_);
   visitor->Trace(event_listener_observers_);
+  visitor->Trace(current_event_);
   visitor->Trace(trusted_types_);
   visitor->Trace(input_method_controller_);
   visitor->Trace(spell_checker_);

third_party/blink/renderer/core/frame/local_dom_window.h

@@ -27,6 +27,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_LOCAL_DOM_WINDOW_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_LOCAL_DOM_WINDOW_H_
 
+#include "third_party/blink/renderer/bindings/core/v8/script_value.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/dom/events/event_target.h"
@@ -353,6 +354,11 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
 
   void AcceptLanguagesChanged();
 
+  // https://dom.spec.whatwg.org/#dom-window-event
+  ScriptValue event(ScriptState*) const;
+  Event* CurrentEvent() const;
+  void SetCurrentEvent(Event*);
+
   TrustedTypePolicyFactory* trustedTypes() const;
 
   // Returns true if this window is cross-site to the main frame. Defaults to
@@ -435,6 +441,10 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
 
   HeapHashSet<WeakMember<EventListenerObserver>> event_listener_observers_;
 
+  // https://dom.spec.whatwg.org/#window-current-event
+  // We represent the "undefined" value as nullptr.
+  Member<Event> current_event_;
+
   mutable Member<TrustedTypePolicyFactory> trusted_types_;
 
   // A dummy scheduler to return when the window is detached.

third_party/blink/renderer/core/frame/window.idl

@@ -180,9 +180,11 @@
     // TODO(yhirano): Move this to url.idl when LegacyWindowAlias is supported.
     [DisableInNewIDLCompiler] attribute URLConstructor webkitURL;
 
+    // https://dom.spec.whatwg.org/#interface-window-extensions
+    [Replaceable, GetterCallWith=ScriptState, MeasureAs=WindowEvent, NotEnumerable] readonly attribute any event;
+
     // Non-standard APIs
     [MeasureAs=WindowClientInformation, Replaceable] readonly attribute Navigator clientInformation;
-    [Replaceable, MeasureAs=WindowEvent, Custom=Getter, NotEnumerable] readonly attribute Event event;
     [MeasureAs=WindowFind] boolean find(optional DOMString string = "",
                                         optional boolean caseSensitive = false,
                                         optional boolean backwards = false,