Changeset 199087 in webkit


Ignore:
Timestamp:
Apr 5, 2016 6:18:05 PM (8 years ago)
Author:
Chris Dumez
Message:

MessageEvent.source window is incorrect once window has been reified
​https://bugs.webkit.org/show_bug.cgi?id=156227
<rdar://problem/25545831>

Reviewed by Mark Lam.

Source/WebCore:

MessageEvent.source window was incorrect once window had been reified.

If the Window had not been reified, we kept constructing new
postMessage() functions when calling window.postMessage(). We used to
pass activeDOMWindow(execState) as source Window to
DOMWindow::postMessage(). activeDOMWindow() uses
exec->lexicalGlobalObject() which did the right thing because we
used to construct a new postMessage() function in the caller's context.

However, after reification, due to the way JSDOMWindow::getOwnPropertySlot()
was implemented, we would stop constructing new postMessage() functions
when calling window.postMessage(). As a result, the source window would
become incorrect because exec->lexicalGlobalObject() would return the
target Window instead.

In this patch, the following is done:

  1. Stop constructing a new function every time in the same origin case for postMessage, blur, focus and close. This was inefficient and lead to incorrect behavior:
    • The behavior would differ depending if the Window is reified or not
    • It would be impossible to delete those operations, which is incompatible with the specification and other browsers (tested Firefox and Chrome).
  2. Use callerDOMWindow(execState) instead of activeDOMWindow(execState) as source Window in JSDOMWindow::handlePostMessage(). callerDOMWindow() is a new utility function that returns the caller's Window object.

Tests: fast/dom/Window/delete-operations.html

fast/dom/Window/messageevent-source-postmessage-reified.html
fast/dom/Window/messageevent-source-postmessage.html
fast/dom/Window/messageevent-source-postmessage2.html
fast/dom/Window/window-postmessage-clone-frames.html
fast/dom/Window/post-message-crash2.html

  • bindings/js/JSDOMBinding.cpp:

(WebCore::GetCallerCodeBlockFunctor::operator()):
(WebCore::GetCallerCodeBlockFunctor::codeBlock):
(WebCore::callerDOMWindow):

  • bindings/js/JSDOMBinding.h:
  • bindings/js/JSDOMWindowCustom.cpp:

(WebCore::handlePostMessage):

LayoutTests:

Add tests that cover using MessageEvent.source Window for messaging
using postMessage(). There are 2 versions of the test, one where the
main window is reified and one where it is not. The test that has a
reified main window was failing because this fix.

  • fast/dom/Window/delete-operations-expected.txt: Added.
  • fast/dom/Window/delete-operations.html: Added.

Make sure that operations on Window are indeed deletable. Previously,
it would be impossible to delete postMessage, blur, focus and close.

  • fast/dom/Window/messageevent-source-postmessage-expected.txt: Added.
  • fast/dom/Window/messageevent-source-postmessage-reified-expected.txt: Added.
  • fast/dom/Window/messageevent-source-postmessage-reified.html: Added.
  • fast/dom/Window/messageevent-source-postmessage.html: Added.
  • fast/dom/Window/messageevent-source-postmessage2.html: Added.
  • fast/dom/Window/resources/messageevent-source-postmessage-frame.html: Added.
  • fast/dom/Window/post-message-crash2-expected.txt: Added.
  • fast/dom/Window/post-message-crash2.html: Added.
