Skip to content
Permalink
Browse files
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):


Canonical link: https://commits.webkit.org/95077@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@107170 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
ajklein committed Feb 9, 2012
1 parent 91c76f9 commit 3ce6a33296d2b5c0fb4463c9a400554621392b04
Showing 13 changed files with 93 additions and 20 deletions.
@@ -1,3 +1,53 @@
2012-02-08 Adam Klein <adamk@chromium.org>

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

Reviewed by Dmitry Titov.

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):

2012-02-08 Anders Carlsson <andersca@apple.com>

Don't use the wheel event handler count to track if a page has horizontal scrollbars
@@ -197,7 +197,7 @@ ScriptValue ScriptCallback::call(bool& hadException)
for (size_t i = 0; i < m_arguments.size(); ++i)
args[i] = m_arguments[i].v8Value();

v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(0 /* page */, function, object, m_arguments.size(), args.get());
v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(0 /* frame */, function, object, m_arguments.size(), args.get());

if (exceptionCatcher.HasCaught()) {
hadException = true;
@@ -83,7 +83,7 @@ short V8NodeFilterCondition::acceptNode(ScriptState* state, Node* node) const
OwnArrayPtr<v8::Handle<v8::Value> > args = adoptArrayPtr(new v8::Handle<v8::Value>[1]);
args[0] = toV8(node);

v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(0 /* page */, callback, object, 1, args.get());
v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(0 /* frame */, callback, object, 1, args.get());

if (exceptionCatcher.HasCaught()) {
state->setException(exceptionCatcher.Exception());
@@ -378,7 +378,7 @@ v8::Local<v8::Value> V8Proxy::runScript(v8::Handle<v8::Script> script)
v8::TryCatch tryCatch;
tryCatch.SetVerbose(true);
{
V8RecursionScope recursionScope;
V8RecursionScope recursionScope(frame()->document());
result = script->Run();
}

@@ -404,31 +404,31 @@ v8::Local<v8::Value> V8Proxy::callFunction(v8::Handle<v8::Function> function, v8
{
// Keep Frame (and therefore ScriptController and V8Proxy) alive.
RefPtr<Frame> protect(frame());
return V8Proxy::instrumentedCallFunction(m_frame->page(), function, receiver, argc, args);
return V8Proxy::instrumentedCallFunction(frame(), function, receiver, argc, args);
}

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[])
v8::Local<v8::Value> V8Proxy::instrumentedCallFunction(Frame* frame, v8::Handle<v8::Function> function, v8::Handle<v8::Object> receiver, int argc, v8::Handle<v8::Value> args[])
{
V8GCController::checkMemoryUsage();

if (V8RecursionScope::recursionLevel() >= kMaxRecursionDepth)
return handleMaxRecursionDepthExceeded();

InspectorInstrumentationCookie cookie;
if (InspectorInstrumentation::hasFrontends()) {
if (InspectorInstrumentation::hasFrontends() && frame) {
String resourceName("undefined");
int lineNumber = 1;
v8::ScriptOrigin origin = function->GetScriptOrigin();
if (!origin.ResourceName().IsEmpty()) {
resourceName = toWebCoreString(origin.ResourceName());
lineNumber = function->GetScriptLineNumber() + 1;
}
cookie = InspectorInstrumentation::willCallFunction(page, resourceName, lineNumber);
cookie = InspectorInstrumentation::willCallFunction(frame->page(), resourceName, lineNumber);
}

v8::Local<v8::Value> result;
{
V8RecursionScope recursionScope;
V8RecursionScope recursionScope(frame ? frame->document() : 0);
result = function->Call(receiver, argc, args);
}

@@ -57,7 +57,6 @@ namespace WebCore {
class DOMWindow;
class Frame;
class Node;
class Page;
class ScriptExecutionContext;
class ScriptSourceCode;
class SecurityOrigin;
@@ -162,7 +161,7 @@ namespace WebCore {
v8::Local<v8::Value> callFunction(v8::Handle<v8::Function>, v8::Handle<v8::Object>, int argc, v8::Handle<v8::Value> argv[]);

// call the function with the given receiver and arguments and report times to DevTools.
static v8::Local<v8::Value> instrumentedCallFunction(Page*, v8::Handle<v8::Function>, v8::Handle<v8::Object> receiver, int argc, v8::Handle<v8::Value> args[]);
static v8::Local<v8::Value> instrumentedCallFunction(Frame*, v8::Handle<v8::Function>, v8::Handle<v8::Object> receiver, int argc, v8::Handle<v8::Value> args[]);

// Call the function as constructor with the given arguments.
v8::Local<v8::Value> newInstance(v8::Handle<v8::Function>, int argc, v8::Handle<v8::Value> argv[]);
@@ -32,11 +32,12 @@
#include "V8RecursionScope.h"

#include "IDBPendingTransactionMonitor.h"
#include "ScriptExecutionContext.h"
#include "WebKitMutationObserver.h"

namespace WebCore {

void V8RecursionScope::didLeaveScriptContext()
void V8RecursionScope::didLeaveScriptContext(ScriptExecutionContext* context)
{
// FIXME: Instrument any work that takes place when script exits to c++ (e.g. Mutation Observers).

@@ -48,7 +49,8 @@ void V8RecursionScope::didLeaveScriptContext()
#endif

#if ENABLE(MUTATION_OBSERVERS)
WebKitMutationObserver::deliverAllMutations();
if (context && context->isDocument())
WebKitMutationObserver::deliverAllMutations();
#endif
}

@@ -35,20 +35,29 @@

namespace WebCore {

class ScriptExecutionContext;

class V8RecursionScope {
WTF_MAKE_NONCOPYABLE(V8RecursionScope);
public:
V8RecursionScope() { V8BindingPerIsolateData::current()->incrementRecursionLevel(); }
explicit V8RecursionScope(ScriptExecutionContext* context)
: m_context(context)
{
V8BindingPerIsolateData::current()->incrementRecursionLevel();
}

~V8RecursionScope()
{
if (!V8BindingPerIsolateData::current()->decrementRecursionLevel())
didLeaveScriptContext();
didLeaveScriptContext(m_context);
}

static int recursionLevel() { return V8BindingPerIsolateData::current()->recursionLevel(); }

private:
static void didLeaveScriptContext();
static void didLeaveScriptContext(ScriptExecutionContext*);

ScriptExecutionContext* m_context;
};

} // namespace WebCore
@@ -58,7 +58,7 @@ v8::Local<v8::Value> V8WindowErrorHandler::callListenerFunction(ScriptExecutionC
v8::Handle<v8::Value> parameters[3] = { v8String(errorEvent->message()), v8String(errorEvent->filename()), v8::Integer::New(errorEvent->lineno()) };
v8::TryCatch tryCatch;
tryCatch.SetVerbose(true);
returnValue = V8Proxy::instrumentedCallFunction(0 /* page */, callFunction, thisValue, 3, parameters);
returnValue = V8Proxy::instrumentedCallFunction(0 /* frame */, callFunction, thisValue, 3, parameters);
}
return returnValue;
}
@@ -83,8 +83,8 @@ bool invokeCallback(v8::Persistent<v8::Object> callback, int argc, v8::Handle<v8

v8::Handle<v8::Object> thisObject = v8::Context::GetCurrent()->Global();

Page* page = scriptExecutionContext && scriptExecutionContext->isDocument() ? static_cast<Document*>(scriptExecutionContext)->page() : 0;
v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(page, callbackFunction, thisObject, argc, argv);
Frame* frame = scriptExecutionContext && scriptExecutionContext->isDocument() ? static_cast<Document*>(scriptExecutionContext)->frame() : 0;
v8::Handle<v8::Value> result = V8Proxy::instrumentedCallFunction(frame, callbackFunction, thisObject, argc, argv);

callbackReturnValue = !result.IsEmpty() && result->BooleanValue();
return exceptionCatcher.HasCaught();
@@ -79,7 +79,7 @@ String V8CustomXPathNSResolver::lookupNamespaceURI(const String& prefix)
v8::Handle<v8::Value> argv[argc] = { v8String(prefix) };
v8::Handle<v8::Function> function = lookupNamespaceURIFunc.IsEmpty() ? v8::Handle<v8::Function>::Cast(m_resolver) : lookupNamespaceURIFunc;

v8::Handle<v8::Value> retval = V8Proxy::instrumentedCallFunction(0 /* page */, function, m_resolver, argc, argv);
v8::Handle<v8::Value> retval = V8Proxy::instrumentedCallFunction(0 /* frame */, function, m_resolver, argc, argv);

// Eat exceptions from namespace resolver and return an empty string. This will most likely cause NAMESPACE_ERR.
if (try_catch.HasCaught())
@@ -41,6 +41,7 @@
#include "MutationRecord.h"
#include "Node.h"
#include <wtf/ListHashSet.h>
#include <wtf/MainThread.h>

namespace WebCore {

@@ -115,6 +116,7 @@ static MutationObserverSet& activeMutationObservers()

void WebKitMutationObserver::enqueueMutationRecord(PassRefPtr<MutationRecord> mutation)
{
ASSERT(isMainThread());
m_records.append(mutation);
activeMutationObservers().add(this);
}
@@ -132,6 +134,7 @@ void WebKitMutationObserver::deliver()

void WebKitMutationObserver::deliverAllMutations()
{
ASSERT(isMainThread());
static bool deliveryInProgress = false;
if (deliveryInProgress)
return;
@@ -1,3 +1,13 @@
2012-02-08 Adam Klein <adamk@chromium.org>

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

Reviewed by Dmitry Titov.

* src/WebDevToolsFrontendImpl.cpp:
(WebKit::WebDevToolsFrontendImpl::dispatchOnInspectorFrontend):

2012-02-08 Scott Graham <scottmg@chromium.org>

Roll Chromium DEPS
@@ -123,7 +123,7 @@ void WebDevToolsFrontendImpl::dispatchOnInspectorFrontend(const WebString& messa
args.append(ToV8String(message));
v8::TryCatch tryCatch;
tryCatch.SetVerbose(true);
V8Proxy::instrumentedCallFunction(frame->frame()->page(), function, inspectorBackend, args.size(), args.data());
V8Proxy::instrumentedCallFunction(frame->frame(), function, inspectorBackend, args.size(), args.data());
}

} // namespace WebKit

0 comments on commit 3ce6a33

Please sign in to comment.