Changeset 100721 in webkit


Ignore:
Timestamp:
Nov 17, 2011 9:41:33 PM (12 years ago)
Author:
adamk@chromium.org
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.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r100719 r100721  
     12011-11-17  Adam Klein  <adamk@chromium.org>
     2
     3        Move JS recursion counter from V8Proxy to V8BindingPerIsolateData
     4        https://bugs.webkit.org/show_bug.cgi?id=72645
     5
     6        Reviewed by Adam Barth.
     7
     8        Added tests to exercise new timing of call to V8Proxy::didLeaveScriptContext().
     9
     10        * storage/indexeddb/transaction-abort-with-js-recursion-cross-frame-expected.txt: Added.
     11        * storage/indexeddb/transaction-abort-with-js-recursion-cross-frame.html: Added.
     12
    1132011-11-17  Peter Kasting  <pkasting@google.com>
    214

  • trunk/Source/WebCore/ChangeLog

    r100718 r100721  
     12011-11-17  Adam Klein  <adamk@chromium.org>
     2
     3        Move JS recursion counter from V8Proxy to V8BindingPerIsolateData
     4        https://bugs.webkit.org/show_bug.cgi?id=72645
     5
     6        Reviewed by Adam Barth.
     7
     8        With the JS recursion level stored as a member of V8Proxy, it's tied
     9        to a frame. But this is incorrect, as there's no reason that a JS call
     10        stack need be restricted to a single frame (see my new test case for
     11        an example of code going across frames).
     12
     13        In order to get the correct accounting of JS recursion level, per-Isolate
     14        is the right granularity (per dslomov), which is what this patch accomplishes.
     15
     16        Test: storage/indexeddb/transaction-abort-with-js-recursion-cross-frame.html
     17
     18        * bindings/v8/V8Binding.cpp:
     19        (WebCore::V8BindingPerIsolateData::V8BindingPerIsolateData):
     20        * bindings/v8/V8Binding.h:
     21        (WebCore::V8BindingPerIsolateData::recursionLevel):
     22        (WebCore::V8BindingPerIsolateData::incrementRecursionLevel):
     23        (WebCore::V8BindingPerIsolateData::decrementRecursionLevel):
     24        (WebCore::V8RecursionScope::V8RecursionScope):
     25        (WebCore::V8RecursionScope::~V8RecursionScope):
     26        * bindings/v8/V8Proxy.cpp:
     27        (WebCore::incrementRecursionLevel):
     28        (WebCore::decrementRecursionLevel):
     29        (WebCore::recursionLevel):
     30        (WebCore::V8Proxy::V8Proxy):
     31        (WebCore::V8Proxy::runScript):
     32        (WebCore::V8Proxy::callFunction):
     33        (WebCore::V8Proxy::didLeaveScriptContext):
     34        * bindings/v8/V8Proxy.h:
     35
    1362011-11-17  Robin Cao  <robin.cao@torchmobile.com.cn>
    237

  • trunk/Source/WebCore/bindings/v8/V8Binding.cpp

    r97839 r100721  
    5353    : m_domDataStore(0)
    5454    , m_constructorMode(ConstructorMode::CreateNewObject)
     55    , m_recursionLevel(0)
    5556{
    5657}

  • trunk/Source/WebCore/bindings/v8/V8Binding.h

    r99632 r100721  
    3939#include "V8GCController.h"
    4040#include "V8HiddenPropertyName.h"
     41#include <wtf/Noncopyable.h>
    4142#include <wtf/text/AtomicString.h>
    4243
    … …  
    144145        void setDOMDataStore(DOMDataStore* store) { m_domDataStore = store; }
    145146
     147        int recursionLevel() const { return m_recursionLevel; }
     148        void incrementRecursionLevel() { ++m_recursionLevel; }
     149        void decrementRecursionLevel() { --m_recursionLevel; }
     150
    146151#ifndef NDEBUG
    147152        GlobalHandleMap& globalHandleMap() { return m_globalHandleMap; }
    … …  
    167172        friend class ConstructorMode;
    168173
     174        int m_recursionLevel;
     175
    169176#ifndef NDEBUG
    170177        GlobalHandleMap m_globalHandleMap;
    171178#endif
     179    };
     180
     181    class V8RecursionScope {
     182        WTF_MAKE_NONCOPYABLE(V8RecursionScope);
     183    public:
     184        V8RecursionScope() { V8BindingPerIsolateData::current()->incrementRecursionLevel(); }
     185        ~V8RecursionScope() { V8BindingPerIsolateData::current()->decrementRecursionLevel(); }
    172186    };
    173187

  • trunk/Source/WebCore/bindings/v8/V8Proxy.cpp

    r100376 r100721  
    176176}
    177177
     178static int recursionLevel()
     179{
     180    return V8BindingPerIsolateData::current()->recursionLevel();
     181}
     182
    178183static v8::Local<v8::Value> handleMaxRecursionDepthExceeded()
    179184{
    … …  
    186191    , m_windowShell(V8DOMWindowShell::create(frame))
    187192    , m_inlineCode(false)
    188     , m_recursion(0)
    189193{
    190194}
    … …  
    391395
    392396    V8GCController::checkMemoryUsage();
    393     if (m_recursion >= kMaxRecursionDepth)
     397    if (recursionLevel() >= kMaxRecursionDepth)
    394398        return handleMaxRecursionDepthExceeded();
    395399
    … …  
    410414    tryCatch.SetVerbose(true);
    411415    {
    412         m_recursion++;
     416        V8RecursionScope recursionScope;
    413417        result = script->Run();
    414         m_recursion--;
    415     }
    416 
    417     // Release the storage mutex if applicable.
     418    }
     419
    418420    didLeaveScriptContext();
    419421
    … …  
    443445    V8GCController::checkMemoryUsage();
    444446
    445     if (m_recursion >= kMaxRecursionDepth)
     447    if (recursionLevel() >= kMaxRecursionDepth)
    446448        return handleMaxRecursionDepthExceeded();
    447449
    … …  
    451453    v8::Local<v8::Value> result;
    452454    {
    453         m_recursion++;
     455        V8RecursionScope recursionScope;
    454456        result = V8Proxy::instrumentedCallFunction(m_frame->page(), function, receiver, argc, args);
    455         m_recursion--;
    456     }
    457 
    458     // Release the storage mutex if applicable.
     457    }
     458
    459459    didLeaveScriptContext();
    460460
    … …  
    577577void V8Proxy::didLeaveScriptContext()
    578578{
    579     if (m_recursion)
     579    if (recursionLevel())
    580580        return;
    581581

  • trunk/Source/WebCore/bindings/v8/V8Proxy.h

    r100003 r100721  
    272272
    273273    private:
    274         void didLeaveScriptContext();
     274        static void didLeaveScriptContext();
    275275
    276276        void resetIsolatedWorlds();
    … …  
    303303        // Only valid during execution.
    304304        bool m_inlineCode;
    305 
    306         // Track the recursion depth to be able to avoid too deep recursion. The V8
    307         // engine allows much more recursion than KJS does so we need to guard against
    308         // excessive recursion in the binding layer.
    309         int m_recursion;
    310305
    311306        // All of the extensions registered with the context.
Note: See TracChangeset for help on using the changeset viewer.