Title: [93567] trunk/Source/WebCore
- Revision
- 93567
- Author
- [email protected]
- Date
- 2011-08-22 17:43:32 -0700 (Mon, 22 Aug 2011)
Log Message
showModalDialog does not correctly return the defined returnValue in case domain relaxing is used
https://bugs.webkit.org/show_bug.cgi?id=53191
<rdar://problem/8629478>
Reviewed by Geoff Garen.
Cannot test domain relaxing, we only have 127.0.0.1 and localhost.
* bindings/js/JSDOMWindowCustom.cpp: (WebCore::JSDOMWindow::getOwnPropertySlot):
Added a FIXME about a difference with Firefox.
(WebCore::DialogHandler::DialogHandler):
(WebCore::DialogHandler::dialogCreated):
(WebCore::DialogHandler::returnValue):
Changed to fetch returnValue from the global object that's in the frame when the dialog is
dismissed. A dialog can navigate itself, and it also creates a new JSDOMWindow on first load
if the origin doesn't match opener origin (which the case with domain relaxing).
Re-added a security check for returnValue that got lost in r73829, so that we don't send the
result across origins. This matches Firefox.
* bindings/js/JSDOMWindowShell.cpp: (WebCore::JSDOMWindowShell::setWindow): Added an assertion
against a very confusing case that should no longer occur.
* page/Frame.cpp: (WebCore::Frame::pageDestroyed): Don't clear the window shell, it doesn't
seem necessary, but prevents DialogHandler from fetching the return value. Added a keepAlive
call to avoid changing life support timer behavior in this patch.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (93566 => 93567)
--- trunk/Source/WebCore/ChangeLog 2011-08-23 00:37:15 UTC (rev 93566)
+++ trunk/Source/WebCore/ChangeLog 2011-08-23 00:43:32 UTC (rev 93567)
@@ -1,3 +1,32 @@
+2011-08-22 Alexey Proskuryakov <[email protected]>
+
+ showModalDialog does not correctly return the defined returnValue in case domain relaxing is used
+ https://bugs.webkit.org/show_bug.cgi?id=53191
+ <rdar://problem/8629478>
+
+ Reviewed by Geoff Garen.
+
+ Cannot test domain relaxing, we only have 127.0.0.1 and localhost.
+
+ * bindings/js/JSDOMWindowCustom.cpp: (WebCore::JSDOMWindow::getOwnPropertySlot):
+ Added a FIXME about a difference with Firefox.
+
+ (WebCore::DialogHandler::DialogHandler):
+ (WebCore::DialogHandler::dialogCreated):
+ (WebCore::DialogHandler::returnValue):
+ Changed to fetch returnValue from the global object that's in the frame when the dialog is
+ dismissed. A dialog can navigate itself, and it also creates a new JSDOMWindow on first load
+ if the origin doesn't match opener origin (which the case with domain relaxing).
+ Re-added a security check for returnValue that got lost in r73829, so that we don't send the
+ result across origins. This matches Firefox.
+
+ * bindings/js/JSDOMWindowShell.cpp: (WebCore::JSDOMWindowShell::setWindow): Added an assertion
+ against a very confusing case that should no longer occur.
+
+ * page/Frame.cpp: (WebCore::Frame::pageDestroyed): Don't clear the window shell, it doesn't
+ seem necessary, but prevents DialogHandler from fetching the return value. Added a keepAlive
+ call to avoid changing life support timer behavior in this patch.
+
2011-08-22 Alice Boxhall <[email protected]>
Reviewed by Chris Fleizach.
Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (93566 => 93567)
--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2011-08-23 00:37:15 UTC (rev 93566)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2011-08-23 00:43:32 UTC (rev 93567)
@@ -127,7 +127,9 @@
const HashEntry* entry;
- // We don't want any properties other than "close" and "closed" on a closed window.
+ // We don't want any properties other than "close" and "closed" on a frameless window (i.e. one whose page got closed,
+ // or whose iframe got removed).
+ // FIXME: This doesn't fully match Firefox, which allows at least toString in addition to those.
if (!impl()->frame()) {
// The following code is safe for cross-domain and same domain use.
// It ignores any custom properties that might be set on the DOMWindow (including a custom prototype).
@@ -644,7 +646,6 @@
public:
explicit DialogHandler(ExecState* exec)
: m_exec(exec)
- , m_globalObject(0)
{
}
@@ -653,25 +654,27 @@
private:
ExecState* m_exec;
- JSDOMWindow* m_globalObject;
+ RefPtr<Frame> m_frame;
};
inline void DialogHandler::dialogCreated(DOMWindow* dialog)
{
+ m_frame = dialog->frame();
// FIXME: This looks like a leak between the normal world and an isolated
// world if dialogArguments comes from an isolated world.
- m_globalObject = toJSDOMWindow(dialog->frame(), normalWorld(m_exec->globalData()));
+ JSDOMWindow* globalObject = toJSDOMWindow(m_frame.get(), normalWorld(m_exec->globalData()));
if (JSValue dialogArguments = m_exec->argument(1))
- m_globalObject->putDirect(m_exec->globalData(), Identifier(m_exec, "dialogArguments"), dialogArguments);
+ globalObject->putDirect(m_exec->globalData(), Identifier(m_exec, "dialogArguments"), dialogArguments);
}
inline JSValue DialogHandler::returnValue() const
{
- if (!m_globalObject)
+ JSDOMWindow* globalObject = toJSDOMWindow(m_frame.get(), normalWorld(m_exec->globalData()));
+ if (!globalObject)
return jsUndefined();
Identifier identifier(m_exec, "returnValue");
PropertySlot slot;
- if (!m_globalObject->JSGlobalObject::getOwnPropertySlot(m_exec, identifier, slot))
+ if (!globalObject->getOwnPropertySlot(m_exec, identifier, slot))
return jsUndefined();
return slot.getValue(m_exec, identifier);
}
Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowShell.cpp (93566 => 93567)
--- trunk/Source/WebCore/bindings/js/JSDOMWindowShell.cpp 2011-08-23 00:37:15 UTC (rev 93566)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowShell.cpp 2011-08-23 00:43:32 UTC (rev 93567)
@@ -57,6 +57,9 @@
void JSDOMWindowShell::setWindow(PassRefPtr<DOMWindow> domWindow)
{
+ // Replacing JSDOMWindow via telling JSDOMWindowShell to use the same DOMWindow it already uses makes no sense,
+ // so we'd better never try to.
+ ASSERT(!m_window || domWindow.get() != m_window->impl());
// Explicitly protect the global object's prototype so it isn't collected
// when we allocate the global object. (Once the global object is fully
// constructed, it can mark its own prototype.)
Modified: trunk/Source/WebCore/page/Frame.cpp (93566 => 93567)
--- trunk/Source/WebCore/page/Frame.cpp 2011-08-23 00:37:15 UTC (rev 93566)
+++ trunk/Source/WebCore/page/Frame.cpp 2011-08-23 00:43:32 UTC (rev 93567)
@@ -731,6 +731,9 @@
void Frame::pageDestroyed()
{
+ // FIXME: Rename this function, since it's called not only from Page destructor, but in several other cases.
+ // This cleanup is needed whenever we remove a frame from page.
+
if (Frame* parent = tree()->parent())
parent->loader()->checkLoadComplete();
@@ -749,7 +752,10 @@
if (page() && page()->focusController()->focusedFrame() == this)
page()->focusController()->setFocusedFrame(0);
- script()->clearWindowShell();
+ // FIXME: Removing keepalive will make behavior better match Firefox.
+ // Some code that used to be here indirectly triggered keepalive, and we don't want to change behavior at the moment.
+ keepAlive();
+
script()->clearScriptObjects();
script()->updatePlatformScriptObjects();
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
