Changeset 110315 in webkit


Ignore:
Timestamp:
Mar 9, 2012 12:12:35 PM (12 years ago)
Author:
arv@chromium.org
Message:

[V8] Fix object scope for inline event attribute handlers
https://bugs.webkit.org/show_bug.cgi?id=80329

Reviewed by Ojan Vafai.

Source/WebCore:

We now create the funciton inside the with-statements with the current scope objects.
This is important for a few reasons:

  • We need to use the real objects and not just lookup the JS properties because these might have been overridden.
  • We need to use the node, form and document at the time of the preparation and not at the time of calling.
  • We need to ensure that event/evt is bound closer than a property with the same name in the object environment created by the with-statements.

Tests: fast/dom/inline-event-attributes-lookup-removed-form.html

fast/dom/inline-event-attributes-lookup-removed.html
fast/dom/inline-event-attributes-lookup.html

  • bindings/v8/ScriptEventListener.cpp:

(WebCore::eventParameterName):
(WebCore):
(WebCore::createAttributeEventListener):

  • bindings/v8/V8LazyEventListener.cpp:

(WebCore::V8LazyEventListener::V8LazyEventListener):
(WebCore):
(WebCore::toObjectWrapper):
(WebCore::V8LazyEventListener::callListenerFunction):
(WebCore::V8LazyEventListener::prepareListenerObject):

  • bindings/v8/V8LazyEventListener.h:

(WebCore):
(WebCore::V8LazyEventListener::create):
(V8LazyEventListener):

LayoutTests:

  • fast/dom/inline-event-attributes-lookup-expected.txt: Added.
  • fast/dom/inline-event-attributes-lookup-removed-expected.txt: Added.
  • fast/dom/inline-event-attributes-lookup-removed-form-expected.txt: Added.
  • fast/dom/inline-event-attributes-lookup-removed-form.html: Added.
  • fast/dom/inline-event-attributes-lookup-removed.html: Added.
  • fast/dom/inline-event-attributes-lookup.html: Added.
  • fast/forms/lazy-event-listener-scope-chain-expected.txt:
  • fast/forms/lazy-event-listener-scope-chain.html:
