Title: [135874] trunk
- Revision
- 135874
- Author
- [email protected]
- Date
- 2012-11-27 10:14:08 -0800 (Tue, 27 Nov 2012)
Log Message
freeV8NPObject: Skip trying to remove object from per context data's ObjectMap if the object does not exist in the map.
https://bugs.webkit.org/show_bug.cgi?id=103356
Patch by Istiaque Ahmed <[email protected]> on 2012-11-27
Reviewed by Adam Barth.
Before introducing per context data, it used to be the case that we would ignore removing v8 object
from context data's map if we couldn't find it. With perContextData change, this turned into an ASSERT
instead and we had crash reports.
Related crbug.com/154462.
Source/WebCore:
Test: platform/chromium/plugins/release-frame-content-window.html
* bindings/v8/NPV8Object.cpp:
(WebCore::freeV8NPObject):
LayoutTests:
* platform/chromium/plugins/release-frame-content-window-expected.txt: Added.
* platform/chromium/plugins/release-frame-content-window.html: Added.
* platform/chromium/plugins/resources/empty.html: Added.
* platform/chromium/plugins/resources/empty2.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (135873 => 135874)
--- trunk/LayoutTests/ChangeLog 2012-11-27 18:11:29 UTC (rev 135873)
+++ trunk/LayoutTests/ChangeLog 2012-11-27 18:14:08 UTC (rev 135874)
@@ -1,3 +1,20 @@
+2012-11-27 Istiaque Ahmed <[email protected]>
+
+ freeV8NPObject: Skip trying to remove object from per context data's ObjectMap if the object does not exist in the map.
+ https://bugs.webkit.org/show_bug.cgi?id=103356
+
+ Reviewed by Adam Barth.
+
+ Before introducing per context data, it used to be the case that we would ignore removing v8 object
+ from context data's map if we couldn't find it. With perContextData change, this turned into an ASSERT
+ instead and we had crash reports.
+ Related crbug.com/154462.
+
+ * platform/chromium/plugins/release-frame-content-window-expected.txt: Added.
+ * platform/chromium/plugins/release-frame-content-window.html: Added.
+ * platform/chromium/plugins/resources/empty.html: Added.
+ * platform/chromium/plugins/resources/empty2.html: Added.
+
2012-11-27 Justin Novosad <[email protected]>
Skipping fast/backgrounds/background-opaque-images-over-color.html on mac
Added: trunk/LayoutTests/platform/chromium/plugins/release-frame-content-window-expected.txt (0 => 135874)
--- trunk/LayoutTests/platform/chromium/plugins/release-frame-content-window-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/chromium/plugins/release-frame-content-window-expected.txt 2012-11-27 18:14:08 UTC (rev 135874)
@@ -0,0 +1,5 @@
+
+
+
+This tests that releasing a reference to a contentWindow that has already been navigated away works correctly and does not crash. If this test is successful, the word SUCCESS should be seen below.
+SUCCESS
Added: trunk/LayoutTests/platform/chromium/plugins/release-frame-content-window.html (0 => 135874)
--- trunk/LayoutTests/platform/chromium/plugins/release-frame-content-window.html (rev 0)
+++ trunk/LayoutTests/platform/chromium/plugins/release-frame-content-window.html 2012-11-27 18:14:08 UTC (rev 135874)
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<script type="text/_javascript_">
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+
+
+function runTest() {
+ var p = document.getElementById('plg');
+ var iframe = document.getElementById('frame-id');
+ p.remember(iframe.contentWindow);
+
+ var someObject = {a: 1, b: 2};
+ // Load another page in iframe, so plugin keeps reference to old frame's contentWindow.
+ iframe._onload_ = function() {
+ // This will trigger Release Object for previously remembered reference.
+ p.remember(someObject);
+ document.getElementById('result').innerHTML = 'SUCCESS';
+ if (window.testRunner)
+ testRunner.notifyDone();
+ };
+ iframe.src = '';
+}
+
+</script>
+<body>
+<embed id="plg" type="application/x-webkit-test-netscape"></embed><P>
+<iframe id="frame-id" src="" _onload_='runTest()'></iframe>
+<div>This tests that releasing a reference to a contentWindow that has already been navigated away works correctly and does not crash. If this test is successful, the word SUCCESS should be seen below.</div>
+<div id="result">FAILED</div>
+</body>
+</html>
+
Added: trunk/LayoutTests/platform/chromium/plugins/resources/empty.html (0 => 135874)
--- trunk/LayoutTests/platform/chromium/plugins/resources/empty.html (rev 0)
+++ trunk/LayoutTests/platform/chromium/plugins/resources/empty.html 2012-11-27 18:14:08 UTC (rev 135874)
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+</body>
+</html>
Added: trunk/LayoutTests/platform/chromium/plugins/resources/empty2.html (0 => 135874)
--- trunk/LayoutTests/platform/chromium/plugins/resources/empty2.html (rev 0)
+++ trunk/LayoutTests/platform/chromium/plugins/resources/empty2.html 2012-11-27 18:14:08 UTC (rev 135874)
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (135873 => 135874)
--- trunk/Source/WebCore/ChangeLog 2012-11-27 18:11:29 UTC (rev 135873)
+++ trunk/Source/WebCore/ChangeLog 2012-11-27 18:14:08 UTC (rev 135874)
@@ -1,3 +1,20 @@
+2012-11-27 Istiaque Ahmed <[email protected]>
+
+ freeV8NPObject: Skip trying to remove object from per context data's ObjectMap if the object does not exist in the map.
+ https://bugs.webkit.org/show_bug.cgi?id=103356
+
+ Reviewed by Adam Barth.
+
+ Before introducing per context data, it used to be the case that we would ignore removing v8 object
+ from context data's map if we couldn't find it. With perContextData change, this turned into an ASSERT
+ instead and we had crash reports.
+ Related crbug.com/154462.
+
+ Test: platform/chromium/plugins/release-frame-content-window.html
+
+ * bindings/v8/NPV8Object.cpp:
+ (WebCore::freeV8NPObject):
+
2012-11-27 Marja Hölttä <[email protected]>
Add callbacks to the FrameLoaderClient when a resource is requested
Modified: trunk/Source/WebCore/bindings/v8/NPV8Object.cpp (135873 => 135874)
--- trunk/Source/WebCore/bindings/v8/NPV8Object.cpp 2012-11-27 18:11:29 UTC (rev 135873)
+++ trunk/Source/WebCore/bindings/v8/NPV8Object.cpp 2012-11-27 18:14:08 UTC (rev 135874)
@@ -80,16 +80,17 @@
int v8ObjectHash = v8NpObject->v8Object->GetIdentityHash();
ASSERT(v8ObjectHash);
V8NPObjectMap::iterator iter = v8NPObjectMap->find(v8ObjectHash);
- ASSERT(iter != v8NPObjectMap->end());
- V8NPObjectVector& objects = iter->value;
- for (size_t index = 0; index < objects.size(); ++index) {
- if (objects.at(index) == v8NpObject) {
- objects.remove(index);
- break;
+ if (iter != v8NPObjectMap->end()) {
+ V8NPObjectVector& objects = iter->value;
+ for (size_t index = 0; index < objects.size(); ++index) {
+ if (objects.at(index) == v8NpObject) {
+ objects.remove(index);
+ break;
+ }
}
+ if (objects.isEmpty())
+ v8NPObjectMap->remove(v8ObjectHash);
}
- if (objects.isEmpty())
- v8NPObjectMap->remove(v8ObjectHash);
}
v8NpObject->v8Object.Dispose();
v8NpObject->v8Object.Clear();
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
