Simplify and correct mutation delivery timing for JSC

https://bugs.webkit.org/show_bug.cgi?id=78172

Reviewed by Adam Barth.

Instead of keeping a static recursion counter in JSMainThreadExecState,
simply wait for a state change from non-null ExecState to null ExecState.
Because s_mainThreadState is initially null, this equivalent to
waiting for s_recursionLevel to rewind to zero.

This also properly handles the usage of JSMainThreadNullState (and
does not do mutation delivery), since that class is only used by
non-JS bindings. Now fast/mutation/end-of-task-delivery.html properly
fails, whereas it was passing before due to usage of the ObjC DOM API
from DumpRenderTree.

* bindings/js/JSMainThreadExecState.cpp:
(WebCore):
* bindings/js/JSMainThreadExecState.h: Added a comment explaining the purpose of JSMainThreadNullState.
(WebCore::JSMainThreadExecState::JSMainThreadExecState):
(WebCore::JSMainThreadExecState::~JSMainThreadExecState):
(JSMainThreadExecState):
(WebCore):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@107149 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 91f0f7a..ae5726a 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,29 @@
+2012-02-08  Adam Klein  <adamk@chromium.org>
+
+        Simplify and correct mutation delivery timing for JSC
+        https://bugs.webkit.org/show_bug.cgi?id=78172
+
+        Reviewed by Adam Barth.
+
+        Instead of keeping a static recursion counter in JSMainThreadExecState,
+        simply wait for a state change from non-null ExecState to null ExecState.
+        Because s_mainThreadState is initially null, this equivalent to
+        waiting for s_recursionLevel to rewind to zero.
+
+        This also properly handles the usage of JSMainThreadNullState (and
+        does not do mutation delivery), since that class is only used by
+        non-JS bindings. Now fast/mutation/end-of-task-delivery.html properly
+        fails, whereas it was passing before due to usage of the ObjC DOM API
+        from DumpRenderTree.
+
+        * bindings/js/JSMainThreadExecState.cpp:
+        (WebCore):
+        * bindings/js/JSMainThreadExecState.h: Added a comment explaining the purpose of JSMainThreadNullState.
+        (WebCore::JSMainThreadExecState::JSMainThreadExecState):
+        (WebCore::JSMainThreadExecState::~JSMainThreadExecState):
+        (JSMainThreadExecState):
+        (WebCore):
+
 2012-02-08  Kentaro Hara  <haraken@chromium.org>
 
         Remove [ConvertToString] from CodeGeneratorCPP.pm and rename

diff --git a/Source/WebCore/bindings/js/JSMainThreadExecState.cpp b/Source/WebCore/bindings/js/JSMainThreadExecState.cpp
index 1e21bfa..9177a4b 100644 (file)
--- a/Source/WebCore/bindings/js/JSMainThreadExecState.cpp
+++ b/Source/WebCore/bindings/js/JSMainThreadExecState.cpp
@@ -32,8 +32,6 @@ namespace WebCore {
 JSC::ExecState* JSMainThreadExecState::s_mainThreadState = 0;
 
 #if ENABLE(MUTATION_OBSERVERS)
-int JSMainThreadExecState::s_recursionLevel = 0;
-
 void JSMainThreadExecState::didLeaveScriptContext()
 {
     WebKitMutationObserver::deliverAllMutations();

diff --git a/Source/WebCore/bindings/js/JSMainThreadExecState.h b/Source/WebCore/bindings/js/JSMainThreadExecState.h
index 9093db6..5e4c4e2 100644 (file)
--- a/Source/WebCore/bindings/js/JSMainThreadExecState.h
+++ b/Source/WebCore/bindings/js/JSMainThreadExecState.h
@@ -91,21 +91,20 @@ protected:
     {
         ASSERT(isMainThread());
         s_mainThreadState = exec;
-
-#if ENABLE(MUTATION_OBSERVERS)
-        ASSERT(s_recursionLevel >= 0);
-        ++s_recursionLevel;
-#endif
     };
 
     ~JSMainThreadExecState()
     {
         ASSERT(isMainThread());
+
+#if ENABLE(MUTATION_OBSERVERS)
+        bool didExitJavaScript = s_mainThreadState && !m_previousState;
+#endif
+
         s_mainThreadState = m_previousState;
 
 #if ENABLE(MUTATION_OBSERVERS)
-        ASSERT(s_recursionLevel > 0);
-        if (!--s_recursionLevel)
+        if (didExitJavaScript)
             didLeaveScriptContext();
 #endif
     }
@@ -116,11 +115,11 @@ private:
 
 #if ENABLE(MUTATION_OBSERVERS)
     static void didLeaveScriptContext();
-    static int s_recursionLevel;
 #endif
 };
 
 // Null state prevents origin security checks.
+// Used by non-JavaScript bindings (ObjC, GObject).
 class JSMainThreadNullState : private JSMainThreadExecState {
 public:
     explicit JSMainThreadNullState() : JSMainThreadExecState(0) {};