Changeset 291933 in webkit

Timestamp:

Mar 25, 2022 10:26:02 PM (2 years ago)

Author:

Chris Dumez

Message:

Simplify / Optimize JSNodeOwner::isReachableFromOpaqueRoots()
​https://bugs.webkit.org/show_bug.cgi?id=238380

Reviewed by Geoffrey Garen.

Drop checks specific to HTMLAudioElement and HTMLImageElement from
JSNodeOwner::isReachableFromOpaqueRoots() so that other Node wrappers
that are not audio or image elements do not have to pay the cost.

In the HTMLAudioElement case, HTMLAudioElement already subclasses HTMLMediaElement which
is an ActiveDOMObject and HTMLMediaElement::virtualHasPendingActivity() already takes
care of keeping the JS wrapper alive is there is audio playing.

For HTMLImageElement, I made it subclass ActiveDOMObject so that the JSHTMLImageElement
wrapper is now calling HTMLImageElement::hasPendingActivity() instead of every JS Node
wrapper via JSNodeOwner::isReachableFromOpaqueRoots().

• bindings/js/JSNodeCustom.cpp:

(WebCore::isReachableFromDOM):

• html/HTMLImageElement.cpp:

(WebCore::HTMLImageElement::HTMLImageElement):
(WebCore::HTMLImageElement::create):
(WebCore::HTMLImageElement::createForLegacyFactoryFunction):
(WebCore::HTMLImageElement::activeDOMObjectName const):
(WebCore::HTMLImageElement::virtualHasPendingActivity const):
(WebCore::HTMLImageElement::hasPendingActivity const): Deleted.

• html/HTMLImageElement.h:
• html/HTMLImageElement.idl:
• html/ImageDocument.cpp:

(WebCore::ImageDocumentElement::create):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/js/JSNodeCustom.cpp (modified) (3 diffs)
• html/HTMLImageElement.cpp (modified) (4 diffs)
• html/HTMLImageElement.h (modified) (4 diffs)
• html/HTMLImageElement.idl (modified) (1 diff)
• html/ImageDocument.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-03-25  Chris Dumez  <cdumez@apple.com>
+
+        Simplify / Optimize JSNodeOwner::isReachableFromOpaqueRoots()
+        https://bugs.webkit.org/show_bug.cgi?id=238380
+
+        Reviewed by Geoffrey Garen.
+
+        Drop checks specific to HTMLAudioElement and HTMLImageElement from
+        JSNodeOwner::isReachableFromOpaqueRoots() so that other Node wrappers
+        that are not audio or image elements do not have to pay the cost.
+
+        In the HTMLAudioElement case, HTMLAudioElement already subclasses HTMLMediaElement which
+        is an ActiveDOMObject and HTMLMediaElement::virtualHasPendingActivity() already takes
+        care of keeping the JS wrapper alive is there is audio playing.
+
+        For HTMLImageElement, I made it subclass ActiveDOMObject so that the JSHTMLImageElement
+        wrapper is now calling HTMLImageElement::hasPendingActivity() instead of every JS Node
+        wrapper via JSNodeOwner::isReachableFromOpaqueRoots().
+
+        * bindings/js/JSNodeCustom.cpp:
+        (WebCore::isReachableFromDOM):
+        * html/HTMLImageElement.cpp:
+        (WebCore::HTMLImageElement::HTMLImageElement):
+        (WebCore::HTMLImageElement::create):
+        (WebCore::HTMLImageElement::createForLegacyFactoryFunction):
+        (WebCore::HTMLImageElement::activeDOMObjectName const):
+        (WebCore::HTMLImageElement::virtualHasPendingActivity const):
+        (WebCore::HTMLImageElement::hasPendingActivity const): Deleted.
+        * html/HTMLImageElement.h:
+        * html/HTMLImageElement.idl:
+        * html/ImageDocument.cpp:
+        (WebCore::ImageDocumentElement::create):
+
 2022-03-25  Nikolaos Mouchtaris  <nmouchtaris@apple.com>
 

trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp

@@ -33 +33 @@
 #include "DocumentFragment.h"
 #include "DocumentType.h"
-#include "HTMLAudioElement.h"
-#include "HTMLCanvasElement.h"
 #include "HTMLElement.h"
-#include "HTMLFrameElementBase.h"
-#include "HTMLImageElement.h"
-#include "HTMLLinkElement.h"
 #include "HTMLNames.h"
-#include "HTMLScriptElement.h"
-#include "HTMLStyleElement.h"
 #include "JSAttr.h"
 #include "JSCDATASection.h"
@@ -65 +58 @@
 #include "ShadowRoot.h"
 #include "GCReachableRef.h"
-#include "StyleSheet.h"
-#include "StyledElement.h"
 #include "Text.h"
 