Location:
trunk
Files:
11 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r199086 r199087  
     12016-04-05  Chris Dumez  <cdumez@apple.com>
     2
     3        MessageEvent.source window is incorrect once window has been reified
     4        https://bugs.webkit.org/show_bug.cgi?id=156227
     5        <rdar://problem/25545831>
     6
     7        Reviewed by Mark Lam.
     8
     9        Add tests that cover using MessageEvent.source Window for messaging
     10        using postMessage(). There are 2 versions of the test, one where the
     11        main window is reified and one where it is not. The test that has a
     12        reified main window was failing because this fix.
     13
     14        * fast/dom/Window/delete-operations-expected.txt: Added.
     15        * fast/dom/Window/delete-operations.html: Added.
     16        Make sure that operations on Window are indeed deletable. Previously,
     17        it would be impossible to delete postMessage, blur, focus and close.
     18
     19        * fast/dom/Window/messageevent-source-postmessage-expected.txt: Added.
     20        * fast/dom/Window/messageevent-source-postmessage-reified-expected.txt: Added.
     21        * fast/dom/Window/messageevent-source-postmessage-reified.html: Added.
     22        * fast/dom/Window/messageevent-source-postmessage.html: Added.
     23        * fast/dom/Window/messageevent-source-postmessage2.html: Added.
     24        * fast/dom/Window/resources/messageevent-source-postmessage-frame.html: Added.
     25        * fast/dom/Window/post-message-crash2-expected.txt: Added.
     26        * fast/dom/Window/post-message-crash2.html: Added.
     27
    1282016-04-05  Myles C. Maxfield  <mmaxfield@apple.com>
    229

  • trunk/Source/WebCore/ChangeLog

    r199083 r199087  
     12016-04-05  Chris Dumez  <cdumez@apple.com>
     2
     3        MessageEvent.source window is incorrect once window has been reified
     4        https://bugs.webkit.org/show_bug.cgi?id=156227
     5        <rdar://problem/25545831>
     6
     7        Reviewed by Mark Lam.
     8
     9        MessageEvent.source window was incorrect once window had been reified.
     10
     11        If the Window had not been reified, we kept constructing new
     12        postMessage() functions when calling window.postMessage(). We used to
     13        pass activeDOMWindow(execState) as source Window to
     14        DOMWindow::postMessage(). activeDOMWindow() uses
     15        exec->lexicalGlobalObject() which did the right thing because we
     16        used to construct a new postMessage() function in the caller's context.
     17
     18        However, after reification, due to the way JSDOMWindow::getOwnPropertySlot()
     19        was implemented, we would stop constructing new postMessage() functions
     20        when calling window.postMessage(). As a result, the source window would
     21        become incorrect because exec->lexicalGlobalObject() would return the
     22        target Window instead.
     23
     24        In this patch, the following is done:
     25        1. Stop constructing a new function every time in the same origin case
     26           for postMessage, blur, focus and close. This was inefficient and lead
     27           to incorrect behavior:
     28           - The behavior would differ depending if the Window is reified or not
     29           - It would be impossible to delete those operations, which is
     30             incompatible with the specification and other browsers (tested
     31             Firefox and Chrome).
     32        2. Use callerDOMWindow(execState) instead of activeDOMWindow(execState)
     33           as source Window in JSDOMWindow::handlePostMessage(). callerDOMWindow()
     34           is a new utility function that returns the caller's Window object.
     35
     36        Tests: fast/dom/Window/delete-operations.html
     37               fast/dom/Window/messageevent-source-postmessage-reified.html
     38               fast/dom/Window/messageevent-source-postmessage.html
     39               fast/dom/Window/messageevent-source-postmessage2.html
     40               fast/dom/Window/window-postmessage-clone-frames.html
     41               fast/dom/Window/post-message-crash2.html
     42
     43        * bindings/js/JSDOMBinding.cpp:
     44        (WebCore::GetCallerCodeBlockFunctor::operator()):
     45        (WebCore::GetCallerCodeBlockFunctor::codeBlock):
     46        (WebCore::callerDOMWindow):
     47        * bindings/js/JSDOMBinding.h:
     48        * bindings/js/JSDOMWindowCustom.cpp:
     49        (WebCore::handlePostMessage):
     50
    1512016-04-05  Beth Dakin  <bdakin@apple.com>
    252

  • trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp

    r198023 r199087  
    3636#include "JSExceptionBase.h"
    3737#include "SecurityOrigin.h"
     38#include <bytecode/CodeBlock.h>
    3839#include <inspector/ScriptCallStack.h>
    3940#include <inspector/ScriptCallStackFactory.h>
    … …  
    556557    doubleToInteger(x, n);
    557558    return n;
     559}
     560
     561class GetCallerGlobalObjectFunctor {
     562public:
     563    GetCallerGlobalObjectFunctor() = default;
     564
     565    StackVisitor::Status operator()(StackVisitor& visitor) const
     566    {
     567        if (!m_hasSkippedFirstFrame) {
     568            m_hasSkippedFirstFrame = true;
     569            return StackVisitor::Continue;
     570        }
     571
     572        if (auto* codeBlock = visitor->codeBlock())
     573            m_globalObject = codeBlock->globalObject();
     574        else {
     575            ASSERT(visitor->callee());
     576            m_globalObject = visitor->callee()->globalObject();
     577        }
     578        return StackVisitor::Done;
     579    }
     580
     581    JSGlobalObject* globalObject() const { return m_globalObject; }
     582
     583private:
     584    mutable bool m_hasSkippedFirstFrame { false };
     585    mutable JSGlobalObject* m_globalObject { nullptr };
     586};
     587
     588DOMWindow& callerDOMWindow(ExecState* exec)
     589{
     590    GetCallerGlobalObjectFunctor iter;
     591    exec->iterate(iter);
     592    return iter.globalObject() ? asJSDOMWindow(iter.globalObject())->wrapped() : firstDOMWindow(exec);
    558593}
    559594

  • trunk/Source/WebCore/bindings/js/JSDOMBinding.h

    r198023 r199087  
    7979typedef int ExceptionCode;
    8080
     81DOMWindow& callerDOMWindow(JSC::ExecState*);
    8182DOMWindow& activeDOMWindow(JSC::ExecState*);
    8283DOMWindow& firstDOMWindow(JSC::ExecState*);

  • trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp

    r199017 r199087  
    238238    slot.setWatchpointSet(thisObject->m_windowCloseWatchpoints);
    239239
    240     // FIXME: These are all bogus. Keeping these here make some tests pass that check these properties
    241     // are own properties of the window, but introduces other problems instead (e.g. if you overwrite
    242     // & delete then the original value is restored!) Should be removed.
    243     if (propertyName == exec->propertyNames().blur) {
    244         if (!Base::getOwnPropertySlot(thisObject, exec, propertyName, slot))
    245             slot.setCustom(thisObject, ReadOnly | DontDelete | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionBlur, 0>);
    246         return true;
    247     }
    248     if (propertyName == exec->propertyNames().close) {
    249         if (!Base::getOwnPropertySlot(thisObject, exec, propertyName, slot))
    250             slot.setCustom(thisObject, ReadOnly | DontDelete | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionClose, 0>);
    251         return true;
    252     }
    253     if (propertyName == exec->propertyNames().focus) {
    254         if (!Base::getOwnPropertySlot(thisObject, exec, propertyName, slot))
    255             slot.setCustom(thisObject, ReadOnly | DontDelete | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionFocus, 0>);
    256         return true;
    257     }
    258     if (propertyName == exec->propertyNames().postMessage) {
    259         if (!Base::getOwnPropertySlot(thisObject, exec, propertyName, slot))
    260             slot.setCustom(thisObject, ReadOnly | DontDelete | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionPostMessage, 2>);
    261         return true;
    262     }
    263 
    264240    if (propertyName == exec->propertyNames().showModalDialog) {
    265241        if (Base::getOwnPropertySlot(thisObject, exec, propertyName, slot))
    … …  
    589565
    590566    ExceptionCode ec = 0;
    591     impl.postMessage(message.release(), &messagePorts, targetOrigin, activeDOMWindow(&state), ec);
     567    impl.postMessage(message.release(), &messagePorts, targetOrigin, callerDOMWindow(&state), ec);
    592568    setDOMException(&state, ec);
    593569
Note: See TracChangeset for help on using the changeset viewer.