- Revision
- 100721
- Author
- [email protected]
- Date
- 2011-11-17 21:41:33 -0800 (Thu, 17 Nov 2011)
Log Message
Move JS recursion counter from V8Proxy to V8BindingPerIsolateData
https://bugs.webkit.org/show_bug.cgi?id=72645
Reviewed by Adam Barth.
Source/WebCore:
With the JS recursion level stored as a member of V8Proxy, it's tied
to a frame. But this is incorrect, as there's no reason that a JS call
stack need be restricted to a single frame (see my new test case for
an example of code going across frames).
In order to get the correct accounting of JS recursion level, per-Isolate
is the right granularity (per dslomov), which is what this patch accomplishes.
Test: storage/indexeddb/transaction-abort-with-js-recursion-cross-frame.html
* bindings/v8/V8Binding.cpp:
(WebCore::V8BindingPerIsolateData::V8BindingPerIsolateData):
* bindings/v8/V8Binding.h:
(WebCore::V8BindingPerIsolateData::recursionLevel):
(WebCore::V8BindingPerIsolateData::incrementRecursionLevel):
(WebCore::V8BindingPerIsolateData::decrementRecursionLevel):
(WebCore::V8RecursionScope::V8RecursionScope):
(WebCore::V8RecursionScope::~V8RecursionScope):
* bindings/v8/V8Proxy.cpp:
(WebCore::incrementRecursionLevel):
(WebCore::decrementRecursionLevel):
(WebCore::recursionLevel):
(WebCore::V8Proxy::V8Proxy):
(WebCore::V8Proxy::runScript):
(WebCore::V8Proxy::callFunction):
(WebCore::V8Proxy::didLeaveScriptContext):
* bindings/v8/V8Proxy.h:
LayoutTests:
Added tests to exercise new timing of call to V8Proxy::didLeaveScriptContext().
* storage/indexeddb/transaction-abort-with-js-recursion-cross-frame-expected.txt: Added.
* storage/indexeddb/transaction-abort-with-js-recursion-cross-frame.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (100720 => 100721)
--- trunk/LayoutTests/ChangeLog 2011-11-18 05:38:57 UTC (rev 100720)
+++ trunk/LayoutTests/ChangeLog 2011-11-18 05:41:33 UTC (rev 100721)
@@ -1,3 +1,15 @@
+2011-11-17 Adam Klein <[email protected]>
+
+ Move JS recursion counter from V8Proxy to V8BindingPerIsolateData
+ https://bugs.webkit.org/show_bug.cgi?id=72645
+
+ Reviewed by Adam Barth.
+
+ Added tests to exercise new timing of call to V8Proxy::didLeaveScriptContext().
+
+ * storage/indexeddb/transaction-abort-with-js-recursion-cross-frame-expected.txt: Added.
+ * storage/indexeddb/transaction-abort-with-js-recursion-cross-frame.html: Added.
+
2011-11-17 Peter Kasting <[email protected]>
[chromium] acid3.html didn't actually get rebaselined like I wanted.
Added: trunk/LayoutTests/storage/indexeddb/transaction-abort-with-js-recursion-cross-frame-expected.txt (0 => 100721)
--- trunk/LayoutTests/storage/indexeddb/transaction-abort-with-js-recursion-cross-frame-expected.txt (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/transaction-abort-with-js-recursion-cross-frame-expected.txt 2011-11-18 05:41:33 UTC (rev 100721)
@@ -0,0 +1,21 @@
+Test that pending transactions are not aborted during recursive JS calls until all JS (in all frames) is finished.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS 'webkitIndexedDB' in window is true
+PASS webkitIndexedDB == null is false
+webkitIndexedDB.open('transaction-abort-with-js-recursion-cross-frame')
+db = event.target.result
+db.setVersion('new version')
+pendingTransaction = db.transaction(['objectStore'], webkitIDBTransaction.READ_WRITE)
+Start re-entrant JS
+transaction = db.transaction(['objectStore'], webkitIDBTransaction.READ_WRITE)
+End re-entrant JS
+store = pendingTransaction.objectStore('objectStore')
+PASS store !== undefined is true
+Pending transaction aborted
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/storage/indexeddb/transaction-abort-with-js-recursion-cross-frame.html (0 => 100721)
--- trunk/LayoutTests/storage/indexeddb/transaction-abort-with-js-recursion-cross-frame.html (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/transaction-abort-with-js-recursion-cross-frame.html 2011-11-18 05:41:33 UTC (rev 100721)
@@ -0,0 +1,73 @@
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<iframe>
+ <body>
+ </body>
+</iframe>
+<script>
+
+var transaction;
+var store;
+var db;
+var body = document.getElementsByTagName("iframe")[0].contentDocument.body;
+
+description("Test that pending transactions are not aborted during recursive JS calls until all JS (in all frames) is finished.");
+if (window.layoutTestController)
+ layoutTestController.waitUntilDone();
+
+function setup() {
+ shouldBeTrue("'webkitIndexedDB' in window");
+ shouldBeFalse("webkitIndexedDB == null");
+
+ request = evalAndLog("webkitIndexedDB.open('transaction-abort-with-js-recursion-cross-frame')");
+ request._onsuccess_ = setVersion;
+ request._onerror_ = unexpectedErrorCallback;
+}
+
+function setVersion() {
+ db = evalAndLog("db = event.target.result");
+ request = evalAndLog("db.setVersion('new version')");
+ request._onsuccess_ = click;
+ request._onerror_ = unexpectedErrorCallback;
+}
+
+function click() {
+ store = db.createObjectStore('objectStore', null);
+ body._onclick_ = test;
+ var pendingTransaction = evalAndLog("pendingTransaction = db.transaction(['objectStore'], webkitIDBTransaction.READ_WRITE)");
+ pendingTransaction._onsuccess_ = unexpectedErrorCallback;
+ pendingTransaction._onerror_ = unexpectedErrorCallback;
+ pendingTransaction._onabort_ = abortCallback;
+ var event = document.createEvent("MouseEvent");
+ event.initMouseEvent("click", true, true, document.defaultView, 0, 0, 0, 0, 0, false, false, false, false, 0, null);
+ body.dispatchEvent(event);
+ var store = evalAndLog("store = pendingTransaction.objectStore('objectStore')");
+ shouldBeTrue("store !== undefined");
+ body._onclick_ = undefined;
+}
+
+function test()
+{
+ debug("Start re-entrant JS");
+ transaction = evalAndLog("transaction = db.transaction(['objectStore'], webkitIDBTransaction.READ_WRITE)");
+ debug("End re-entrant JS");
+}
+
+
+function abortCallback()
+{
+ debug("Pending transaction aborted");
+ done();
+}
+
+setup();
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (100720 => 100721)
--- trunk/Source/WebCore/ChangeLog 2011-11-18 05:38:57 UTC (rev 100720)
+++ trunk/Source/WebCore/ChangeLog 2011-11-18 05:41:33 UTC (rev 100721)
@@ -1,3 +1,38 @@
+2011-11-17 Adam Klein <[email protected]>
+
+ Move JS recursion counter from V8Proxy to V8BindingPerIsolateData
+ https://bugs.webkit.org/show_bug.cgi?id=72645
+
+ Reviewed by Adam Barth.
+
+ With the JS recursion level stored as a member of V8Proxy, it's tied
+ to a frame. But this is incorrect, as there's no reason that a JS call
+ stack need be restricted to a single frame (see my new test case for
+ an example of code going across frames).
+
+ In order to get the correct accounting of JS recursion level, per-Isolate
+ is the right granularity (per dslomov), which is what this patch accomplishes.
+
+ Test: storage/indexeddb/transaction-abort-with-js-recursion-cross-frame.html
+
+ * bindings/v8/V8Binding.cpp:
+ (WebCore::V8BindingPerIsolateData::V8BindingPerIsolateData):
+ * bindings/v8/V8Binding.h:
+ (WebCore::V8BindingPerIsolateData::recursionLevel):
+ (WebCore::V8BindingPerIsolateData::incrementRecursionLevel):
+ (WebCore::V8BindingPerIsolateData::decrementRecursionLevel):
+ (WebCore::V8RecursionScope::V8RecursionScope):
+ (WebCore::V8RecursionScope::~V8RecursionScope):
+ * bindings/v8/V8Proxy.cpp:
+ (WebCore::incrementRecursionLevel):
+ (WebCore::decrementRecursionLevel):
+ (WebCore::recursionLevel):
+ (WebCore::V8Proxy::V8Proxy):
+ (WebCore::V8Proxy::runScript):
+ (WebCore::V8Proxy::callFunction):
+ (WebCore::V8Proxy::didLeaveScriptContext):
+ * bindings/v8/V8Proxy.h:
+
2011-11-17 Robin Cao <[email protected]>
[chromium] Font::drawComplexText can not draw a segment of text run
Modified: trunk/Source/WebCore/bindings/v8/V8Binding.cpp (100720 => 100721)
--- trunk/Source/WebCore/bindings/v8/V8Binding.cpp 2011-11-18 05:38:57 UTC (rev 100720)
+++ trunk/Source/WebCore/bindings/v8/V8Binding.cpp 2011-11-18 05:41:33 UTC (rev 100721)
@@ -52,6 +52,7 @@
V8BindingPerIsolateData::V8BindingPerIsolateData(v8::Isolate* isolate)
: m_domDataStore(0)
, m_constructorMode(ConstructorMode::CreateNewObject)
+ , m_recursionLevel(0)
{
}
Modified: trunk/Source/WebCore/bindings/v8/V8Binding.h (100720 => 100721)
--- trunk/Source/WebCore/bindings/v8/V8Binding.h 2011-11-18 05:38:57 UTC (rev 100720)
+++ trunk/Source/WebCore/bindings/v8/V8Binding.h 2011-11-18 05:41:33 UTC (rev 100721)
@@ -38,6 +38,7 @@
#include "V8DOMWrapper.h"
#include "V8GCController.h"
#include "V8HiddenPropertyName.h"
+#include <wtf/Noncopyable.h>
#include <wtf/text/AtomicString.h>
#include <v8.h>
@@ -143,6 +144,10 @@
// DOMDataStore is owned outside V8BindingPerIsolateData.
void setDOMDataStore(DOMDataStore* store) { m_domDataStore = store; }
+ int recursionLevel() const { return m_recursionLevel; }
+ void incrementRecursionLevel() { ++m_recursionLevel; }
+ void decrementRecursionLevel() { --m_recursionLevel; }
+
#ifndef NDEBUG
GlobalHandleMap& globalHandleMap() { return m_globalHandleMap; }
#endif
@@ -166,11 +171,20 @@
bool m_constructorMode;
friend class ConstructorMode;
+ int m_recursionLevel;
+
#ifndef NDEBUG
GlobalHandleMap m_globalHandleMap;
#endif
};
+ class V8RecursionScope {
+ WTF_MAKE_NONCOPYABLE(V8RecursionScope);
+ public:
+ V8RecursionScope() { V8BindingPerIsolateData::current()->incrementRecursionLevel(); }
+ ~V8RecursionScope() { V8BindingPerIsolateData::current()->decrementRecursionLevel(); }
+ };
+
class ConstructorMode {
public:
enum Mode {
Modified: trunk/Source/WebCore/bindings/v8/V8Proxy.cpp (100720 => 100721)
--- trunk/Source/WebCore/bindings/v8/V8Proxy.cpp 2011-11-18 05:38:57 UTC (rev 100720)
+++ trunk/Source/WebCore/bindings/v8/V8Proxy.cpp 2011-11-18 05:41:33 UTC (rev 100721)
@@ -175,6 +175,11 @@
CRASH();
}
+static int recursionLevel()
+{
+ return V8BindingPerIsolateData::current()->recursionLevel();
+}
+
static v8::Local<v8::Value> handleMaxRecursionDepthExceeded()
{
throwError("Maximum call stack size exceeded.", V8Proxy::RangeError);
@@ -185,7 +190,6 @@
: m_frame(frame)
, m_windowShell(V8DOMWindowShell::create(frame))
, m_inlineCode(false)
- , m_recursion(0)
{
}
@@ -390,7 +394,7 @@
return notHandledByInterceptor();
V8GCController::checkMemoryUsage();
- if (m_recursion >= kMaxRecursionDepth)
+ if (recursionLevel() >= kMaxRecursionDepth)
return handleMaxRecursionDepthExceeded();
if (handleOutOfMemory())
@@ -409,12 +413,10 @@
v8::TryCatch tryCatch;
tryCatch.SetVerbose(true);
{
- m_recursion++;
+ V8RecursionScope recursionScope;
result = script->Run();
- m_recursion--;
}
- // Release the storage mutex if applicable.
didLeaveScriptContext();
if (handleOutOfMemory())
@@ -442,7 +444,7 @@
{
V8GCController::checkMemoryUsage();
- if (m_recursion >= kMaxRecursionDepth)
+ if (recursionLevel() >= kMaxRecursionDepth)
return handleMaxRecursionDepthExceeded();
// Keep Frame (and therefore ScriptController and V8Proxy) alive.
@@ -450,12 +452,10 @@
v8::Local<v8::Value> result;
{
- m_recursion++;
+ V8RecursionScope recursionScope;
result = V8Proxy::instrumentedCallFunction(m_frame->page(), function, receiver, argc, args);
- m_recursion--;
}
- // Release the storage mutex if applicable.
didLeaveScriptContext();
if (v8::V8::IsDead())
@@ -576,7 +576,7 @@
void V8Proxy::didLeaveScriptContext()
{
- if (m_recursion)
+ if (recursionLevel())
return;
#if ENABLE(INDEXED_DATABASE)
Modified: trunk/Source/WebCore/bindings/v8/V8Proxy.h (100720 => 100721)
--- trunk/Source/WebCore/bindings/v8/V8Proxy.h 2011-11-18 05:38:57 UTC (rev 100720)
+++ trunk/Source/WebCore/bindings/v8/V8Proxy.h 2011-11-18 05:41:33 UTC (rev 100721)
@@ -271,7 +271,7 @@
static void reportUnsafeAccessTo(Frame* target);
private:
- void didLeaveScriptContext();
+ static void didLeaveScriptContext();
void resetIsolatedWorlds();
@@ -303,11 +303,6 @@
// Only valid during execution.
bool m_inlineCode;
- // Track the recursion depth to be able to avoid too deep recursion. The V8
- // engine allows much more recursion than KJS does so we need to guard against
- // excessive recursion in the binding layer.
- int m_recursion;
-
// All of the extensions registered with the context.
static V8Extensions m_extensions;
