Diff
Modified: trunk/Source/WebCore/ChangeLog (110130 => 110131)
--- trunk/Source/WebCore/ChangeLog 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/ChangeLog 2012-03-08 03:07:44 UTC (rev 110131)
@@ -1,3 +1,52 @@
+2012-03-07 Michael Nordman <micha...@google.com>
+
+ [Chromium] Don't be so CRASH() happy in the bindings layer.
+ https://bugs.webkit.org/show_bug.cgi?id=75111
+ - change the v8 bindings generated code to check for the 'worker is terminating'
+ condition prior to committing a suicidal CRASH()
+ - fixup custom v8 bindings accordingly
+ - simplify bindings/generic/ActiveDOMCallback, there is no need for it to support
+ destruction on a different thread
+
+ Reviewed by David Levin.
+
+ No new tests, existing tests apply.
+
+ * bindings/generic/ActiveDOMCallback.cpp: Simplified in general.
+ (WebCore):
+ (WebCore::ActiveDOMCallback::ActiveDOMCallback):
+ (WebCore::ActiveDOMCallback::~ActiveDOMCallback):
+ (WebCore::ActiveDOMCallback::canInvokeCallback):
+ (WebCore::ActiveDOMCallback::isScriptControllerTerminating): New method to avoid CRASH()ing in exceptional conditions in v8 bindings.
+ * bindings/generic/ActiveDOMCallback.h: Derive from ContextDestructionObserver.
+ * bindings/js/WorkerScriptController.cpp:
+ (WebCore::WorkerScriptController::scheduleExecutionTermination): Use a mutex to provide a memory barrier.
+ (WebCore::WorkerScriptController::isExecutionTerminating): New supporting method to avoid CRASH()ing in exceptional conditions.
+ * bindings/js/WorkerScriptController.h:
+ (WorkerScriptController):
+ * bindings/scripts/CodeGeneratorV8.pm: Generates v8 bindding code that uses isScriptControllerTerminating to avoid CRASH()ing.
+ (GenerateCallbackImplementation):
+ * bindings/scripts/test/V8/V8TestCallback.cpp: Fixup expected outputs of the modified CodeGeneratorV8.pm script.
+ (WebCore::V8TestCallback::callbackWithClass1Param):
+ (WebCore::V8TestCallback::callbackWithClass2Param):
+ (WebCore::V8TestCallback::callbackWithStringList):
+ * bindings/v8/WorkerScriptController.cpp:
+ (WebCore::WorkerScriptController::WorkerScriptController): Initialize a new data member.
+ (WebCore::WorkerScriptController::scheduleExecutionTermination): Use a mutex to provide a memory barrier.
+ (WebCore::WorkerScriptController::isExecutionTerminating): New supporting method to avoid CRASH()ing in exceptional conditions.
+ * bindings/v8/WorkerScriptController.h: Add a pair of new data members, bool + mutex.
+ (WorkerScriptController):
+ * bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp:
+ (WebCore::V8SQLStatementErrorCallback::handleEvent):
+ * bindings/v8/custom/V8MutationCallbackCustom.cpp:
+ (WebCore::V8MutationCallback::handleEvent):
+ * dom/ScriptExecutionContext.cpp:
+ (WebCore::ScriptExecutionContext::ScriptExecutionContext): Initilaize data members.
+ (WebCore::ScriptExecutionContext::stopActiveDOMObjects): Set m_activeDOMObjectsAreStopped.
+ * dom/ScriptExecutionContext.h: Add m_activeDOMObjectsAreStopped data member.
+ (WebCore::ScriptExecutionContext::activeDOMObjectsAreStopped): Simple getter.
+ (ScriptExecutionContext):
+
2012-03-07 Kent Tamura <tk...@chromium.org>
Remove meaningless code in RenderTextControlSingleLine::preferredContentWidth()
Modified: trunk/Source/WebCore/bindings/generic/ActiveDOMCallback.cpp (110130 => 110131)
--- trunk/Source/WebCore/bindings/generic/ActiveDOMCallback.cpp 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/bindings/generic/ActiveDOMCallback.cpp 2012-03-08 03:07:44 UTC (rev 110131)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010 Google Inc. All rights reserved.
+ * Copyright (C) 2010. 2012 Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -33,111 +33,34 @@
#include "ActiveDOMObject.h"
#include "ScriptExecutionContext.h"
-#include <wtf/PassOwnPtr.h>
-#include <wtf/ThreadingPrimitives.h>
+#include "WorkerContext.h"
namespace WebCore {
-static void destroyOnContextThread(PassOwnPtr<ActiveDOMObjectCallbackImpl>);
-
-class DestroyOnContextThreadTask : public ScriptExecutionContext::Task {
-public:
- static PassOwnPtr<DestroyOnContextThreadTask> create(PassOwnPtr<ActiveDOMObjectCallbackImpl> impl)
- {
- return adoptPtr(new DestroyOnContextThreadTask(impl));
- }
-
- virtual void performTask(ScriptExecutionContext*)
- {
- destroyOnContextThread(m_impl.release());
- }
-
-private:
- DestroyOnContextThreadTask(PassOwnPtr<ActiveDOMObjectCallbackImpl> impl)
- : m_impl(impl)
- {
- }
-
- OwnPtr<ActiveDOMObjectCallbackImpl> m_impl;
-};
-
-class ActiveDOMObjectCallbackImpl : public ActiveDOMObject {
-public:
- ActiveDOMObjectCallbackImpl(ScriptExecutionContext* context)
- : ActiveDOMObject(context, this)
- , m_suspended(false)
- , m_stopped(false)
- {
- }
-
- virtual void contextDestroyed()
- {
- MutexLocker locker(m_mutex);
- ActiveDOMObject::contextDestroyed();
- }
- virtual bool canSuspend() const { return false; }
- virtual void suspend(ReasonForSuspension)
- {
- MutexLocker locker(m_mutex);
- m_suspended = true;
- }
- virtual void resume()
- {
- MutexLocker locker(m_mutex);
- m_suspended = false;
- }
- virtual void stop()
- {
- MutexLocker locker(m_mutex);
- m_stopped = true;
- }
- bool canInvokeCallback()
- {
- MutexLocker locker(m_mutex);
- return (!m_suspended && !m_stopped);
- }
- ScriptExecutionContext* scriptExecutionContext()
- {
- MutexLocker locker(m_mutex);
- return ActiveDOMObject::scriptExecutionContext();
- }
- Mutex& mutex() { return m_mutex; }
-
-private:
- Mutex m_mutex;
- bool m_suspended;
- bool m_stopped;
-};
-
-static void destroyOnContextThread(PassOwnPtr<ActiveDOMObjectCallbackImpl> impl)
-{
- OwnPtr<ActiveDOMObjectCallbackImpl> implOwnPtr = impl;
-
- ScriptExecutionContext* context = implOwnPtr->scriptExecutionContext();
- MutexLocker locker(implOwnPtr->mutex());
- if (context && !context->isContextThread())
- context->postTask(DestroyOnContextThreadTask::create(implOwnPtr.release()));
-}
-
ActiveDOMCallback::ActiveDOMCallback(ScriptExecutionContext* context)
- : m_impl(adoptPtr(new ActiveDOMObjectCallbackImpl(context)))
+ : ContextDestructionObserver(context)
{
- m_impl->suspendIfNeeded();
}
ActiveDOMCallback::~ActiveDOMCallback()
{
- destroyOnContextThread(m_impl.release());
}
bool ActiveDOMCallback::canInvokeCallback() const
{
- return m_impl->canInvokeCallback();
+ ScriptExecutionContext* context = scriptExecutionContext();
+ return context && !context->activeDOMObjectsAreSuspended() && !context->activeDOMObjectsAreStopped();
}
-ScriptExecutionContext* ActiveDOMCallback::scriptExecutionContext() const
+bool ActiveDOMCallback::isScriptControllerTerminating() const
{
- return m_impl->scriptExecutionContext();
+ ScriptExecutionContext* context = scriptExecutionContext();
+ if (context && context->isWorkerContext()) {
+ WorkerScriptController* scriptController = static_cast<WorkerContext*>(context)->script();
+ if (!scriptController || scriptController->isExecutionForbidden() || scriptController->isExecutionTerminating())
+ return true;
+ }
+ return false;
}
} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/generic/ActiveDOMCallback.h (110130 => 110131)
--- trunk/Source/WebCore/bindings/generic/ActiveDOMCallback.h 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/bindings/generic/ActiveDOMCallback.h 2012-03-08 03:07:44 UTC (rev 110131)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010 Google Inc. All rights reserved.
+ * Copyright (C) 2010, 2012 Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -31,26 +31,27 @@
#ifndef ActiveDOMCallback_h
#define ActiveDOMCallback_h
+#include "ContextDestructionObserver.h"
#include <wtf/OwnPtr.h>
namespace WebCore {
-class ActiveDOMObjectCallbackImpl;
class ScriptExecutionContext;
-// A class that allows callbacks to behave like ActiveDOMObjects, and also
-// be destroyed on the context thread or any other thread.
-class ActiveDOMCallback {
+// A base class that prevents binding callbacks from executing when
+// active dom objects are stopped or suspended, and is used by the
+// generated callback v8 bindings code to avoid erroneously CRASH()'ing
+// after script execution on a worker has been scheduled to terminate.
+//
+// Should only be created, used, and destroyed on the script execution
+// context thread.
+class ActiveDOMCallback : public ContextDestructionObserver {
public:
ActiveDOMCallback(ScriptExecutionContext* context);
- ~ActiveDOMCallback();
+ virtual ~ActiveDOMCallback();
bool canInvokeCallback() const;
- ScriptExecutionContext* scriptExecutionContext() const;
-
-private:
- // The ActiveDOMObject part of the callback.
- OwnPtr<ActiveDOMObjectCallbackImpl> m_impl;
+ bool isScriptControllerTerminating() const;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp (110130 => 110131)
--- trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp 2012-03-08 03:07:44 UTC (rev 110131)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2008 Apple Inc. All Rights Reserved.
- * Copyright (C) 2011 Google Inc. All Rights Reserved.
+ * Copyright (C) 2011, 2012 Google Inc. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -162,9 +162,20 @@
void WorkerScriptController::scheduleExecutionTermination()
{
+ // The mutex provides a memory barrier to ensure that once
+ // termination is scheduled, isExecutionTerminating will
+ // accurately reflect that state when called from another thread.
+ MutexLocker locker(m_scheduledTerminationMutex);
m_globalData->terminator.terminateSoon();
}
+bool WorkerScriptController::isExecutionTerminating() const
+{
+ // See comments in scheduleExecutionTermination regarding mutex usage.
+ MutexLocker locker(m_scheduledTerminationMutex);
+ return m_globalData->terminator.shouldTerminate();
+}
+
void WorkerScriptController::forbidExecution()
{
ASSERT(m_workerContext->isContextThread());
Modified: trunk/Source/WebCore/bindings/js/WorkerScriptController.h (110130 => 110131)
--- trunk/Source/WebCore/bindings/js/WorkerScriptController.h 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/bindings/js/WorkerScriptController.h 2012-03-08 03:07:44 UTC (rev 110131)
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2008 Apple Inc. All Rights Reserved.
+ * Copyright (C) 2012 Google Inc. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -67,6 +68,7 @@
// forbidExecution()/isExecutionForbidden() to guard against reentry into JS.
// Can be called from any thread.
void scheduleExecutionTermination();
+ bool isExecutionTerminating() const;
// Called on Worker thread when JS exits with termination exception caused by forbidExecution() request,
// or by Worker thread termination code to prevent future entry into JS.
@@ -89,6 +91,7 @@
WorkerContext* m_workerContext;
JSC::Strong<JSWorkerContext> m_workerContextWrapper;
bool m_executionForbidden;
+ mutable Mutex m_scheduledTerminationMutex;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm (110130 => 110131)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-03-08 03:07:44 UTC (rev 110131)
@@ -3,7 +3,7 @@
# Copyright (C) 2006 Samuel Weinig <sam.wei...@gmail.com>
# Copyright (C) 2006 Alexey Proskuryakov <a...@webkit.org>
# Copyright (C) 2006 Apple Computer, Inc.
-# Copyright (C) 2007, 2008, 2009 Google Inc.
+# Copyright (C) 2007, 2008, 2009, 2012 Google Inc.
# Copyright (C) 2009 Cameron McCormack <c...@mcc.id.au>
# Copyright (C) Research In Motion Limited 2010. All rights reserved.
# Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
@@ -2998,7 +2998,8 @@
my $paramName = $param->name;
push(@implContent, " v8::Handle<v8::Value> ${paramName}Handle = " . NativeToJSValue($param, $paramName) . ";\n");
push(@implContent, " if (${paramName}Handle.IsEmpty()) {\n");
- push(@implContent, " CRASH();\n");
+ push(@implContent, " if (!isScriptControllerTerminating())\n");
+ push(@implContent, " CRASH();\n");
push(@implContent, " return true;\n");
push(@implContent, " }\n");
push(@args, " ${paramName}Handle");
Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestCallback.cpp (110130 => 110131)
--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestCallback.cpp 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestCallback.cpp 2012-03-08 03:07:44 UTC (rev 110131)
@@ -87,7 +87,8 @@
v8::Handle<v8::Value> class1ParamHandle = toV8(class1Param);
if (class1ParamHandle.IsEmpty()) {
- CRASH();
+ if (!isScriptControllerTerminating())
+ CRASH();
return true;
}
@@ -114,12 +115,14 @@
v8::Handle<v8::Value> class2ParamHandle = toV8(class2Param);
if (class2ParamHandle.IsEmpty()) {
- CRASH();
+ if (!isScriptControllerTerminating())
+ CRASH();
return true;
}
v8::Handle<v8::Value> strArgHandle = v8String(strArg);
if (strArgHandle.IsEmpty()) {
- CRASH();
+ if (!isScriptControllerTerminating())
+ CRASH();
return true;
}
@@ -147,7 +150,8 @@
v8::Handle<v8::Value> listParamHandle = toV8(listParam);
if (listParamHandle.IsEmpty()) {
- CRASH();
+ if (!isScriptControllerTerminating())
+ CRASH();
return true;
}
Modified: trunk/Source/WebCore/bindings/v8/WorkerScriptController.cpp (110130 => 110131)
--- trunk/Source/WebCore/bindings/v8/WorkerScriptController.cpp 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/bindings/v8/WorkerScriptController.cpp 2012-03-08 03:07:44 UTC (rev 110131)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009 Google Inc. All rights reserved.
+ * Copyright (C) 2009, 2012 Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -53,6 +53,7 @@
: m_workerContext(workerContext)
, m_isolate(v8::Isolate::New())
, m_executionForbidden(false)
+ , m_executionScheduledToTerminate(false)
{
V8BindingPerIsolateData* data = ""
data->allStores().append(&m_DOMDataStore);
@@ -92,9 +93,23 @@
void WorkerScriptController::scheduleExecutionTermination()
{
+ // The mutex provides a memory barrier to ensure that once
+ // termination is scheduled, isExecutionTerminating will
+ // accurately reflect that state when called from another thread.
+ {
+ MutexLocker locker(m_scheduledTerminationMutex);
+ m_executionScheduledToTerminate = true;
+ }
v8::V8::TerminateExecution(m_isolate);
}
+bool WorkerScriptController::isExecutionTerminating() const
+{
+ // See comments in scheduleExecutionTermination regarding mutex usage.
+ MutexLocker locker(m_scheduledTerminationMutex);
+ return m_executionScheduledToTerminate;
+}
+
void WorkerScriptController::forbidExecution()
{
ASSERT(m_workerContext->isContextThread());
Modified: trunk/Source/WebCore/bindings/v8/WorkerScriptController.h (110130 => 110131)
--- trunk/Source/WebCore/bindings/v8/WorkerScriptController.h 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/bindings/v8/WorkerScriptController.h 2012-03-08 03:07:44 UTC (rev 110131)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009 Google Inc. All rights reserved.
+ * Copyright (C) 2009, 2012 Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -66,6 +66,7 @@
// forbidExecution()/isExecutionForbidden() to guard against reentry into JS.
// Can be called from any thread.
void scheduleExecutionTermination();
+ bool isExecutionTerminating() const;
// Called on Worker thread when JS exits with termination exception caused by forbidExecution() request,
// or by Worker thread termination code to prevent future entry into JS.
@@ -83,6 +84,8 @@
v8::Isolate* m_isolate;
ScopedDOMDataStore m_DOMDataStore;
bool m_executionForbidden;
+ bool m_executionScheduledToTerminate;
+ mutable Mutex m_scheduledTerminationMutex;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp (110130 => 110131)
--- trunk/Source/WebCore/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp 2012-03-08 03:07:44 UTC (rev 110131)
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, Google Inc. All rights reserved.
+ * Copyright (c) 2009, 2012 Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -59,7 +59,8 @@
v8::Handle<v8::Value> transactionHandle = toV8(transaction);
v8::Handle<v8::Value> errorHandle = toV8(error);
if (transactionHandle.IsEmpty() || errorHandle.IsEmpty()) {
- CRASH();
+ if (!isScriptControllerTerminating())
+ CRASH();
return true;
}
Modified: trunk/Source/WebCore/bindings/v8/custom/V8MutationCallbackCustom.cpp (110130 => 110131)
--- trunk/Source/WebCore/bindings/v8/custom/V8MutationCallbackCustom.cpp 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/bindings/v8/custom/V8MutationCallbackCustom.cpp 2012-03-08 03:07:44 UTC (rev 110131)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010 Google Inc. All rights reserved.
+ * Copyright (C) 2010, 2012 Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -70,7 +70,8 @@
v8::Handle<v8::Value> observerHandle = toV8(observer);
if (observerHandle.IsEmpty()) {
- CRASH();
+ if (!isScriptControllerTerminating())
+ CRASH();
return true;
}
Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.cpp (110130 => 110131)
--- trunk/Source/WebCore/dom/ScriptExecutionContext.cpp 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.cpp 2012-03-08 03:07:44 UTC (rev 110131)
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2008 Apple Inc. All Rights Reserved.
+ * Copyright (C) 2012 Google Inc. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -91,6 +92,8 @@
, m_inDestructor(false)
, m_inDispatchErrorEvent(false)
, m_activeDOMObjectsAreSuspended(false)
+ , m_reasonForSuspendingActiveDOMObjects(static_cast<ActiveDOMObject::ReasonForSuspension>(-1))
+ , m_activeDOMObjectsAreStopped(false)
{
}
@@ -214,6 +217,7 @@
void ScriptExecutionContext::stopActiveDOMObjects()
{
+ m_activeDOMObjectsAreStopped = true;
// No protection against m_activeDOMObjects changing during iteration: stop() shouldn't execute arbitrary JS.
m_iteratingActiveDOMObjects = true;
HashMap<ActiveDOMObject*, void*>::iterator activeObjectsEnd = m_activeDOMObjects.end();
Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.h (110130 => 110131)
--- trunk/Source/WebCore/dom/ScriptExecutionContext.h 2012-03-08 03:02:40 UTC (rev 110130)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.h 2012-03-08 03:07:44 UTC (rev 110131)
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2008 Apple Inc. All Rights Reserved.
+ * Copyright (C) 2012 Google Inc. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -97,6 +98,7 @@
virtual void stopActiveDOMObjects();
bool activeDOMObjectsAreSuspended() const { return m_activeDOMObjectsAreSuspended; }
+ bool activeDOMObjectsAreStopped() const { return m_activeDOMObjectsAreStopped; }
// Called from the constructor and destructors of ActiveDOMObject.
void didCreateActiveDOMObject(ActiveDOMObject*, void* upcastPointer);
@@ -210,6 +212,7 @@
bool m_activeDOMObjectsAreSuspended;
ActiveDOMObject::ReasonForSuspension m_reasonForSuspendingActiveDOMObjects;
+ bool m_activeDOMObjectsAreStopped;
#if ENABLE(BLOB) || ENABLE(FILE_SYSTEM)
RefPtr<FileThread> m_fileThread;
