Changeset 107170 in webkit


Ignore:
Timestamp:
Feb 8, 2012 7:34:29 PM (12 years ago)
Author:
adamk@chromium.org
Message:

DOM mutations should not be delivered on worker threads
https://bugs.webkit.org/show_bug.cgi?id=77898

Reviewed by Dmitry Titov.

Source/WebCore:

In V8RecursionScope, only call WebKitMutationObserver::deliverAllMutations
if in a Document context.

This is accomplished through a change to V8Proxy::instrumentedCallFunction
(which now takes a Frame* instead of a Page*), requiring an update to all
callers of that function (accounting for the majority of files changed
in this patch).

Added ASSERT(isMainThread()) in a deliverAllMutations to confirm that
it's no longer called on worker threads, and in enqueueMutationRecord,
where the same global store of active observers is accessed.

See also http://crbug.com/112586, where the problem was initially
reported.

  • bindings/v8/ScriptFunctionCall.cpp:

(WebCore::ScriptCallback::call):

  • bindings/v8/V8NodeFilterCondition.cpp:

(WebCore::V8NodeFilterCondition::acceptNode):

  • bindings/v8/V8Proxy.cpp:

(WebCore::V8Proxy::runScript):
(WebCore::V8Proxy::callFunction):
(WebCore::V8Proxy::instrumentedCallFunction):

  • bindings/v8/V8Proxy.h:

(WebCore):
(V8Proxy):

  • bindings/v8/V8RecursionScope.cpp:

(WebCore::V8RecursionScope::didLeaveScriptContext):

  • bindings/v8/V8RecursionScope.h:

(WebCore):
(WebCore::V8RecursionScope::V8RecursionScope):
(V8RecursionScope):
(WebCore::V8RecursionScope::~V8RecursionScope):

  • bindings/v8/V8WindowErrorHandler.cpp:

(WebCore::V8WindowErrorHandler::callListenerFunction):

  • bindings/v8/custom/V8CustomVoidCallback.cpp:

(WebCore::invokeCallback):

  • bindings/v8/custom/V8CustomXPathNSResolver.cpp:

(WebCore::V8CustomXPathNSResolver::lookupNamespaceURI):

  • dom/WebKitMutationObserver.cpp:

(WebCore::WebKitMutationObserver::enqueueMutationRecord):
(WebCore::WebKitMutationObserver::deliverAllMutations):

Source/WebKit/chromium:

  • src/WebDevToolsFrontendImpl.cpp:

(WebKit::WebDevToolsFrontendImpl::dispatchOnInspectorFrontend):

