Title: [201638] trunk/Source/WebCore
Revision
201638
Author
barraclo...@apple.com
Date
2016-06-03 00:10:04 -0700 (Fri, 03 Jun 2016)

Log Message

Refactor showModalDialog handling in JSDOMWindowCustom
https://bugs.webkit.org/show_bug.cgi?id=158294

Reviewed by Ryosuke Niwa.

The showModalDialog property is currently implemented in a way that  effectively
results in duplication of the tail of the function, but modified to call
Base::getOwnPropertySlot instead of getStaticPropertySlot. It does so based on the
assumption that Base::getOwnPropertySlot is not going to search the static tables
(containing the property we wish to omit).

However as a part of bug #158178 I plan to change it such that Base::getOwnPropertySlot
does also search the static tables. So, refactoring this code to no longer depend on
using a function that bypasses the static tables. Always perform a lookup that will
check both property storage & static tables. If the object does contain the property,
then check explicitly for the value we're intending to suppress.

Covered by exsiting tests.

* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::JSDOMWindow::getOwnPropertySlot):
    - Check result of getStaticPropertySlot for showModalDialog function.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (201637 => 201638)


--- trunk/Source/WebCore/ChangeLog	2016-06-03 05:40:40 UTC (rev 201637)
+++ trunk/Source/WebCore/ChangeLog	2016-06-03 07:10:04 UTC (rev 201638)
@@ -1,3 +1,28 @@
+2016-06-02  Gavin Barraclough  <barraclo...@apple.com>
+
+        Refactor showModalDialog handling in JSDOMWindowCustom
+        https://bugs.webkit.org/show_bug.cgi?id=158294
+
+        Reviewed by Ryosuke Niwa.
+
+        The showModalDialog property is currently implemented in a way that  effectively
+        results in duplication of the tail of the function, but modified to call
+        Base::getOwnPropertySlot instead of getStaticPropertySlot. It does so based on the
+        assumption that Base::getOwnPropertySlot is not going to search the static tables
+        (containing the property we wish to omit).
+
+        However as a part of bug #158178 I plan to change it such that Base::getOwnPropertySlot
+        does also search the static tables. So, refactoring this code to no longer depend on
+        using a function that bypasses the static tables. Always perform a lookup that will
+        check both property storage & static tables. If the object does contain the property,
+        then check explicitly for the value we're intending to suppress.
+
+        Covered by exsiting tests.
+
+        * bindings/js/JSDOMWindowCustom.cpp:
+        (WebCore::JSDOMWindow::getOwnPropertySlot):
+            - Check result of getStaticPropertySlot for showModalDialog function.
+
 2016-06-02  Brady Eidson  <beid...@apple.com>
 
         Fix AtomicString regression caused by r201603.

Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (201637 => 201638)


--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2016-06-03 05:40:40 UTC (rev 201637)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2016-06-03 07:10:04 UTC (rev 201638)
@@ -55,6 +55,8 @@
 
 namespace WebCore {
 
+EncodedJSValue JSC_HOST_CALL jsDOMWindowInstanceFunctionShowModalDialog(ExecState*);
+
 void JSDOMWindow::visitAdditionalChildren(SlotVisitor& visitor)
 {
     if (Frame* frame = wrapped().frame())
@@ -232,17 +234,19 @@
     // (Particularly, is it correct that this exists here but not in getOwnPropertySlotByIndex?)
     slot.setWatchpointSet(thisObject->m_windowCloseWatchpoints);
 
-    if (propertyName == exec->propertyNames().showModalDialog) {
-        if (Base::getOwnPropertySlot(thisObject, exec, propertyName, slot))
+    // (2) Regular own properties.
+    PropertySlot slotCopy = slot;
+    if (getStaticPropertySlot<JSDOMWindow, Base>(exec, *JSDOMWindow::info()->staticPropHashTable, thisObject, propertyName, slot)) {
+        // Detect when we're getting the property 'showModalDialog', this is disabled, and has its original value.
+        bool isShowModalDialogAndShouldHide = propertyName == exec->propertyNames().showModalDialog
+            && !DOMWindow::canShowModalDialog(frame)
+            && slot.isValue() && isHostFunction(slot.getValue(exec, propertyName), jsDOMWindowInstanceFunctionShowModalDialog);
+        // Unless we're in the showModalDialog special case, we're done.
+        if (!isShowModalDialogAndShouldHide)
             return true;
-        if (!DOMWindow::canShowModalDialog(frame))
-            return jsDOMWindowGetOwnPropertySlotNamedItemGetter(thisObject, *frame, exec, propertyName, slot);
+        slot = slotCopy;
     }
 
-    // (2) Regular own properties.
-    if (getStaticPropertySlot<JSDOMWindow, Base>(exec, *JSDOMWindow::info()->staticPropHashTable, thisObject, propertyName, slot))
-        return true;
-
 #if ENABLE(USER_MESSAGE_HANDLERS)
     if (propertyName == exec->propertyNames().webkit && thisObject->wrapped().shouldHaveWebKitNamespaceForWorld(thisObject->world())) {
         slot.setCacheableCustom(thisObject, DontDelete | ReadOnly, jsDOMWindowWebKit);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to