@@ -77 +68 @@
 {
     if (!node->isConnected()) {
-        if (is<Element>(*node)) {
-            auto& element = downcast<Element>(*node);
-
-            // If a wrapper is the last reference to an image element
-            // that is loading but not in the document, the wrapper is observable
-            // because it is the only thing keeping the image element alive, and if
-            // the element is destroyed, its load event will not fire.
-            // FIXME: The DOM should manage this issue without the help of JavaScript wrappers.
-            if (is<HTMLImageElement>(element)) {
-                if (downcast<HTMLImageElement>(element).hasPendingActivity()) {
-                    if (UNLIKELY(reason))
-                        *reason = "Image element with pending activity";
-                    return true;
-                }
-            }
-#if ENABLE(VIDEO)
-            else if (is<HTMLAudioElement>(element)) {
-                if (!downcast<HTMLAudioElement>(element).paused()) {
-                    if (UNLIKELY(reason))
-                        *reason = "Audio element which is not paused";
-                    return true;
-                }
-            }
-#endif
-        }
-
         // If a node is firing event listeners, its wrapper is observable because
         // its wrapper is responsible for marking those event listeners.

trunk/Source/WebCore/html/HTMLImageElement.cpp

@@ -74 +74 @@
 HTMLImageElement::HTMLImageElement(const QualifiedName& tagName, Document& document, HTMLFormElement* form)
     : HTMLElement(tagName, document)
+    , ActiveDOMObject(document)
     , m_imageLoader(makeUnique<HTMLImageLoader>(*this))
     , m_form(nullptr)
@@ -86 +87 @@
 Ref<HTMLImageElement> HTMLImageElement::create(Document& document)
 {
-    return adoptRef(*new HTMLImageElement(imgTag, document));
+    auto image = adoptRef(*new HTMLImageElement(imgTag, document));
+    image->suspendIfNeeded();
+    return image;
 }
 
 Ref<HTMLImageElement> HTMLImageElement::create(const QualifiedName& tagName, Document& document, HTMLFormElement* form)
 {
-    return adoptRef(*new HTMLImageElement(tagName, document, form));
+    auto image = adoptRef(*new HTMLImageElement(tagName, document, form));
+    image->suspendIfNeeded();
+    return image;
 }
 
@@ -109 +114 @@
     if (height)
         image->setHeight(height.value());
+    image->suspendIfNeeded();
     return image;
 }
@@ -867 +873 @@
 }
 
-bool HTMLImageElement::hasPendingActivity() const
+const char* HTMLImageElement::activeDOMObjectName() const
+{
+    return "HTMLImageElement";
+}
+
+bool HTMLImageElement::virtualHasPendingActivity() const
 {
     return m_imageLoader->hasPendingActivity();

trunk/Source/WebCore/html/HTMLImageElement.h

@@ -24 +24 @@
 #pragma once
 
+#include "ActiveDOMObject.h"
 #include "DecodingOptions.h"
 #include "FormNamedItem.h"
@@ -46 +47 @@
 enum class RelevantMutation : bool;
 
-class HTMLImageElement : public HTMLElement, public FormNamedItem {
+class HTMLImageElement : public HTMLElement, public FormNamedItem, public ActiveDOMObject {
     WTF_MAKE_ISO_ALLOCATED(HTMLImageElement);
     friend class HTMLFormElement;
@@ -110 +111 @@
 #endif
 
-    bool hasPendingActivity() const;
     WEBCORE_EXPORT size_t pendingDecodePromisesCountForTesting() const;
 
@@ -166 +166 @@
     void invalidateAttributeMapping();
 
+    // ActiveDOMObject.
+    const char* activeDOMObjectName() const final;
+    bool virtualHasPendingActivity() const final;
+
     void didAttachRenderers() override;
     RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) override;

trunk/Source/WebCore/html/HTMLImageElement.idl

@@ -21 +21 @@
 // https://html.spec.whatwg.org/multipage/embedded-content.html#htmlimageelement
 [
+    ActiveDOMObject,
     ExportMacro=WEBCORE_EXPORT,
+    Exposed=Window,
     JSGenerateToNativeObject,
-    Exposed=Window,
     LegacyFactoryFunctionCallWith=CurrentDocument,
     LegacyFactoryFunction=Image(optional unsigned long width, optional unsigned long height)

trunk/Source/WebCore/html/ImageDocument.cpp

@@ -117 +117 @@
 inline Ref<ImageDocumentElement> ImageDocumentElement::create(ImageDocument& document)
 {
-    return adoptRef(*new ImageDocumentElement(document));
+    auto image = adoptRef(*new ImageDocumentElement(document));
+    image->suspendIfNeeded();
+    return image;
 }