Attachment 159331 Details for Bug 94443 – Patch

[patch] Patch

bug-94443-20120820141452.patch (text/plain), 10.83 KB, created by Kentaro Hara on 2012-08-19 22:15:31 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Kentaro Hara

Created: 2012-08-19 22:15:31 PDT

Size: 10.83 KB

patch

obsolete

>Subversion Revision: 125995
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 8b98078e4ab6b04dfc351a59d98c2d7b95db5e01..ccb694b7eba9d8bff414166d355673c9575b6f24 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,5 +1,46 @@
> 2012-08-19  Kentaro Hara  <haraken@chromium.org>
> 
>+        [V8] Move V8Proxy::newInstance() to V8ObjectConstructor
>+        https://bugs.webkit.org/show_bug.cgi?id=94443
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        To kill V8Proxy, this patch moves V8Proxy::newInstance() to
>+        V8ObjectConstructor::newInstanceInFrame().
>+        In addition, this patch does the following things:
>+
>+        - For consistency with V8ObjectConstructor::newInstanceInFrame(),
>+        this patch inserts an if(v8::V8::IsDead()) check to just after
>+        Function::NewInstance(). The check is done by V8Binding::assertIfV8IsDead().
>+
>+        - To avoid #include circular dependency, this patch de-inline
>+        V8ObjectConstructor::newInstance()s. I didn't observe any perf regression.
>+        I don't think these methods are worth being inlined, because
>+        these methods call Function::NewInstance(), which is not inlined
>+        and calls a bunch of heavy mehtods in V8.
>+
>+        No tests. No change in behavior.
>+
>+        * bindings/v8/NPV8Object.cpp:
>+        (_NPN_Construct):
>+        * bindings/v8/V8Binding.cpp:
>+        (WebCore::assertIfV8IsDead):
>+        (WebCore):
>+        * bindings/v8/V8Binding.h:
>+        (WebCore):
>+        * bindings/v8/V8ObjectConstructor.cpp:
>+        (WebCore::V8ObjectConstructor::newInstance):
>+        (WebCore):
>+        (WebCore::V8ObjectConstructor::newInstanceInFrame):
>+        * bindings/v8/V8ObjectConstructor.h:
>+        (WebCore):
>+        (V8ObjectConstructor):
>+        * bindings/v8/V8Proxy.cpp:
>+        (WebCore::V8Proxy::runScript):
>+        (WebCore::V8Proxy::instrumentedCallFunction):
>+
>+2012-08-19  Kentaro Hara  <haraken@chromium.org>
>+
>         [V8] Rename SafeAllocation to V8ObjectConstructor
>         https://bugs.webkit.org/show_bug.cgi?id=94436
> 
>diff --git a/Source/WebCore/bindings/v8/NPV8Object.cpp b/Source/WebCore/bindings/v8/NPV8Object.cpp
>index c00082d6cf4a14307eb82f40a4433347372c6bac..a08a32ba9fca49cece75eee8a6ff2e4b32460729 100644
>--- a/Source/WebCore/bindings/v8/NPV8Object.cpp
>+++ b/Source/WebCore/bindings/v8/NPV8Object.cpp
>@@ -591,11 +591,10 @@ bool _NPN_Construct(NPP npp, NPObject* npObject, const NPVariant* arguments, uin
>         v8::Local<v8::Value> resultObject;
>         v8::Handle<v8::Function> ctor(v8::Function::Cast(*ctorObj));
>         if (!ctor->IsNull()) {
>-            V8Proxy* proxy = toV8Proxy(npObject);
>-            ASSERT(proxy);
>-
>+            Frame* frame = object->rootObject->frame();
>+            ASSERT(frame);
>             OwnArrayPtr<v8::Handle<v8::Value> > argv = createValueListFromVariantArgs(arguments, argumentCount, npObject);
>-            resultObject = proxy->newInstance(ctor, argumentCount, argv.get());
>+            resultObject = V8ObjectConstructor::newInstanceInFrame(ctor, argumentCount, argv.get(), frame);
>         }
> 
>         if (resultObject.IsEmpty())
>diff --git a/Source/WebCore/bindings/v8/V8Binding.cpp b/Source/WebCore/bindings/v8/V8Binding.cpp
>index f9a3886ae704e3a89d16bc8bf225e66506c30460..80b3cfb231bbbb37f3b8545f39e403fd7bec249a 100644
>--- a/Source/WebCore/bindings/v8/V8Binding.cpp
>+++ b/Source/WebCore/bindings/v8/V8Binding.cpp
>@@ -381,4 +381,13 @@ PassRefPtr<DOMStringList> toDOMStringList(v8::Handle<v8::Value> value)
>     return ret.release();
> }
> 
>+void assertIfV8IsDead()
>+{
>+    if (v8::V8::IsDead()) {
>+        // FIXME: We temporarily deal with V8 internal error situations
>+        // such as out-of-memory by crashing the renderer.
>+        CRASH();
>+    }
>+}
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/bindings/v8/V8Binding.h b/Source/WebCore/bindings/v8/V8Binding.h
>index cc27f39cc4382db22d142e436302f4826ee1eb12..b77c238dec8b5842eeac77b43461c07fdb125eed 100644
>--- a/Source/WebCore/bindings/v8/V8Binding.h
>+++ b/Source/WebCore/bindings/v8/V8Binding.h
>@@ -368,6 +368,8 @@ namespace WebCore {
> 
>     PassRefPtr<DOMStringList> toDOMStringList(v8::Handle<v8::Value>);
> 
>+    void assertIfV8IsDead();
>+
>     class V8ParameterBase {
>     public:
>         operator String() { return toString<String>(); }
>diff --git a/Source/WebCore/bindings/v8/V8ObjectConstructor.cpp b/Source/WebCore/bindings/v8/V8ObjectConstructor.cpp
>index 91589a7a83a67dd27418e45c7482574c7ca30732..419ac0015c0c1ce2314f592aa1f68adc2d8cee94 100644
>--- a/Source/WebCore/bindings/v8/V8ObjectConstructor.cpp
>+++ b/Source/WebCore/bindings/v8/V8ObjectConstructor.cpp
>@@ -25,10 +25,63 @@
> #include "config.h"
> #include "V8ObjectConstructor.h"
> 
>+#include "Frame.h"
> #include "V8Binding.h"
>+#include "V8RecursionScope.h"
>+
>+#if PLATFORM(CHROMIUM)
>+#include "TraceEvent.h"
>+#endif
> 
> namespace WebCore {
> 
>+v8::Local<v8::Object> V8ObjectConstructor::newInstance(v8::Handle<v8::Function> function)
>+{
>+    if (function.IsEmpty())
>+        return v8::Local<v8::Object>();
>+    ConstructorMode constructorMode;
>+    V8RecursionScope::MicrotaskSuppression scope;
>+    v8::Local<v8::Object> result = function->NewInstance();
>+    assertIfV8IsDead();
>+    return result;
>+}
>+
>+v8::Local<v8::Object> V8ObjectConstructor::newInstance(v8::Handle<v8::ObjectTemplate> objectTemplate)
>+{
>+    if (objectTemplate.IsEmpty())
>+        return v8::Local<v8::Object>();
>+    ConstructorMode constructorMode;
>+    V8RecursionScope::MicrotaskSuppression scope;
>+    v8::Local<v8::Object> result = objectTemplate->NewInstance();
>+    assertIfV8IsDead();
>+    return result;
>+}
>+
>+v8::Local<v8::Object> V8ObjectConstructor::newInstance(v8::Handle<v8::Function> function, int argc, v8::Handle<v8::Value> argv[])
>+{
>+    if (function.IsEmpty())
>+        return v8::Local<v8::Object>();
>+    ConstructorMode constructorMode;
>+    V8RecursionScope::MicrotaskSuppression scope;
>+    v8::Local<v8::Object> result = function->NewInstance(argc, argv);
>+    assertIfV8IsDead();
>+    return result;
>+}
>+
>+v8::Local<v8::Value> V8ObjectConstructor::newInstanceInFrame(v8::Handle<v8::Function> function, int argc, v8::Handle<v8::Value> argv[], Frame* frame)
>+{
>+#if PLATFORM(CHROMIUM)
>+    TRACE_EVENT0("v8", "v8.newInstance");
>+#endif
>+
>+    // No artificial limitations on the depth of recursion, see comment in
>+    // V8Proxy::callFunction.
>+    V8RecursionScope recursionScope(frame ? frame->document() : 0);
>+    v8::Local<v8::Object> result = function->NewInstance(argc, argv);
>+    assertIfV8IsDead();
>+    return result;
>+}
>+
> v8::Handle<v8::Value> V8ObjectConstructor::isValidConstructorMode(const v8::Arguments& args)
> {
>     if (ConstructorMode::current() == ConstructorMode::CreateNewObject)
>diff --git a/Source/WebCore/bindings/v8/V8ObjectConstructor.h b/Source/WebCore/bindings/v8/V8ObjectConstructor.h
>index 2f1cc22aaaba2a87d0b9c7494f1fcf86560d414c..b797311c263b4dd2f99c890c74497b99745bd01f 100644
>--- a/Source/WebCore/bindings/v8/V8ObjectConstructor.h
>+++ b/Source/WebCore/bindings/v8/V8ObjectConstructor.h
>@@ -32,12 +32,13 @@
> #define V8ObjectConstructor_h
> 
> #include "V8PerIsolateData.h"
>-#include "V8RecursionScope.h"
> 
> #include <v8.h>
> 
> namespace WebCore {
> 
>+class Frame;
>+
> class ConstructorMode {
> public:
>     enum Mode {
>@@ -66,40 +67,14 @@ private:
> 
> class V8ObjectConstructor {
> public:
>-    static inline v8::Local<v8::Object> newInstance(v8::Handle<v8::Function>);
>-    static inline v8::Local<v8::Object> newInstance(v8::Handle<v8::ObjectTemplate>);
>-    static inline v8::Local<v8::Object> newInstance(v8::Handle<v8::Function>, int argc, v8::Handle<v8::Value> argv[]);
>+    static v8::Local<v8::Object> newInstance(v8::Handle<v8::Function>);
>+    static v8::Local<v8::Object> newInstance(v8::Handle<v8::ObjectTemplate>);
>+    static v8::Local<v8::Object> newInstance(v8::Handle<v8::Function>, int, v8::Handle<v8::Value> argv[]);
>+    static v8::Local<v8::Value> newInstanceInFrame(v8::Handle<v8::Function>, int, v8::Handle<v8::Value> argv[], Frame*);
> 
>     static v8::Handle<v8::Value> isValidConstructorMode(const v8::Arguments&);
> };
> 
>-v8::Local<v8::Object> V8ObjectConstructor::newInstance(v8::Handle<v8::Function> function)
>-{
>-    if (function.IsEmpty())
>-        return v8::Local<v8::Object>();
>-    ConstructorMode constructorMode;
>-    V8RecursionScope::MicrotaskSuppression scope;
>-    return function->NewInstance();
>-}
>-
>-v8::Local<v8::Object> V8ObjectConstructor::newInstance(v8::Handle<v8::ObjectTemplate> objectTemplate)
>-{
>-    if (objectTemplate.IsEmpty())
>-        return v8::Local<v8::Object>();
>-    ConstructorMode constructorMode;
>-    V8RecursionScope::MicrotaskSuppression scope;
>-    return objectTemplate->NewInstance();
>-}
>-
>-v8::Local<v8::Object> V8ObjectConstructor::newInstance(v8::Handle<v8::Function> function, int argc, v8::Handle<v8::Value> argv[])
>-{
>-    if (function.IsEmpty())
>-        return v8::Local<v8::Object>();
>-    ConstructorMode constructorMode;
>-    V8RecursionScope::MicrotaskSuppression scope;
>-    return function->NewInstance(argc, argv);
>-}
>-
> } // namespace WebCore
> 
> #endif // V8ObjectConstructor_h
>diff --git a/Source/WebCore/bindings/v8/V8Proxy.cpp b/Source/WebCore/bindings/v8/V8Proxy.cpp
>index ab273aed0f57851b1a231e986d3fc2055200d838..81671223abb6f30521caaf17d69f89f91b958afd 100644
>--- a/Source/WebCore/bindings/v8/V8Proxy.cpp
>+++ b/Source/WebCore/bindings/v8/V8Proxy.cpp
>@@ -105,13 +105,6 @@ void V8Proxy::reportUnsafeAccessTo(Document* targetDocument)
>     sourceDocument->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, str, stackTrace.release());
> }
> 
>-static void handleFatalErrorInV8()
>-{
>-    // FIXME: We temporarily deal with V8 internal error situations
>-    // such as out-of-memory by crashing the renderer.
>-    CRASH();
>-}
>-
> static v8::Local<v8::Value> handleMaxRecursionDepthExceeded()
> {
>     throwError(RangeError, "Maximum call stack size exceeded.");
>@@ -262,9 +255,7 @@ v8::Local<v8::Value> V8Proxy::runScript(v8::Handle<v8::Script> script)
>     if (result.IsEmpty())
>         return v8::Local<v8::Value>();
> 
>-    if (v8::V8::IsDead())
>-        handleFatalErrorInV8();
>-
>+    assertIfV8IsDead();
>     return result;
> }
> 
>@@ -327,30 +318,7 @@ v8::Local<v8::Value> V8Proxy::instrumentedCallFunction(Frame* frame, v8::Handle<
>     }
> 
>     InspectorInstrumentation::didCallFunction(cookie);
>-
>-    if (v8::V8::IsDead())
>-        handleFatalErrorInV8();
>-
>-    return result;
>-}
>-
>-v8::Local<v8::Value> V8Proxy::newInstance(v8::Handle<v8::Function> constructor, int argc, v8::Handle<v8::Value> args[])
>-{
>-#if PLATFORM(CHROMIUM)
>-    TRACE_EVENT0("v8", "v8.newInstance");
>-#endif
>-
>-    // No artificial limitations on the depth of recursion, see comment in
>-    // V8Proxy::callFunction.
>-    v8::Local<v8::Value> result;
>-    {
>-        V8RecursionScope recursionScope(frame() ? frame()->document() : 0);
>-        result = constructor->NewInstance(argc, args);
>-    }
>-
>-    if (v8::V8::IsDead())
>-        handleFatalErrorInV8();
>-
>+    assertIfV8IsDead();
>     return result;
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 94443: 159331 | 159332 | 159339 | 159345