Location:
trunk/Source
Files:
13 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r107168 r107170  
     12012-02-08  Adam Klein  <adamk@chromium.org>
     2
     3        DOM mutations should not be delivered on worker threads
     4        https://bugs.webkit.org/show_bug.cgi?id=77898
     5
     6        Reviewed by Dmitry Titov.
     7
     8        In V8RecursionScope, only call WebKitMutationObserver::deliverAllMutations
     9        if in a Document context.
     10
     11        This is accomplished through a change to V8Proxy::instrumentedCallFunction
     12        (which now takes a Frame* instead of a Page*), requiring an update to all
     13        callers of that function (accounting for the majority of files changed
     14        in this patch).
     15
     16        Added ASSERT(isMainThread()) in a deliverAllMutations to confirm that
     17        it's no longer called on worker threads, and in enqueueMutationRecord,
     18        where the same global store of active observers is accessed.
     19
     20        See also http://crbug.com/112586, where the problem was initially
     21        reported.
     22
     23        * bindings/v8/ScriptFunctionCall.cpp:
     24        (WebCore::ScriptCallback::call):
     25        * bindings/v8/V8NodeFilterCondition.cpp:
     26        (WebCore::V8NodeFilterCondition::acceptNode):
     27        * bindings/v8/V8Proxy.cpp:
     28        (WebCore::V8Proxy::runScript):
     29        (WebCore::V8Proxy::callFunction):
     30        (WebCore::V8Proxy::instrumentedCallFunction):
     31        * bindings/v8/V8Proxy.h:
     32        (WebCore):
     33        (V8Proxy):
     34        * bindings/v8/V8RecursionScope.cpp:
     35        (WebCore::V8RecursionScope::didLeaveScriptContext):
     36        * bindings/v8/V8RecursionScope.h:
     37        (WebCore):
     38        (WebCore::V8RecursionScope::V8RecursionScope):
     39        (V8RecursionScope):
     40        (WebCore::V8RecursionScope::~V8RecursionScope):
     41        * bindings/v8/V8WindowErrorHandler.cpp:
     42        (WebCore::V8WindowErrorHandler::callListenerFunction):
     43        * bindings/v8/custom/V8CustomVoidCallback.cpp:
     44        (WebCore::invokeCallback):
     45        * bindings/v8/custom/V8CustomXPathNSResolver.cpp:
     46        (WebCore::V8CustomXPathNSResolver::lookupNamespaceURI):
     47        * dom/WebKitMutationObserver.cpp:
     48        (WebCore::WebKitMutationObserver::enqueueMutationRecord):
     49        (WebCore::WebKitMutationObserver::deliverAllMutations):
     50
    1512012-02-08  Anders Carlsson  <andersca@apple.com>
    252
  • trunk/Source/WebCore/bindings/v8/ScriptFunctionCall.cpp

    r101490 r107170  
    198198        args[i] = m_arguments[i].v8Value();
    199199
    200     v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(0 /* page */, function, object, m_arguments.size(), args.get());
     200    v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(0 /* frame */, function, object, m_arguments.size(), args.get());
    201201
    202202    if (exceptionCatcher.HasCaught()) {
  • trunk/Source/WebCore/bindings/v8/V8NodeFilterCondition.cpp

    r101490 r107170  
    8484    args[0] = toV8(node);
    8585
    86     v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(0 /* page */, callback, object, 1, args.get());
     86    v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(0 /* frame */, callback, object, 1, args.get());
    8787
    8888    if (exceptionCatcher.HasCaught()) {
  • trunk/Source/WebCore/bindings/v8/V8Proxy.cpp

    r106698 r107170  
    379379    tryCatch.SetVerbose(true);
    380380    {
    381         V8RecursionScope recursionScope;
     381        V8RecursionScope recursionScope(frame()->document());
    382382        result = script->Run();
    383383    }
     
    405405    // Keep Frame (and therefore ScriptController and V8Proxy) alive.
    406406    RefPtr<Frame> protect(frame());
    407     return V8Proxy::instrumentedCallFunction(m_frame->page(), function, receiver, argc, args);
    408 }
    409 
    410 v8::Local<v8::Value> V8Proxy::instrumentedCallFunction(Page* page, v8::Handle<v8::Function> function, v8::Handle<v8::Object> receiver, int argc, v8::Handle<v8::Value> args[])
     407    return V8Proxy::instrumentedCallFunction(frame(), function, receiver, argc, args);
     408}
     409
     410v8::Local<v8::Value> V8Proxy::instrumentedCallFunction(Frame* frame, v8::Handle<v8::Function> function, v8::Handle<v8::Object> receiver, int argc, v8::Handle<v8::Value> args[])
    411411{
    412412    V8GCController::checkMemoryUsage();
     
    416416
    417417    InspectorInstrumentationCookie cookie;
    418     if (InspectorInstrumentation::hasFrontends()) {
     418    if (InspectorInstrumentation::hasFrontends() && frame) {
    419419        String resourceName("undefined");
    420420        int lineNumber = 1;
     
    424424            lineNumber = function->GetScriptLineNumber() + 1;
    425425        }
    426         cookie = InspectorInstrumentation::willCallFunction(page, resourceName, lineNumber);
     426        cookie = InspectorInstrumentation::willCallFunction(frame->page(), resourceName, lineNumber);
    427427    }
    428428
    429429    v8::Local<v8::Value> result;
    430430    {
    431         V8RecursionScope recursionScope;
     431        V8RecursionScope recursionScope(frame ? frame->document() : 0);
    432432        result = function->Call(receiver, argc, args);
    433433    }
  • trunk/Source/WebCore/bindings/v8/V8Proxy.h

    r105815 r107170  
    5858    class Frame;
    5959    class Node;
    60     class Page;
    6160    class ScriptExecutionContext;
    6261    class ScriptSourceCode;
     
    163162
    164163        // call the function with the given receiver and arguments and report times to DevTools.
    165         static v8::Local<v8::Value> instrumentedCallFunction(Page*, v8::Handle<v8::Function>, v8::Handle<v8::Object> receiver, int argc, v8::Handle<v8::Value> args[]);
     164        static v8::Local<v8::Value> instrumentedCallFunction(Frame*, v8::Handle<v8::Function>, v8::Handle<v8::Object> receiver, int argc, v8::Handle<v8::Value> args[]);
    166165
    167166        // Call the function as constructor with the given arguments.
  • trunk/Source/WebCore/bindings/v8/V8RecursionScope.cpp

    r102067 r107170  
    3333
    3434#include "IDBPendingTransactionMonitor.h"
     35#include "ScriptExecutionContext.h"
    3536#include "WebKitMutationObserver.h"
    3637
    3738namespace WebCore {
    3839
    39 void V8RecursionScope::didLeaveScriptContext()
     40void V8RecursionScope::didLeaveScriptContext(ScriptExecutionContext* context)
    4041{
    4142    // FIXME: Instrument any work that takes place when script exits to c++ (e.g. Mutation Observers).
     
    4950
    5051#if ENABLE(MUTATION_OBSERVERS)
    51     WebKitMutationObserver::deliverAllMutations();
     52    if (context && context->isDocument())
     53        WebKitMutationObserver::deliverAllMutations();
    5254#endif
    5355}
  • trunk/Source/WebCore/bindings/v8/V8RecursionScope.h

    r102067 r107170  
    3636namespace WebCore {
    3737
     38class ScriptExecutionContext;
     39
    3840class V8RecursionScope {
    3941    WTF_MAKE_NONCOPYABLE(V8RecursionScope);
    4042public:
    41     V8RecursionScope() { V8BindingPerIsolateData::current()->incrementRecursionLevel(); }
     43    explicit V8RecursionScope(ScriptExecutionContext* context)
     44        : m_context(context)
     45    {
     46        V8BindingPerIsolateData::current()->incrementRecursionLevel();
     47    }
     48
    4249    ~V8RecursionScope()
    4350    {
    4451        if (!V8BindingPerIsolateData::current()->decrementRecursionLevel())
    45             didLeaveScriptContext();
     52            didLeaveScriptContext(m_context);
    4653    }
    4754
     
    4956
    5057private:
    51     static void didLeaveScriptContext();
     58    static void didLeaveScriptContext(ScriptExecutionContext*);
     59
     60    ScriptExecutionContext* m_context;
    5261};
    5362
  • trunk/Source/WebCore/bindings/v8/V8WindowErrorHandler.cpp

    r101716 r107170  
    5959        v8::TryCatch tryCatch;
    6060        tryCatch.SetVerbose(true);
    61         returnValue = V8Proxy::instrumentedCallFunction(0 /* page */, callFunction, thisValue, 3, parameters);
     61        returnValue = V8Proxy::instrumentedCallFunction(0 /* frame */, callFunction, thisValue, 3, parameters);
    6262    }
    6363    return returnValue;
  • trunk/Source/WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp

    r95901 r107170  
    8484    v8::Handle<v8::Object> thisObject = v8::Context::GetCurrent()->Global();
    8585
    86     Page* page = scriptExecutionContext && scriptExecutionContext->isDocument() ? static_cast<Document*>(scriptExecutionContext)->page() : 0;
    87     v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(page, callbackFunction, thisObject, argc, argv);
     86    Frame* frame = scriptExecutionContext && scriptExecutionContext->isDocument() ? static_cast<Document*>(scriptExecutionContext)->frame() : 0;
     87    v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(frame, callbackFunction, thisObject, argc, argv);
    8888
    8989    callbackReturnValue = !result.IsEmpty() && result->BooleanValue();
  • trunk/Source/WebCore/bindings/v8/custom/V8CustomXPathNSResolver.cpp

    r104803 r107170  
    8080    v8::Handle<v8::Function> function = lookupNamespaceURIFunc.IsEmpty() ? v8::Handle<v8::Function>::Cast(m_resolver) : lookupNamespaceURIFunc;
    8181
    82     v8::Handle<v8::Value> retval = V8Proxy::instrumentedCallFunction(0 /* page */, function, m_resolver, argc, argv);
     82    v8::Handle<v8::Value> retval = V8Proxy::instrumentedCallFunction(0 /* frame */, function, m_resolver, argc, argv);
    8383
    8484    // Eat exceptions from namespace resolver and return an empty string. This will most likely cause NAMESPACE_ERR.
  • trunk/Source/WebCore/dom/WebKitMutationObserver.cpp

    r103001 r107170  
    4242#include "Node.h"
    4343#include <wtf/ListHashSet.h>
     44#include <wtf/MainThread.h>
    4445
    4546namespace WebCore {
     
    116117void WebKitMutationObserver::enqueueMutationRecord(PassRefPtr<MutationRecord> mutation)
    117118{
     119    ASSERT(isMainThread());
    118120    m_records.append(mutation);
    119121    activeMutationObservers().add(this);
     
    133135void WebKitMutationObserver::deliverAllMutations()
    134136{
     137    ASSERT(isMainThread());
    135138    static bool deliveryInProgress = false;
    136139    if (deliveryInProgress)
  • trunk/Source/WebKit/chromium/ChangeLog

    r107166 r107170  
     12012-02-08  Adam Klein  <adamk@chromium.org>
     2
     3        DOM mutations should not be delivered on worker threads
     4        https://bugs.webkit.org/show_bug.cgi?id=77898
     5
     6        Reviewed by Dmitry Titov.
     7
     8        * src/WebDevToolsFrontendImpl.cpp:
     9        (WebKit::WebDevToolsFrontendImpl::dispatchOnInspectorFrontend):
     10
    1112012-02-08  Scott Graham  <scottmg@chromium.org>
    212
  • trunk/Source/WebKit/chromium/src/WebDevToolsFrontendImpl.cpp

    r101503 r107170  
    124124    v8::TryCatch tryCatch;
    125125    tryCatch.SetVerbose(true);
    126     V8Proxy::instrumentedCallFunction(frame->frame()->page(), function, inspectorBackend, args.size(), args.data());
     126    V8Proxy::instrumentedCallFunction(frame->frame(), function, inspectorBackend, args.size(), args.data());
    127127}
    128128
Note: See TracChangeset for help on using the changeset viewer.