Diff
Modified: trunk/Source/WebCore/ChangeLog (274845 => 274846)
--- trunk/Source/WebCore/ChangeLog 2021-03-23 05:26:58 UTC (rev 274845)
+++ trunk/Source/WebCore/ChangeLog 2021-03-23 05:53:22 UTC (rev 274846)
@@ -1,3 +1,42 @@
+2021-03-22 Ryosuke Niwa <[email protected]>
+
+ Use JSValueInWrappedObject to keep the JSObject alive for QuickTimePluginReplacement
+ https://bugs.webkit.org/show_bug.cgi?id=223561
+ <rdar://75634407>
+
+ Reviewed by Geoffrey Garen.
+
+ This patch replaces QuickTimePluginReplacement's m_scriptObject member variable by
+ JSValueInWrappedObject in HTMLPlugInElement and hooks it up with the slot visitor.
+
+ Also use WeakPtr instead of a raw pointer for the back reference to the plugin element
+ in QuickTimePluginReplacement and YouTubePluginReplacement.
+
+ * Modules/plugins/PluginReplacement.h:
+ (WebCore::PluginReplacement::InstallResult): Added; a boolean indicating the installation
+ had succeeded and the script object for QuickTimePluginReplacement.
+ (WebCore::PluginReplacement::scriptObject): Deleted.
+ * Modules/plugins/QuickTimePluginReplacement.h:
+ * Modules/plugins/QuickTimePluginReplacement.mm:
+ (WebCore::QuickTimePluginReplacement::QuickTimePluginReplacement): Deployed WeakPtr.
+ (WebCore::QuickTimePluginReplacement::~QuickTimePluginReplacement): Removed
+ the superflous de-initialization of the member variables.
+ (WebCore::QuickTimePluginReplacement::installReplacement): Now returns JSValue
+ instead of storing it in m_scriptObject.
+ * Modules/plugins/YouTubePluginReplacement.cpp:
+ (WebCore::YouTubePluginReplacement::YouTubePluginReplacement): Deployed WeakPtr.
+ (WebCore::YouTubePluginReplacement::installReplacement):
+ * Modules/plugins/YouTubePluginReplacement.h:
+ * bindings/scripts/CodeGeneratorJS.pm:
+ (InstanceNeedsVisitChildren): Generate visitChildren when an IDL has Plugin attribute.
+ (GenerateImplementation): Visit the newly added JSValueInWrappedObject.
+ * html/HTMLPlugInElement.cpp:
+ (WebCore::HTMLPlugInElement::didAddUserAgentShadowRoot): Store the script object returned
+ by QuickTimePluginReplacement::installReplacement in the newly added member variable.
+ (WebCore::HTMLPlugInElement::scriptObjectForPluginReplacement): Updated.
+ * html/HTMLPlugInElement.h:
+ (WebCore::HTMLPlugInElement::pluginReplacementScriptObject): Added. Used by visitChildren.
+
2021-03-22 Myles C. Maxfield <[email protected]>
[GPU Process]: Improve getImageData() perf part 1: Add a GetImageData display list item
Modified: trunk/Source/WebCore/Modules/plugins/PluginReplacement.h (274845 => 274846)
--- trunk/Source/WebCore/Modules/plugins/PluginReplacement.h 2021-03-23 05:26:58 UTC (rev 274845)
+++ trunk/Source/WebCore/Modules/plugins/PluginReplacement.h 2021-03-23 05:53:22 UTC (rev 274846)
@@ -26,6 +26,8 @@
#pragma once
#include "RenderPtr.h"
+#include <_javascript_Core/JSCJSValue.h>
+#include <_javascript_Core/JSCJSValueInlines.h>
#include <wtf/text/WTFString.h>
namespace JSC {
@@ -45,9 +47,15 @@
public:
virtual ~PluginReplacement() = default;
- virtual bool installReplacement(ShadowRoot&) = 0;
- virtual JSC::JSObject* scriptObject() { return nullptr; }
+ struct InstallResult {
+ bool success;
+#if PLATFORM(COCOA)
+ JSC::JSValue scriptObject { };
+#endif
+ };
+ virtual InstallResult installReplacement(ShadowRoot&) = 0;
+
virtual bool willCreateRenderer() { return false; }
virtual RenderPtr<RenderElement> createElementRenderer(HTMLPlugInElement&, RenderStyle&&, const RenderTreePosition&) = 0;
};
Modified: trunk/Source/WebCore/Modules/plugins/QuickTimePluginReplacement.h (274845 => 274846)
--- trunk/Source/WebCore/Modules/plugins/QuickTimePluginReplacement.h 2021-03-23 05:26:58 UTC (rev 274845)
+++ trunk/Source/WebCore/Modules/plugins/QuickTimePluginReplacement.h 2021-03-23 05:53:22 UTC (rev 274846)
@@ -26,6 +26,7 @@
#pragma once
#include "PluginReplacement.h"
+#include <wtf/WeakPtr.h>
namespace WebCore {
@@ -51,8 +52,7 @@
static bool supportsURL(const URL&) { return true; }
static bool isEnabledBySettings(const Settings&);
- bool installReplacement(ShadowRoot&) final;
- JSC::JSObject* scriptObject() final { return m_scriptObject; }
+ InstallResult installReplacement(ShadowRoot&) final;
bool willCreateRenderer() final { return m_mediaElement; }
RenderPtr<RenderElement> createElementRenderer(HTMLPlugInElement&, RenderStyle&&, const RenderTreePosition&) final;
@@ -60,11 +60,10 @@
bool ensureReplacementScriptInjected();
DOMWrapperWorld& isolatedWorld();
- HTMLPlugInElement* m_parentElement;
+ WeakPtr<HTMLPlugInElement> m_parentElement;
RefPtr<HTMLVideoElement> m_mediaElement;
const Vector<String> m_names;
const Vector<String> m_values;
- JSC::JSObject* m_scriptObject { nullptr }; // FIXME: Why is it safe to have this pointer here? What keeps it alive during GC?
};
}
Modified: trunk/Source/WebCore/Modules/plugins/QuickTimePluginReplacement.mm (274845 => 274846)
--- trunk/Source/WebCore/Modules/plugins/QuickTimePluginReplacement.mm 2021-03-23 05:26:58 UTC (rev 274845)
+++ trunk/Source/WebCore/Modules/plugins/QuickTimePluginReplacement.mm 2021-03-23 05:53:22 UTC (rev 274846)
@@ -109,19 +109,13 @@
}
QuickTimePluginReplacement::QuickTimePluginReplacement(HTMLPlugInElement& plugin, const Vector<String>& paramNames, const Vector<String>& paramValues)
- : m_parentElement(&plugin)
+ : m_parentElement(makeWeakPtr(plugin))
, m_names(paramNames)
, m_values(paramValues)
{
}
-QuickTimePluginReplacement::~QuickTimePluginReplacement()
-{
- // FIXME: Why is it useful to null out pointers in an object that is being destroyed?
- m_parentElement = nullptr;
- m_scriptObject = nullptr;
- m_mediaElement = nullptr;
-}
+QuickTimePluginReplacement::~QuickTimePluginReplacement() = default;
RenderPtr<RenderElement> QuickTimePluginReplacement::createElementRenderer(HTMLPlugInElement& plugin, RenderStyle&& style, const RenderTreePosition& insertionPosition)
{
@@ -166,13 +160,13 @@
return true;
}
-bool QuickTimePluginReplacement::installReplacement(ShadowRoot& root)
+auto QuickTimePluginReplacement::installReplacement(ShadowRoot& root) -> InstallResult
{
if (!ensureReplacementScriptInjected())
- return false;
+ return { false };
if (!m_parentElement->document().frame())
- return false;
+ return { false };
DOMWrapperWorld& world = isolatedWorld();
ScriptController& scriptController = m_parentElement->document().frame()->script();
@@ -185,16 +179,16 @@
// Lookup the "createPluginReplacement" function.
JSC::JSValue replacementFunction = globalObject->get(lexicalGlobalObject, JSC::Identifier::fromString(vm, "createPluginReplacement"));
if (replacementFunction.isUndefinedOrNull())
- return false;
+ return { false };
JSC::JSObject* replacementObject = replacementFunction.toObject(lexicalGlobalObject);
scope.assertNoException();
auto callData = getCallData(vm, replacementObject);
if (callData.type == JSC::CallData::Type::None)
- return false;
+ return { false };
JSC::MarkedArgumentBuffer argList;
argList.append(toJS(lexicalGlobalObject, globalObject, &root));
- argList.append(toJS(lexicalGlobalObject, globalObject, m_parentElement));
+ argList.append(toJS(lexicalGlobalObject, globalObject, m_parentElement.get()));
argList.append(toJS(lexicalGlobalObject, globalObject, this));
argList.append(toJS<IDLSequence<IDLNullable<IDLDOMString>>>(*lexicalGlobalObject, *globalObject, m_names));
argList.append(toJS<IDLSequence<IDLNullable<IDLDOMString>>>(*lexicalGlobalObject, *globalObject, m_values));
@@ -202,7 +196,7 @@
JSC::JSValue replacement = call(lexicalGlobalObject, replacementObject, callData, globalObject, argList);
if (UNLIKELY(scope.exception())) {
scope.clearException();
- return false;
+ return { false };
}
// Get the <video> created to replace the plug-in.
@@ -213,23 +207,18 @@
if (!m_mediaElement) {
LOG(Plugins, "%p - Failed to find <video> element created by QuickTime plugin replacement script.", this);
scope.clearException();
- return false;
+ return { false };
}
// Get the scripting interface.
value = replacement.get(lexicalGlobalObject, JSC::Identifier::fromString(vm, "scriptObject"));
- if (!scope.exception() && !value.isUndefinedOrNull()) {
- m_scriptObject = value.toObject(lexicalGlobalObject);
- scope.assertNoException();
- }
-
- if (!m_scriptObject) {
+ if (!value.isObject()) {
LOG(Plugins, "%p - Failed to find script object created by QuickTime plugin replacement.", this);
scope.clearException();
- return false;
+ return { false };
}
- return true;
+ return { true, value };
}
unsigned long long QuickTimePluginReplacement::movieSize() const
Modified: trunk/Source/WebCore/Modules/plugins/YouTubePluginReplacement.cpp (274845 => 274846)
--- trunk/Source/WebCore/Modules/plugins/YouTubePluginReplacement.cpp 2021-03-23 05:26:58 UTC (rev 274845)
+++ trunk/Source/WebCore/Modules/plugins/YouTubePluginReplacement.cpp 2021-03-23 05:53:22 UTC (rev 274846)
@@ -60,7 +60,7 @@
}
YouTubePluginReplacement::YouTubePluginReplacement(HTMLPlugInElement& plugin, const Vector<String>& paramNames, const Vector<String>& paramValues)
- : m_parentElement(&plugin)
+ : m_parentElement(makeWeakPtr(plugin))
{
ASSERT(paramNames.size() == paramValues.size());
for (size_t i = 0; i < paramNames.size(); ++i)
@@ -77,7 +77,7 @@
return m_embedShadowElement->createElementRenderer(WTFMove(style), insertionPosition);
}
-bool YouTubePluginReplacement::installReplacement(ShadowRoot& root)
+auto YouTubePluginReplacement::installReplacement(ShadowRoot& root) -> InstallResult
{
m_embedShadowElement = YouTubeEmbedShadowElement::create(m_parentElement->document());
@@ -86,7 +86,7 @@
auto iframeElement = HTMLIFrameElement::create(HTMLNames::iframeTag, m_parentElement->document());
if (m_attributes.contains("width"))
iframeElement->setAttributeWithoutSynchronization(HTMLNames::widthAttr, AtomString("100%", AtomString::ConstructFromLiteral));
-
+
const auto& heightValue = m_attributes.find("height");
if (heightValue != m_attributes.end()) {
iframeElement->setAttribute(HTMLNames::styleAttr, AtomString("max-height: 100%", AtomString::ConstructFromLiteral));
@@ -100,7 +100,7 @@
iframeElement->setAttributeWithoutSynchronization(HTMLNames::scrollingAttr, AtomString("no", AtomString::ConstructFromLiteral));
m_embedShadowElement->appendChild(iframeElement);
- return true;
+ return { true };
}
static URL createYouTubeURL(StringView videoID, StringView timeID)
Modified: trunk/Source/WebCore/Modules/plugins/YouTubePluginReplacement.h (274845 => 274846)
--- trunk/Source/WebCore/Modules/plugins/YouTubePluginReplacement.h 2021-03-23 05:26:58 UTC (rev 274845)
+++ trunk/Source/WebCore/Modules/plugins/YouTubePluginReplacement.h 2021-03-23 05:53:22 UTC (rev 274846)
@@ -27,6 +27,7 @@
#include "PluginReplacement.h"
#include <wtf/HashMap.h>
+#include <wtf/WeakPtr.h>
namespace WebCore {
@@ -48,7 +49,7 @@
static bool supportsURL(const URL&);
static bool isEnabledBySettings(const Settings&);
- bool installReplacement(ShadowRoot&) final;
+ InstallResult installReplacement(ShadowRoot&) final;
String youTubeURL(const String& rawURL);
@@ -55,7 +56,7 @@
bool willCreateRenderer() final { return m_embedShadowElement; }
RenderPtr<RenderElement> createElementRenderer(HTMLPlugInElement&, RenderStyle&&, const RenderTreePosition&) final;
- HTMLPlugInElement* m_parentElement;
+ WeakPtr<HTMLPlugInElement> m_parentElement;
RefPtr<YouTubeEmbedShadowElement> m_embedShadowElement;
KeyValueMap m_attributes;
};
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (274845 => 274846)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2021-03-23 05:26:58 UTC (rev 274845)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2021-03-23 05:53:22 UTC (rev 274846)
@@ -2207,6 +2207,7 @@
}
return 1 if $interface->extendedAttributes->{JSCustomMarkFunction};
+ return 1 if $interface->extendedAttributes->{Plugin};
return 1 if $interface->extendedAttributes->{ReportExtraMemoryCost};
return 0;
}
@@ -4921,6 +4922,11 @@
push(@implContent, " ASSERT_GC_OBJECT_INHERITS(thisObject, info());\n");
push(@implContent, " Base::visitChildren(thisObject, visitor);\n");
push(@implContent, " thisObject->visitAdditionalChildren(visitor);\n") if $interface->extendedAttributes->{JSCustomMarkFunction};
+ if ($interface->extendedAttributes->{Plugin}) {
+ push(@implContent, "#if PLATFORM(COCOA)\n");
+ push(@implContent, " thisObject->wrapped().pluginReplacementScriptObject().visit(visitor);\n");
+ push(@implContent, "#endif\n");
+ }
if ($interface->extendedAttributes->{ReportExtraMemoryCost}) {
push(@implContent, " visitor.reportExtraMemoryVisited(thisObject->wrapped().memoryCost());\n");
if ($interface->extendedAttributes->{ReportExternalMemoryCost}) {;
Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestPluginInterface.cpp (274845 => 274846)
--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestPluginInterface.cpp 2021-03-23 05:26:58 UTC (rev 274845)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestPluginInterface.cpp 2021-03-23 05:53:22 UTC (rev 274846)
@@ -254,6 +254,19 @@
return space;
}
+template<typename Visitor>
+void JSTestPluginInterface::visitChildrenImpl(JSCell* cell, Visitor& visitor)
+{
+ auto* thisObject = jsCast<JSTestPluginInterface*>(cell);
+ ASSERT_GC_OBJECT_INHERITS(thisObject, info());
+ Base::visitChildren(thisObject, visitor);
+#if PLATFORM(COCOA)
+ thisObject->wrapped().pluginReplacementScriptObject().visit(visitor);
+#endif
+}
+
+DEFINE_VISIT_CHILDREN(JSTestPluginInterface);
+
void JSTestPluginInterface::analyzeHeap(JSCell* cell, HeapAnalyzer& analyzer)
{
auto* thisObject = jsCast<JSTestPluginInterface*>(cell);
Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestPluginInterface.h (274845 => 274846)
--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestPluginInterface.h 2021-03-23 05:26:58 UTC (rev 274845)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestPluginInterface.h 2021-03-23 05:53:22 UTC (rev 274846)
@@ -63,6 +63,8 @@
return subspaceForImpl(vm);
}
static JSC::IsoSubspace* subspaceForImpl(JSC::VM& vm);
+ DECLARE_VISIT_CHILDREN;
+
static void analyzeHeap(JSCell*, JSC::HeapAnalyzer&);
public:
static constexpr unsigned StructureFlags = Base::StructureFlags | JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::OverridesGetCallData | JSC::OverridesGetOwnPropertySlot | JSC::ProhibitsPropertyCaching;
Modified: trunk/Source/WebCore/html/HTMLPlugInElement.cpp (274845 => 274846)
--- trunk/Source/WebCore/html/HTMLPlugInElement.cpp 2021-03-23 05:26:58 UTC (rev 274845)
+++ trunk/Source/WebCore/html/HTMLPlugInElement.cpp 2021-03-23 05:53:22 UTC (rev 274846)
@@ -293,7 +293,14 @@
return;
root.setResetStyleInheritance(true);
- if (m_pluginReplacement->installReplacement(root)) {
+ auto result = m_pluginReplacement->installReplacement(root);
+
+#if PLATFORM(COCOA)
+ RELEASE_ASSERT(result.success || !result.scriptObject);
+ m_pluginReplacementScriptObject = result.scriptObject;
+#endif
+
+ if (result.success) {
setDisplayState(DisplayingPluginReplacement);
invalidateStyleAndRenderersForSubtree();
}
@@ -389,9 +396,14 @@
JSC::JSObject* HTMLPlugInElement::scriptObjectForPluginReplacement()
{
- if (m_pluginReplacement)
- return m_pluginReplacement->scriptObject();
+#if PLATFORM(COCOA)
+ JSC::JSValue value = m_pluginReplacementScriptObject;
+ if (!value)
+ return nullptr;
+ return value.getObject();
+#else
return nullptr;
+#endif
}
bool HTMLPlugInElement::isBelowSizeThreshold() const
Modified: trunk/Source/WebCore/html/HTMLPlugInElement.h (274845 => 274846)
--- trunk/Source/WebCore/html/HTMLPlugInElement.h 2021-03-23 05:26:58 UTC (rev 274845)
+++ trunk/Source/WebCore/html/HTMLPlugInElement.h 2021-03-23 05:53:22 UTC (rev 274846)
@@ -24,6 +24,7 @@
#include "HTMLFrameOwnerElement.h"
#include "Image.h"
+#include "JSValueInWrappedObject.h"
#include "RenderEmbeddedObject.h"
namespace JSC {
@@ -59,6 +60,9 @@
void setDisplayState(DisplayState);
JSC::JSObject* scriptObjectForPluginReplacement();
+#if PLATFORM(COCOA)
+ JSValueInWrappedObject& pluginReplacementScriptObject() { return m_pluginReplacementScriptObject; }
+#endif
bool isCapturingMouseEvents() const { return m_isCapturingMouseEvents; }
void setIsCapturingMouseEvents(bool capturing) { m_isCapturingMouseEvents = capturing; }
@@ -116,6 +120,9 @@
RefPtr<JSC::Bindings::Instance> m_instance;
Timer m_swapRendererTimer;
RefPtr<PluginReplacement> m_pluginReplacement;
+#if PLATFORM(COCOA)
+ JSValueInWrappedObject m_pluginReplacementScriptObject;
+#endif
bool m_isCapturingMouseEvents { false };
bool m_inBeforeLoadEventHandler { false };
DisplayState m_displayState { Playing };