Location:
trunk
Files:
6 added
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r110313 r110315  
     12012-03-08  Erik Arvidsson  <arv@chromium.org>
     2
     3        [V8] Fix object scope for inline event attribute handlers
     4        https://bugs.webkit.org/show_bug.cgi?id=80329
     5
     6        Reviewed by Ojan Vafai.
     7
     8        * fast/dom/inline-event-attributes-lookup-expected.txt: Added.
     9        * fast/dom/inline-event-attributes-lookup-removed-expected.txt: Added.
     10        * fast/dom/inline-event-attributes-lookup-removed-form-expected.txt: Added.
     11        * fast/dom/inline-event-attributes-lookup-removed-form.html: Added.
     12        * fast/dom/inline-event-attributes-lookup-removed.html: Added.
     13        * fast/dom/inline-event-attributes-lookup.html: Added.
     14        * fast/forms/lazy-event-listener-scope-chain-expected.txt:
     15        * fast/forms/lazy-event-listener-scope-chain.html:
     16
    1172012-03-09  Ojan Vafai  <ojan@chromium.org>
    218
  • trunk/LayoutTests/fast/forms/lazy-event-listener-scope-chain-expected.txt

    r52502 r110315  
    1 This test tests that a lazy event listener attached to a form element keeps its form in the scope chain when the listener is called by javascript.
     1This test tests that a lazy event listener attached to a form element keeps its form in the scope chain when the listener is called by JavaScript.
     2
     3On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     4
     5
     6PASS result is "abc"
     7PASS result is "abc"
     8PASS result is "abc"
    29 
    3 abc
    4 
  • trunk/LayoutTests/fast/forms/lazy-event-listener-scope-chain.html

    r52502 r110315  
    1 <html>
     1<!DOCTYPE html>
     2<script src="../js/resources/js-test-pre.js"></script>
     3<form action="javascript:void(0)" onsubmit="runTest(username.value)">
     4<input type="text" value="abc" name="username">
     5<input type="submit" name="login" value="Login">
     6</form>
     7<script>
    28
    3 <script>
    4 if (window.layoutTestController)
    5   layoutTestController.dumpAsText();
     9description('This test tests that a lazy event listener attached to a form element keeps its form in the scope chain when the listener is called by JavaScript.');
    610
    7 function run_test(x) {
    8   var r = document.getElementById('result');
    9   r.innerHTML = x + '<br>';
     11var result;
     12
     13function runTest(x)
     14{
     15    result = x;
     16    shouldBeEqualToString('result', 'abc');
    1017}
    1118
    12 function setup() {
    13   var f = document.getElementsByTagName('form')[0];
    14   f.old_f = f.onsubmit;
    15   f.onsubmit = function() {this.old_f();};
    16   f.login.click();
    17 }
     19var f = document.querySelector('form');
     20
     21f.onsubmit();
     22
     23// Should keep the ObjectEnvironment even when called without context.
     24(0, f.onsubmit)();
     25
     26f.oldF = f.onsubmit;
     27
     28f.onsubmit = function() {
     29    this.oldF();
     30};
     31
     32f.login.click();
    1833
    1934</script>
    20 This test tests that a lazy event listener attached to a form element
    21 keeps its form in the scope chain when the listener is called by
    22 javascript. <br>
    23 
    24 <body onload="setup()">
    25 <form action="javascript:void(0)" onsubmit="run_test(username.value)">
    26 <input type="text" value="abc" name="username"/>
    27 <input type="submit" name="login" value="Login"/>
    28 </form>
    29 
    30 <div id="result"></div>
    31 </body>
    32 </html>
     35<script src="../js/resources/js-test-pre.js"></script>
  • trunk/Source/WebCore/ChangeLog

    r110314 r110315  
     12012-03-08  Erik Arvidsson  <arv@chromium.org>
     2
     3        [V8] Fix object scope for inline event attribute handlers
     4        https://bugs.webkit.org/show_bug.cgi?id=80329
     5
     6        Reviewed by Ojan Vafai.
     7
     8        We now create the funciton inside the with-statements with the current scope objects.
     9        This is important for a few reasons:
     10
     11        - We need to use the real objects and not just lookup the JS properties because these might have been overridden.
     12        - We need to use the node, form and document at the time of the preparation and not at the time of calling.
     13        - We need to ensure that event/evt is bound closer than a property with the same name in the object environment
     14          created by the with-statements.
     15
     16        Tests: fast/dom/inline-event-attributes-lookup-removed-form.html
     17               fast/dom/inline-event-attributes-lookup-removed.html
     18               fast/dom/inline-event-attributes-lookup.html
     19
     20        * bindings/v8/ScriptEventListener.cpp:
     21        (WebCore::eventParameterName):
     22        (WebCore):
     23        (WebCore::createAttributeEventListener):
     24        * bindings/v8/V8LazyEventListener.cpp:
     25        (WebCore::V8LazyEventListener::V8LazyEventListener):
     26        (WebCore):
     27        (WebCore::toObjectWrapper):
     28        (WebCore::V8LazyEventListener::callListenerFunction):
     29        (WebCore::V8LazyEventListener::prepareListenerObject):
     30        * bindings/v8/V8LazyEventListener.h:
     31        (WebCore):
     32        (WebCore::V8LazyEventListener::create):
     33        (V8LazyEventListener):
     34
    1352012-03-09  Stephen Chenney  <schenney@chromium.org>
    236
  • trunk/Source/WebCore/bindings/v8/ScriptEventListener.cpp

    r95901 r110315  
    4343namespace WebCore {
    4444
     45static const String& eventParameterName(bool isSVGEvent)
     46{
     47    DEFINE_STATIC_LOCAL(const String, eventString, ("event"));
     48    DEFINE_STATIC_LOCAL(const String, evtString, ("evt"));
     49    return isSVGEvent ? evtString : eventString;
     50}
     51
    4552PassRefPtr<V8LazyEventListener> createAttributeEventListener(Node* node, Attribute* attr)
    4653{
     
    5865        if (!scriptController->canExecuteScripts(AboutToExecuteScript))
    5966            return 0;
    60 
    6167        position = scriptController->eventHandlerPosition();
    6268        sourceURL = node->document()->url().string();
    6369    }
    6470
    65     return V8LazyEventListener::create(attr->localName().string(), node->isSVGElement(), attr->value(), sourceURL, position, WorldContextHandle(UseMainWorld));
     71    return V8LazyEventListener::create(attr->localName().string(), eventParameterName(node->isSVGElement()), attr->value(), sourceURL, position, node, WorldContextHandle(UseMainWorld));
    6672}
    6773
     
    8187    TextPosition position = scriptController->eventHandlerPosition();
    8288    String sourceURL = frame->document()->url().string();
    83     return V8LazyEventListener::create(attr->localName().string(), frame->document()->isSVGDocument(), attr->value(), sourceURL, position, WorldContextHandle(UseMainWorld));
     89
     90    return V8LazyEventListener::create(attr->localName().string(), eventParameterName(frame->document()->isSVGDocument()), attr->value(), sourceURL, position, 0, WorldContextHandle(UseMainWorld));
    8491}
    8592
  • trunk/Source/WebCore/bindings/v8/V8LazyEventListener.cpp

    r109599 r110315  
    3333
    3434#include "ContentSecurityPolicy.h"
     35#include "Document.h"
    3536#include "Frame.h"
     37#include "HTMLElement.h"
     38#include "HTMLFormElement.h"
     39#include "Node.h"
    3640#include "V8Binding.h"
     41#include "V8DOMWrapper.h"
     42#include "V8Document.h"
     43#include "V8HTMLFormElement.h"
    3744#include "V8HiddenPropertyName.h"
     45#include "V8Node.h"
    3846#include "V8Proxy.h"
    3947#include "WorldContextHandle.h"
     
    4351namespace WebCore {
    4452
    45 V8LazyEventListener::V8LazyEventListener(const String& functionName, bool isSVGEvent, const String& code, const String sourceURL, const TextPosition& position, const WorldContextHandle& worldContext)
     53V8LazyEventListener::V8LazyEventListener(const AtomicString& functionName, const AtomicString& eventParameterName, const String& code, const String sourceURL, const TextPosition& position, PassRefPtr<Node> node, const WorldContextHandle& worldContext)
    4654    : V8AbstractEventListener(true, worldContext)
    4755    , m_functionName(functionName)
    48     , m_isSVGEvent(isSVGEvent)
     56    , m_eventParameterName(eventParameterName)
    4957    , m_code(code)
    5058    , m_sourceURL(sourceURL)
     59    , m_node(node)
     60    , m_formElement(0)
    5161    , m_position(position)
    5262{
     63    if (m_node && m_node->isHTMLElement())
     64        m_formElement = static_cast<HTMLElement*>(m_node.get())->form();
     65}
     66
     67template<typename T>
     68v8::Handle<v8::Object> toObjectWrapper(T* domObject)
     69{
     70    if (!domObject)
     71        return v8::Object::New();
     72    v8::Handle<v8::Value> value = toV8(domObject);
     73    if (value.IsEmpty())
     74        return v8::Object::New();
     75    return value.As<v8::Object>();
    5376}
    5477
     
    5982        return v8::Local<v8::Value>();
    6083
    61     v8::Local<v8::Function> handlerFunction = v8::Local<v8::Function>::Cast(listenerObject);
     84    v8::Local<v8::Function> handlerFunction = listenerObject.As<v8::Function>();
    6285    v8::Local<v8::Object> receiver = getReceiverObject(event);
    6386    if (handlerFunction.IsEmpty() || receiver.IsEmpty())
     
    104127
    105128    v8::Context::Scope scope(v8Context);
    106 
    107     // FIXME: cache the wrapper function.
    108129
    109130    // Nodes other than the document object, when executing inline event
     
    116137    // Don't use new lines so that lines in the modified handler
    117138    // have the same numbers as in the original code.
    118     // FIXME: This approach is a giant hack! What if m_code escapes to run
    119     //        arbitrary script?
    120     String eventParameterName = m_isSVGEvent ? "evt" : "event";
    121     String code = "(function(";
    122     code.append(eventParameterName);
    123     code.append(") {" \
    124             "with (this.ownerDocument ? this.ownerDocument : {}) {" \
    125             "with (this.form ? this.form : {}) {" \
    126             "with (this) {" \
    127             "return (function(");
    128     code.append(eventParameterName);
    129     code.append("){");
     139    // FIXME: V8 does not allow us to programmatically create object environments so
     140    //        we have to do this hack! What if m_code escapes to run arbitrary script?
     141    //
     142    String code = "(function() {" \
     143        "with (arguments[2]) {" \
     144        "with (arguments[1]) {" \
     145        "with (arguments[0]) {";
     146    code.append("return function(");
     147    code.append(m_eventParameterName);
     148    code.append(") {");
     149
     150    int codePrexixLength = code.length();
     151
    130152    code.append(m_code);
    131153    // Insert '\n' otherwise //-style comments could break the handler.
    132     code.append("\n}).call(this, ");
    133     code.append(eventParameterName);
    134     code.append(");}}}})");
     154    code.append("\n};}}}})");
    135155    v8::Handle<v8::String> codeExternalString = v8ExternalString(code);
    136     v8::Handle<v8::Script> script = V8Proxy::compileScript(codeExternalString, m_sourceURL, m_position);
    137     if (!script.IsEmpty()) {
    138         // Call v8::Script::Run() directly to avoid an erroneous call to V8RecursionScope::didLeaveScriptContext().
    139         // FIXME: Remove this code when we stop doing the 'with' hack above.
    140         v8::Local<v8::Value> value = script->Run();
    141         if (!value.IsEmpty()) {
    142             ASSERT(value->IsFunction());
    143 
    144             v8::Local<v8::Function> wrappedFunction = v8::Local<v8::Function>::Cast(value);
    145 
    146             // Change the toString function on the wrapper function to avoid it
    147             // returning the source for the actual wrapper function. Instead it
    148             // returns source for a clean wrapper function with the event
    149             // argument wrapping the event source code. The reason for this is
    150             // that some web sites use toString on event functions and eval the
    151             // source returned (sometimes a RegExp is applied as well) for some
    152             // other use. That fails miserably if the actual wrapper source is
    153             // returned.
    154             v8::Persistent<v8::FunctionTemplate>& toStringTemplate =
    155                 V8BindingPerIsolateData::current()->lazyEventListenerToStringTemplate();
    156             if (toStringTemplate.IsEmpty())
    157                 toStringTemplate = v8::Persistent<v8::FunctionTemplate>::New(v8::FunctionTemplate::New(V8LazyEventListenerToString));
    158             v8::Local<v8::Function> toStringFunction;
    159             if (!toStringTemplate.IsEmpty())
    160                 toStringFunction = toStringTemplate->GetFunction();
    161             if (!toStringFunction.IsEmpty()) {
    162                 String toStringResult = "function ";
    163                 toStringResult.append(m_functionName);
    164                 toStringResult.append("(");
    165                 toStringResult.append(eventParameterName);
    166                 toStringResult.append(") {\n  ");
    167                 toStringResult.append(m_code);
    168                 toStringResult.append("\n}");
    169                 wrappedFunction->SetHiddenValue(V8HiddenPropertyName::toStringString(), v8ExternalString(toStringResult));
    170                 wrappedFunction->Set(v8::String::New("toString"), toStringFunction);
    171             }
    172 
    173             wrappedFunction->SetName(v8::String::New(fromWebCoreString(m_functionName), m_functionName.length()));
    174 
    175             setListenerObject(wrappedFunction);
    176         }
     156
     157    TextPosition adjustedPosition(m_position.m_line, OrdinalNumber::fromZeroBasedInt(m_position.m_column.zeroBasedInt() - codePrexixLength));
     158
     159    v8::Handle<v8::Script> script = V8Proxy::compileScript(codeExternalString, m_sourceURL, adjustedPosition);
     160    if (script.IsEmpty())
     161        return;
     162
     163    // Call v8::Script::Run() directly to avoid an erroneous call to V8RecursionScope::didLeaveScriptContext().
     164    // FIXME: Remove this code when we stop doing the 'with' hack above.
     165    v8::Local<v8::Value> value = script->Run();
     166    if (value.IsEmpty())
     167        return;
     168
     169    // Call the outer function to get the inner function.
     170    ASSERT(value->IsFunction());
     171    v8::Local<v8::Function> intermediateFunction = value.As<v8::Function>();
     172
     173    v8::Handle<v8::Object> nodeWrapper = toObjectWrapper<Node>(m_node.get());
     174    v8::Handle<v8::Object> formWrapper = toObjectWrapper<HTMLFormElement>(m_formElement.get());
     175    v8::Handle<v8::Object> documentWrapper = toObjectWrapper<Document>(m_node ? m_node->ownerDocument() : 0);
     176
     177    m_node.clear();
     178    m_formElement.clear();
     179
     180    v8::Handle<v8::Value> parameters[3] = { nodeWrapper, formWrapper, documentWrapper };
     181
     182    // Use Call directly to avoid an erroneous call to V8RecursionScope::didLeaveScriptContext().
     183    // FIXME: Remove this code when we stop doing the 'with' hack above.
     184    v8::Local<v8::Value> innerValue = intermediateFunction->Call(v8Context->Global(), 3, parameters);
     185
     186    ASSERT(innerValue->IsFunction());
     187    v8::Local<v8::Function> wrappedFunction = innerValue.As<v8::Function>();
     188
     189    // Change the toString function on the wrapper function to avoid it
     190    // returning the source for the actual wrapper function. Instead it
     191    // returns source for a clean wrapper function with the event
     192    // argument wrapping the event source code. The reason for this is
     193    // that some web sites use toString on event functions and eval the
     194    // source returned (sometimes a RegExp is applied as well) for some
     195    // other use. That fails miserably if the actual wrapper source is
     196    // returned.
     197    v8::Persistent<v8::FunctionTemplate>& toStringTemplate =
     198        V8BindingPerIsolateData::current()->lazyEventListenerToStringTemplate();
     199    if (toStringTemplate.IsEmpty())
     200        toStringTemplate = v8::Persistent<v8::FunctionTemplate>::New(v8::FunctionTemplate::New(V8LazyEventListenerToString));
     201    v8::Local<v8::Function> toStringFunction;
     202    if (!toStringTemplate.IsEmpty())
     203        toStringFunction = toStringTemplate->GetFunction();
     204    if (!toStringFunction.IsEmpty()) {
     205        String toStringResult = "function ";
     206        toStringResult.append(m_functionName);
     207        toStringResult.append("(");
     208        toStringResult.append(m_eventParameterName);
     209        toStringResult.append(") {\n  ");
     210        toStringResult.append(m_code);
     211        toStringResult.append("\n}");
     212        wrappedFunction->SetHiddenValue(V8HiddenPropertyName::toStringString(), v8ExternalString(toStringResult));
     213        wrappedFunction->Set(v8::String::New("toString"), toStringFunction);
    177214    }
     215
     216    wrappedFunction->SetName(v8::String::New(fromWebCoreString(m_functionName), m_functionName.length()));
     217
     218    setListenerObject(wrappedFunction);
    178219}
    179220
  • trunk/Source/WebCore/bindings/v8/V8LazyEventListener.h

    r95901 r110315  
    4242    class Event;
    4343    class Frame;
     44    class HTMLFormElement;
     45    class Node;
    4446
    4547    // V8LazyEventListener is a wrapper for a JavaScript code string that is compiled and evaluated when an event is fired.
    46     // A V8LazyEventListener is always a HTML event handler.
     48    // A V8LazyEventListener is either a HTML or SVG event handler.
    4749    class V8LazyEventListener : public V8AbstractEventListener {
    4850    public:
    49         static PassRefPtr<V8LazyEventListener> create(const String& functionName, bool isSVGEvent, const String& code, const String& sourceURL, const TextPosition& position, const WorldContextHandle& worldContext)
     51        static PassRefPtr<V8LazyEventListener> create(const AtomicString& functionName, const AtomicString& eventParameterName, const String& code, const String& sourceURL, const TextPosition& position, PassRefPtr<Node> node, const WorldContextHandle& worldContext)
    5052        {
    51             return adoptRef(new V8LazyEventListener(functionName, isSVGEvent, code, sourceURL, position, worldContext));
     53            return adoptRef(new V8LazyEventListener(functionName, eventParameterName, code, sourceURL, position, node, worldContext));
    5254        }
    5355
     
    5860
    5961    private:
    60         V8LazyEventListener(const String& functionName, bool isSVGEvent, const String& code, const String sourceURL, const TextPosition&, const WorldContextHandle&);
     62        V8LazyEventListener(const AtomicString& functionName, const AtomicString& eventParameterName, const String& code, const String sourceURL, const TextPosition&, PassRefPtr<Node>, const WorldContextHandle&);
    6163
    6264        virtual v8::Local<v8::Value> callListenerFunction(ScriptExecutionContext*, v8::Handle<v8::Value> jsEvent, Event*);
     
    6870        virtual bool wasCreatedFromMarkup() const { return true; }
    6971
    70         String m_functionName;
    71         bool m_isSVGEvent;
     72        AtomicString m_functionName;
     73        AtomicString m_eventParameterName;
    7274        String m_code;
    7375        String m_sourceURL;
     76        RefPtr<Node> m_node;
     77        RefPtr<HTMLFormElement> m_formElement;
    7478        TextPosition m_position;
    7579    };
Note: See TracChangeset for help on using the changeset viewer.