Attachment 162624 Details for Bug 96034 – Patch for landing

[patch] Patch for landing

bug-96034-20120906172015.patch (text/plain), 13.08 KB, created by Elliott Sprehn on 2012-09-06 17:20:35 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Elliott Sprehn

Created: 2012-09-06 17:20:35 PDT

Size: 13.08 KB

patch

obsolete

>Subversion Revision: 127764
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 39a6a3a13ab3aba5f711a39057d160290130820e..8d71cffb01448d5ca061e0430d8c5e71019a4286 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,33 @@
>+2012-09-06  Elliott Sprehn  <esprehn@chromium.org>
>+
>+        Add new JSDependentRetained that allows keeping a JSObject alive as long as another is alive
>+        https://bugs.webkit.org/show_bug.cgi?id=96034
>+
>+        Reviewed by Geoffrey Garen.
>+
>+        Add new JSDependentRetained (with the same API as V8DependentRetained) that allows keeping
>+        a JSObject alive as long as another is alive. This is useful for keeping callbacks on
>+        wrappers without keeping strong references into the JS heap in C++ which can result in
>+        cycles that create memory leaks.
>+
>+        No new tests needed, this will be used to fix MutationObservers and UndoManager which will have tests.
>+
>+        * GNUmakefile.list.am:
>+        * WebCore.vcproj/WebCore.vcproj:
>+        * WebCore.xcodeproj/project.pbxproj:
>+        * bindings/js/JSDependentRetained.h: Added.
>+        (WebCore):
>+        (JSDependentRetained):
>+        (WebCore::JSDependentRetained::JSDependentRetained):
>+        (WebCore::JSDependentRetained::~JSDependentRetained):
>+        (WebCore::JSDependentRetained::get): Get the JSObject value.
>+        (WebCore::JSDependentRetained::isEmpty): Check if the value is still alive.
>+        (WebCore::JSDependentRetained::retain): Sets the owner of the object, should only be used once.
>+        (WebCore::JSDependentRetained::release):
>+        * bindings/v8/V8DependentRetained.h:
>+        (WebCore::V8DependentRetained::V8DependentRetained):
>+        (WebCore::V8DependentRetained::retain): Added this method so the API for JSC and V8 are identical.
>+
> 2012-09-06  Zan Dobersek  <zandobersek@gmail.com>
> 
>         Unreviewed GTK build fix.
>diff --git a/Source/WebCore/GNUmakefile.list.am b/Source/WebCore/GNUmakefile.list.am
>index 1e5410f5ae8745625cedc7a6ae0b8ab8421a3349..86ff5e2f68bccd7ce8303c63c0777d96d9e58830 100644
>--- a/Source/WebCore/GNUmakefile.list.am
>+++ b/Source/WebCore/GNUmakefile.list.am
>@@ -2235,6 +2235,7 @@ webcore_sources += \
> 	Source/WebCore/bindings/js/JSCustomSQLStatementErrorCallback.cpp \
> 	Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp \
> 	Source/WebCore/bindings/js/JSCustomXPathNSResolver.h \
>+	Source/WebCore/bindings/js/JSDependentRetained.h \
> 	Source/WebCore/bindings/js/JSDictionary.cpp \
> 	Source/WebCore/bindings/js/JSDictionary.h \
> 	Source/WebCore/bindings/js/JSDOMBinding.cpp \
>diff --git a/Source/WebCore/WebCore.vcproj/WebCore.vcproj b/Source/WebCore/WebCore.vcproj/WebCore.vcproj
>index 1ff7c27b823864596ed30a1640574646f95b0d3b..b0027bf73dd5bb272067b9b5366cceab86cd6393 100755
>--- a/Source/WebCore/WebCore.vcproj/WebCore.vcproj
>+++ b/Source/WebCore/WebCore.vcproj/WebCore.vcproj
>@@ -65878,6 +65878,10 @@
> 					>
> 				</File>
> 				<File
>+					RelativePath="..\bindings\js\JSDependentRetained.h"
>+					>
>+				</File>
>+				<File
> 					RelativePath="..\bindings\js\JSDedicatedWorkerContextCustom.cpp"
> 					>
> 					<FileConfiguration
>diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>index 53ec34847d1621facafe4d146bb89f6d370a7460..c44713036a69bd3f10865d8e978df2809a664261 100644
>--- a/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>+++ b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>@@ -6545,6 +6545,7 @@
> 		FE80DA720E9C472F000D6F75 /* JSPositionError.h in Headers */ = {isa = PBXBuildFile; fileRef = FE80DA6E0E9C472F000D6F75 /* JSPositionError.h */; };
> 		FFD5B97A135CC97800D5E92A /* PageVisibilityState.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FFD5B977135CC97800D5E92A /* PageVisibilityState.cpp */; };
> 		FFD5B97B135CC97800D5E92A /* PageVisibilityState.h in Headers */ = {isa = PBXBuildFile; fileRef = FFD5B978135CC97800D5E92A /* PageVisibilityState.h */; settings = {ATTRIBUTES = (Private, ); }; };
>+		FFD86E7815F9583600047233 /* JSDependentRetained.h in Headers */ = {isa = PBXBuildFile; fileRef = FFD86E7715F9583600047233 /* JSDependentRetained.h */; };
> /* End PBXBuildFile section */
> 
> /* Begin PBXContainerItemProxy section */
>@@ -13971,6 +13972,7 @@
> 		FE80DA6E0E9C472F000D6F75 /* JSPositionError.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSPositionError.h; sourceTree = "<group>"; };
> 		FFD5B977135CC97800D5E92A /* PageVisibilityState.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PageVisibilityState.cpp; sourceTree = "<group>"; };
> 		FFD5B978135CC97800D5E92A /* PageVisibilityState.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PageVisibilityState.h; sourceTree = "<group>"; };
>+		FFD86E7715F9583600047233 /* JSDependentRetained.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSDependentRetained.h; sourceTree = "<group>"; };
> /* End PBXFileReference section */
> 
> /* Begin PBXFrameworksBuildPhase section */
>@@ -20161,6 +20163,7 @@
> 				C585A66111D4FAC5004C3E4B /* IDBBindingUtilities.h */,
> 				1C81BA030E97348300266E07 /* JavaScriptCallFrame.cpp */,
> 				1C81BA040E97348300266E07 /* JavaScriptCallFrame.h */,
>+				FFD86E7715F9583600047233 /* JSDependentRetained.h */,
> 				BCE438A1140C0DBF005E437E /* JSDictionary.cpp */,
> 				BCE4389B140B1BA7005E437E /* JSDictionary.h */,
> 				93B70D4709EB0C7C009D8468 /* JSDOMBinding.cpp */,
>@@ -25500,6 +25503,7 @@
> 				FBD6AF8A15EF25DF008B7110 /* BasicShapeFunctions.h in Headers */,
> 				FBD6AF8C15EF2604008B7110 /* BasicShapes.h in Headers */,
> 				76F2E2EC15F74CEF005FF664 /* RenderDialog.h in Headers */,
>+				FFD86E7815F9583600047233 /* JSDependentRetained.h in Headers */,
> 			);
> 			runOnlyForDeploymentPostprocessing = 0;
> 		};
>diff --git a/Source/WebCore/bindings/js/JSDependentRetained.h b/Source/WebCore/bindings/js/JSDependentRetained.h
>new file mode 100644
>index 0000000000000000000000000000000000000000..fc4e8f31f3e4f6fbf800c2bfbf1a934689624310
>--- /dev/null
>+++ b/Source/WebCore/bindings/js/JSDependentRetained.h
>@@ -0,0 +1,87 @@
>+/*
>+ * Copyright (C) 2012 Google Inc. All Rights Reserved.
>+ * Copyright (C) 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#ifndef JSDependentRetained_h
>+#define JSDependentRetained_h
>+
>+#include <heap/Weak.h>
>+#include <runtime/JSObject.h>
>+#include <runtime/PrivateName.h>
>+#include <wtf/Forward.h>
>+
>+namespace WebCore {
>+
>+class JSDependentRetained {
>+public:
>+    JSDependentRetained(JSC::JSObject* owner, JSC::JSObject* value)
>+        : m_value(value)
>+    {
>+        ASSERT(value);
>+        if (owner)
>+            retain(owner);
>+    }
>+
>+    ~JSDependentRetained()
>+    {
>+        release();
>+    }
>+
>+    JSC::JSObject* get() const
>+    {
>+        return m_value.get();
>+    }
>+
>+    bool isEmpty() const
>+    {
>+        return !m_owner || !m_value;
>+    }
>+
>+    void retain(JSC::JSObject* owner)
>+    {
>+        ASSERT(!m_owner && owner);
>+        ASSERT(m_value);
>+        m_owner = JSC::PassWeak<JSC::JSObject>(owner);
>+        m_owner->putDirect(m_propertyName, get());
>+    }
>+
>+private:
>+
>+    void release()
>+    {
>+        if (m_owner)
>+            m_owner->removeDirect(m_propertyName);
>+        m_value.clear();
>+        m_owner.clear();
>+    }
>+
>+    JSC::Weak<JSC::JSObject> m_owner;
>+    JSC::Weak<JSC::JSObject> m_value;
>+    JSC::PrivateName m_propertyName;
>+};
>+
>+} // namespace WebCore
>+
>+#endif // JSDependentRetained_h
>diff --git a/Source/WebCore/bindings/v8/V8DependentRetained.h b/Source/WebCore/bindings/v8/V8DependentRetained.h
>index 1522ea57a23bc81cbe15ba6df4d047fc34de977e..b2337a12c0d0065fe82d1465547a69e277ca77fe 100644
>--- a/Source/WebCore/bindings/v8/V8DependentRetained.h
>+++ b/Source/WebCore/bindings/v8/V8DependentRetained.h
>@@ -1,29 +1,25 @@
> /*
>- * Copyright (C) 2012 Google Inc. All rights reserved.
>+ * Copyright (C) 2012 Google Inc. All Rights Reserved.
>+ * Copyright (C) 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>- * modification, are permitted provided that the following conditions are
>- * met:
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>  *
>- *     * Redistributions of source code must retain the above copyright
>- * notice, this list of conditions and the following disclaimer.
>- *     * Redistributions in binary form must reproduce the above
>- * copyright notice, this list of conditions and the following disclaimer
>- * in the documentation and/or other materials provided with the
>- * distribution.
>- *     * Neither the name of Google Inc. nor the names of its
>- * contributors may be used to endorse or promote products derived from
>- * this software without specific prior written permission.
>- *
>- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
>- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  */
>@@ -44,15 +40,13 @@ namespace WebCore {
> class V8DependentRetained {
> public:
>     V8DependentRetained(v8::Handle<v8::Object> owner, v8::Handle<v8::Object> value)
>-        : m_owner(owner)
>-        , m_value(value)
>+        : m_value(value)
>         , m_propertyName(createPropertyName())
>     {
>-        ASSERT(!m_owner.IsEmpty());
>         ASSERT(!m_value.IsEmpty());
>-        owner->SetHiddenValue(m_propertyName.get(), value);
>-        m_owner.get().MakeWeak(this, &V8DependentRetained::ownerWeakCallback);
>         m_value.get().MakeWeak(this, &V8DependentRetained::valueWeakCallback);
>+        if (!owner.IsEmpty())
>+            retain(owner);
>     }
> 
>     ~V8DependentRetained()
>@@ -70,9 +64,14 @@ public:
>         return m_owner.isEmpty() || m_value.isEmpty();
>     }
> 
>-    // FIXME: We might add an explicit retain(v8::Handle<v8::Object>) method to allow
>-    // creating one of these and choosing the owner later. Such behavior is required
>-    // in JSC, but not in v8 right now.
>+    void retain(v8::Handle<v8::Object> owner)
>+    {
>+        ASSERT(m_owner.isEmpty() && !owner.IsEmpty());
>+        ASSERT(!m_value.isEmpty());
>+        owner->SetHiddenValue(m_propertyName.get(), get());
>+        m_owner = owner;
>+        m_owner.get().MakeWeak(this, &V8DependentRetained::ownerWeakCallback);
>+    }
> 
> private:
>     static v8::Handle<v8::String> createPropertyName()

Actions: View | Formatted Diff | Diff

Attachments on bug 96034: 162609 | 162621 | 162624 | 162625 | 163441 | 163463