- Revision
- 288763
- Author
- [email protected]
- Date
- 2022-01-28 16:05:54 -0800 (Fri, 28 Jan 2022)
Log Message
Remove showModalDialog-specific logic from JSDOMWindow::getOwnPropertySlot()
https://bugs.webkit.org/show_bug.cgi?id=234282
Reviewed by Yusuke Suzuki.
Source/WebCore:
This change removes showModalDialog-specific logic from JSDOMWindow::getOwnPropertySlot()
in favor of a setter-less CustomValue property on `window`, which returns a function only
if modals are allowed, and can be overriden by userland code.
Since we need to support setCanRunModal(true) being called after DOMWindow is initialized,
[EnabledByQuirk] and friends could not be used. However, once the function was exposed,
there is no point in hiding it, so the CustomValue getter replaces itself with a regular
data property to preserve function's identity (covered by fast/dom/wrapper-identity.html).
The patch makes JSDOMWindow's getOwnPropertySlot() consistent with its getOwnPropertyNames()
regarding the presence of "showModalDialog" property, and fixes JSDOMWindow::getOwnPropertySlot()
being ignored by LLInt when accessing "showModalDialog" from scope, which resulted in exposing
the function even if modals were disallowed.
Test: fast/dom/Window/forbid-showModalDialog.html
* bindings/js/JSDOMWindowBase.cpp:
(WebCore::JSDOMWindowBase::finishCreation):
* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::JSDOMWindow::getOwnPropertySlot):
(WebCore::JSC_DEFINE_CUSTOM_GETTER):
(WebCore::JSC_DEFINE_HOST_FUNCTION):
(WebCore::JSDOMWindow::showModalDialog): Deleted.
* bindings/js/JSDOMWindowCustom.h:
* page/DOMWindow.idl:
LayoutTests:
* fast/dom/Window/forbid-showModalDialog-expected.txt:
* fast/dom/Window/forbid-showModalDialog.html:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (288762 => 288763)
--- trunk/LayoutTests/ChangeLog 2022-01-28 23:12:40 UTC (rev 288762)
+++ trunk/LayoutTests/ChangeLog 2022-01-29 00:05:54 UTC (rev 288763)
@@ -1,3 +1,13 @@
+2022-01-28 Alexey Shvayka <[email protected]>
+
+ Remove showModalDialog-specific logic from JSDOMWindow::getOwnPropertySlot()
+ https://bugs.webkit.org/show_bug.cgi?id=234282
+
+ Reviewed by Yusuke Suzuki.
+
+ * fast/dom/Window/forbid-showModalDialog-expected.txt:
+ * fast/dom/Window/forbid-showModalDialog.html:
+
2022-01-28 Jon Lee <[email protected]>
Add copy-to-clipboard button for fuzzy matching meta tag
Modified: trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog-expected.txt (288762 => 288763)
--- trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog-expected.txt 2022-01-28 23:12:40 UTC (rev 288762)
+++ trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog-expected.txt 2022-01-29 00:05:54 UTC (rev 288763)
@@ -6,11 +6,12 @@
Make sure window.showModalDialog is undefined when modal dialogs are not allowed.
internals.setCanShowModalDialogOverride(false)
PASS window.showModalDialog is undefined
-PASS window.hasOwnProperty('showModalDialog') is false
+PASS window.hasOwnProperty('showModalDialog') is true
+PASS Object.getOwnPropertyNames(window).includes('showModalDialog') is true
Tests having a named property with name 'showModalDialog'.
document.body.append(testFrame)
-PASS window.showModalDialog is testFrame.contentWindow
+PASS window.showModalDialog is undefined
testFrame.remove()
PASS window.showModalDialog is undefined
@@ -19,6 +20,7 @@
PASS window.showModalDialog is not undefined
PASS window.hasOwnProperty('showModalDialog') is true
PASS window.showModalDialog is an instance of Function
+PASS window.showModalDialog.name is 'showModalDialog'
Make sure window.showModalDialog can be shadowed.
window.showModalDialog = 1
Modified: trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog.html (288762 => 288763)
--- trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog.html 2022-01-28 23:12:40 UTC (rev 288762)
+++ trunk/LayoutTests/fast/dom/Window/forbid-showModalDialog.html 2022-01-29 00:05:54 UTC (rev 288763)
@@ -8,8 +8,11 @@
debug("Make sure window.showModalDialog is undefined when modal dialogs are not allowed.");
evalAndLog("internals.setCanShowModalDialogOverride(false)");
shouldBe("window.showModalDialog", "undefined");
-shouldBeFalse("window.hasOwnProperty('showModalDialog')");
+shouldBeTrue("window.hasOwnProperty('showModalDialog')");
+shouldBeTrue("Object.getOwnPropertyNames(window).includes('showModalDialog')");
+if (showModalDialog !== undefined)
+ testFailed("showModalDialog !== undefined");
debug("");
debug("Tests having a named property with name 'showModalDialog'.");
@@ -16,7 +19,7 @@
var testFrame = document.createElement("iframe");
testFrame.name = "showModalDialog";
evalAndLog("document.body.append(testFrame)");
-shouldBe("window.showModalDialog", "testFrame.contentWindow");
+shouldBe("window.showModalDialog", "undefined");
evalAndLog("testFrame.remove()");
shouldBe("window.showModalDialog", "undefined");
@@ -26,6 +29,7 @@
shouldNotBe("window.showModalDialog", "undefined");
shouldBeTrue("window.hasOwnProperty('showModalDialog')");
shouldBeType("window.showModalDialog", "Function");
+shouldBe("window.showModalDialog.name", "'showModalDialog'");
debug("");
debug("Make sure window.showModalDialog can be shadowed.");
Modified: trunk/Source/WebCore/ChangeLog (288762 => 288763)
--- trunk/Source/WebCore/ChangeLog 2022-01-28 23:12:40 UTC (rev 288762)
+++ trunk/Source/WebCore/ChangeLog 2022-01-29 00:05:54 UTC (rev 288763)
@@ -1,3 +1,36 @@
+2022-01-28 Alexey Shvayka <[email protected]>
+
+ Remove showModalDialog-specific logic from JSDOMWindow::getOwnPropertySlot()
+ https://bugs.webkit.org/show_bug.cgi?id=234282
+
+ Reviewed by Yusuke Suzuki.
+
+ This change removes showModalDialog-specific logic from JSDOMWindow::getOwnPropertySlot()
+ in favor of a setter-less CustomValue property on `window`, which returns a function only
+ if modals are allowed, and can be overriden by userland code.
+
+ Since we need to support setCanRunModal(true) being called after DOMWindow is initialized,
+ [EnabledByQuirk] and friends could not be used. However, once the function was exposed,
+ there is no point in hiding it, so the CustomValue getter replaces itself with a regular
+ data property to preserve function's identity (covered by fast/dom/wrapper-identity.html).
+
+ The patch makes JSDOMWindow's getOwnPropertySlot() consistent with its getOwnPropertyNames()
+ regarding the presence of "showModalDialog" property, and fixes JSDOMWindow::getOwnPropertySlot()
+ being ignored by LLInt when accessing "showModalDialog" from scope, which resulted in exposing
+ the function even if modals were disallowed.
+
+ Test: fast/dom/Window/forbid-showModalDialog.html
+
+ * bindings/js/JSDOMWindowBase.cpp:
+ (WebCore::JSDOMWindowBase::finishCreation):
+ * bindings/js/JSDOMWindowCustom.cpp:
+ (WebCore::JSDOMWindow::getOwnPropertySlot):
+ (WebCore::JSC_DEFINE_CUSTOM_GETTER):
+ (WebCore::JSC_DEFINE_HOST_FUNCTION):
+ (WebCore::JSDOMWindow::showModalDialog): Deleted.
+ * bindings/js/JSDOMWindowCustom.h:
+ * page/DOMWindow.idl:
+
2022-01-28 Chris Dumez <[email protected]>
ASSERTION FAILED: m_processCallback in WebCore::AudioWorkletProcessor::process
Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp (288762 => 288763)
--- trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp 2022-01-28 23:12:40 UTC (rev 288762)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp 2022-01-29 00:05:54 UTC (rev 288763)
@@ -127,6 +127,8 @@
if (m_wrapped && m_wrapped->frame() && m_wrapped->frame()->settings().needsSiteSpecificQuirks())
setNeedsSiteSpecificQuirks(true);
+
+ putDirectCustomAccessor(vm, static_cast<JSVMClientData*>(vm.clientData)->builtinNames().showModalDialogPublicName(), CustomGetterSetter::create(vm, showModalDialogGetter, nullptr), static_cast<unsigned>(PropertyAttribute::CustomValue));
}
void JSDOMWindowBase::destroy(JSCell* cell)
Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (288762 => 288763)
--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2022-01-28 23:12:40 UTC (rev 288762)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2022-01-29 00:05:54 UTC (rev 288763)
@@ -208,20 +208,9 @@
slot.setWatchpointSet(*thisObject->m_windowCloseWatchpoints);
// (2) Regular own properties.
- PropertySlot slotCopy = slot;
- if (Base::getOwnPropertySlot(thisObject, lexicalGlobalObject, propertyName, slot)) {
- auto* frame = thisObject->wrapped().frame();
-
- // Detect when we're getting the property 'showModalDialog', this is disabled, and has its original value.
- bool isShowModalDialogAndShouldHide = propertyName == static_cast<JSVMClientData*>(lexicalGlobalObject->vm().clientData)->builtinNames().showModalDialogPublicName()
- && (!frame || !DOMWindow::canShowModalDialog(*frame))
- && slot.isValue() && isHostFunction(slot.getValue(lexicalGlobalObject, propertyName), s_info.staticPropHashTable->entry(propertyName)->function());
- // Unless we're in the showModalDialog special case, we're done.
- if (!isShowModalDialogAndShouldHide)
- return true;
- slot = slotCopy;
-
- } else if (UNLIKELY(slot.isVMInquiry() && slot.isTaintedByOpaqueObject()))
+ if (Base::getOwnPropertySlot(thisObject, lexicalGlobalObject, propertyName, slot))
+ return true;
+ if (UNLIKELY(slot.isVMInquiry() && slot.isTaintedByOpaqueObject()))
return false;
#if ENABLE(USER_MESSAGE_HANDLERS)
@@ -499,26 +488,60 @@
return slot.getValue(&m_globalObject, identifier);
}
-JSValue JSDOMWindow::showModalDialog(JSGlobalObject& lexicalGlobalObject, CallFrame& callFrame)
+static JSC_DECLARE_HOST_FUNCTION(showModalDialog);
+
+JSC_DEFINE_CUSTOM_GETTER(showModalDialogGetter, (JSGlobalObject* lexicalGlobalObject, EncodedJSValue thisValue, PropertyName propertyName))
{
+ VM& vm = lexicalGlobalObject->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+
+ auto* thisObject = castThisValue<JSDOMWindow>(*lexicalGlobalObject, JSValue::decode(thisValue));
+ if (UNLIKELY(!thisObject))
+ return throwVMDOMAttributeGetterTypeError(lexicalGlobalObject, scope, JSDOMWindow::info(), propertyName);
+
+ if (auto* frame = thisObject->wrapped().frame()) {
+ if (DOMWindow::canShowModalDialog(*frame)) {
+ auto* jsFunction = JSFunction::create(vm, lexicalGlobalObject, 1, "showModalDialog"_s, showModalDialog);
+ thisObject->putDirect(vm, propertyName, jsFunction);
+ return JSValue::encode(jsFunction);
+ }
+ }
+
+ return JSValue::encode(jsUndefined());
+}
+
+JSC_DEFINE_HOST_FUNCTION(showModalDialog, (JSGlobalObject* lexicalGlobalObjectPtr, CallFrame* callFramePtr))
+{
+ auto& lexicalGlobalObject = *lexicalGlobalObjectPtr;
+ auto& callFrame = *callFramePtr;
+
VM& vm = lexicalGlobalObject.vm();
auto scope = DECLARE_THROW_SCOPE(vm);
+ auto* thisObject = IDLOperation<JSDOMWindow>::cast(lexicalGlobalObject, callFrame);
+ if (UNLIKELY(!thisObject))
+ return throwThisTypeError(lexicalGlobalObject, scope, "Window"_s, "showModalDialog"_s);
+
+ bool shouldAllowAccess = BindingSecurity::shouldAllowAccessToDOMWindow(lexicalGlobalObjectPtr, thisObject->wrapped(), ThrowSecurityError);
+ EXCEPTION_ASSERT_UNUSED(scope, !scope.exception() || !shouldAllowAccess);
+ if (!shouldAllowAccess)
+ return JSValue::encode(jsUndefined());
+
if (UNLIKELY(callFrame.argumentCount() < 1))
- return throwException(&lexicalGlobalObject, scope, createNotEnoughArgumentsError(&lexicalGlobalObject));
+ return throwVMException(&lexicalGlobalObject, scope, createNotEnoughArgumentsError(&lexicalGlobalObject));
String urlString = convert<IDLNullable<IDLDOMString>>(lexicalGlobalObject, callFrame.argument(0));
- RETURN_IF_EXCEPTION(scope, JSValue());
+ RETURN_IF_EXCEPTION(scope, { });
String dialogFeaturesString = convert<IDLNullable<IDLDOMString>>(lexicalGlobalObject, callFrame.argument(2));
- RETURN_IF_EXCEPTION(scope, JSValue());
+ RETURN_IF_EXCEPTION(scope, { });
DialogHandler handler(lexicalGlobalObject, callFrame);
- wrapped().showModalDialog(urlString, dialogFeaturesString, activeDOMWindow(lexicalGlobalObject), firstDOMWindow(lexicalGlobalObject), [&handler](DOMWindow& dialog) {
+ thisObject->wrapped().showModalDialog(urlString, dialogFeaturesString, activeDOMWindow(lexicalGlobalObject), firstDOMWindow(lexicalGlobalObject), [&handler](DOMWindow& dialog) {
handler.dialogCreated(dialog);
});
- return handler.returnValue();
+ return JSValue::encode(handler.returnValue());
}
JSValue JSDOMWindow::queueMicrotask(JSGlobalObject& lexicalGlobalObject, CallFrame& callFrame)
Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.h (288762 => 288763)
--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.h 2022-01-28 23:12:40 UTC (rev 288762)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.h 2022-01-29 00:05:54 UTC (rev 288763)
@@ -54,4 +54,6 @@
return frame ? &mainWorldGlobalObject(*frame) : nullptr;
}
+JSC_DECLARE_CUSTOM_GETTER(showModalDialogGetter);
+
} // namespace WebCore
Modified: trunk/Source/WebCore/page/DOMWindow.idl (288762 => 288763)
--- trunk/Source/WebCore/page/DOMWindow.idl 2022-01-28 23:12:40 UTC (rev 288762)
+++ trunk/Source/WebCore/page/DOMWindow.idl 2022-01-29 00:05:54 UTC (rev 288763)
@@ -114,9 +114,6 @@
[NotEnumerable, Conditional=IOS_GESTURE_EVENTS] attribute EventHandler ongestureend;
[NotEnumerable, Conditional=IOS_GESTURE_EVENTS] attribute EventHandler ongesturestart;
- // Non-standard: This has been dropped from the HTML specification and by other browsers.
- [Custom] any showModalDialog(DOMString url, optional any dialogArgs, optional DOMString featureArgs);
-
// Non-standard: should probably deprecate this (https://bugs.webkit.org/show_bug.cgi?id=79653).
// Blink already deprecated it (https://bugs.chromium.org/p/chromium/issues/detail?id=437569).
CSSRuleList getMatchedCSSRules(optional Element? element = null, optional DOMString? pseudoElement = null);
