Skip to content
Permalink
Browse files
NPObjectWrapper may not address all window script object lifetime issues
https://bugs.webkit.org/show_bug.cgi?id=85679

Source/WebCore:

The ScriptController implementations force-deallocate the window script object to ensure that DOM objects are not leaked if an NPAPI plugin fails to release a reference to it before being destroyed. The NPObjectWrapper was added to ensure that NPAPI scripting could not touch the real window script object after it had been deallocated, by providing the plugin with a small wrapper which will leak if the plugin fails to dereference it.

This patch removes NPObjectWrapper and instead drops the window script NPObject's reference to the underlying V8Object in ScriptController::clearScriptObjects(). If a plugin fails to dereference the object then the NPV8Object wrapper will be leaked but the DOM objects it references will not.

Patch by James Weatherall <wez@chromium.org> on 2012-06-29
Reviewed by Nate Chapin.

Test: plugins/npruntime/leak-window-scriptable-object.html

* WebCore.gypi:
* bindings/v8/NPObjectWrapper.cpp: Removed.
* bindings/v8/NPObjectWrapper.h: Removed.
* bindings/v8/NPV8Object.cpp:
(WebCore::disposeUnderlyingV8Object):
(WebCore):
(WebCore::freeV8NPObject):
(_NPN_Invoke):
(_NPN_InvokeDefault):
(_NPN_EvaluateHelper):
(_NPN_GetProperty):
(_NPN_SetProperty):
(_NPN_RemoveProperty):
(_NPN_HasProperty):
(_NPN_HasMethod):
(_NPN_Enumerate):
(_NPN_Construct):
* bindings/v8/NPV8Object.h:
(WebCore):
* bindings/v8/ScriptController.cpp:
(WebCore::ScriptController::ScriptController):
(WebCore::ScriptController::clearScriptObjects):
(WebCore::ScriptController::windowScriptNPObject):
* bindings/v8/ScriptController.h:
(ScriptController):

Tools:

TestNetscapePlugin now has a leak-window-scriptable-object test which takes a reference to the window script object, and a second reference to it via the "self" property, and does not release those references. This is used to simulate a leaky plugin in layout tests of the NPAPI scripting interface glue code.

Patch by James Weatherall <wez@chromium.org> on 2012-06-29
Reviewed by Nate Chapin.

* DumpRenderTree/DumpRenderTree.gypi:
* DumpRenderTree/TestNetscapePlugIn/PluginTest.cpp:
(PluginTest::NPN_GetProperty):
* DumpRenderTree/TestNetscapePlugIn/PluginTest.h:
(PluginTest):
* DumpRenderTree/TestNetscapePlugIn/Tests/LeakWindowScriptableObject.cpp: Added.
(LeakWindowScriptableObject):
(LeakWindowScriptableObject::LeakWindowScriptableObject):
(LeakWindowScriptableObject::NPP_New):

LayoutTests:

Test that NPAPI plugins that leak references to the window script object don't cause the containing document to be leaked.

Patch by James Weatherall <wez@chromium.org> on 2012-06-29
Reviewed by Nate Chapin.

* plugins/npruntime/leak-window-scriptable-object-expected.txt: Added.
* plugins/npruntime/leak-window-scriptable-object.html: Added.

Canonical link: https://commits.webkit.org/108175@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@121610 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
James Weatherall authored and webkit-commit-queue committed Jun 30, 2012
1 parent 33fa2f3 commit 6b4539012a568c78e7cd64b62360d23eaa74cda3
Showing 17 changed files with 269 additions and 339 deletions.
@@ -1,3 +1,15 @@
2012-06-29 James Weatherall <wez@chromium.org>

NPObjectWrapper may not address all window script object lifetime issues
https://bugs.webkit.org/show_bug.cgi?id=85679

Test that NPAPI plugins that leak references to the window script object don't cause the containing document to be leaked.

Reviewed by Nate Chapin.

* plugins/npruntime/leak-window-scriptable-object-expected.txt: Added.
* plugins/npruntime/leak-window-scriptable-object.html: Added.

2012-06-29 Emil A Eklund <eae@chromium.org>

Unreviewed chromium windows rebaselines for r121599.
@@ -0,0 +1,11 @@
Test that a plugin that fails to release the window script object doesn't result in the window's document being leaked

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS window.internals.numberOfLiveDocuments() > initialLiveDocumentCount is true
PASS window.internals.numberOfLiveDocuments() == initialLiveDocumentCount is true
PASS successfullyParsed is true

TEST COMPLETE

@@ -0,0 +1,31 @@
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="../../fast/js/resources/js-test-pre.js"></script>
</head>
<body>
<script>
description("Test that a plugin that fails to release the window script object doesn't result in the window's document being leaked");

if (window.internals !== undefined) {
// Test that the live document count returns to the initial value, rather than to a specific value, so this test is robust to leaks in preceding tests.
gc();
var initialLiveDocumentCount = window.internals.numberOfLiveDocuments();

var iframe = document.createElement('iframe');
document.body.appendChild(iframe);
iframe.contentWindow.document.documentElement.innerHTML = '<embed id="plugin" type="application/x-webkit-test-netscape" test="leak-window-scriptable-object"></embed>';

shouldBeTrue("window.internals.numberOfLiveDocuments() > initialLiveDocumentCount");

document.body.removeChild(iframe);

gc();
shouldBeTrue("window.internals.numberOfLiveDocuments() == initialLiveDocumentCount");
}

var successfullyParsed = true;
</script>
<script src="../../fast/js/resources/js-test-post.js"></script>
</body>
</html>
@@ -1,3 +1,42 @@
2012-06-29 James Weatherall <wez@chromium.org>

NPObjectWrapper may not address all window script object lifetime issues
https://bugs.webkit.org/show_bug.cgi?id=85679

The ScriptController implementations force-deallocate the window script object to ensure that DOM objects are not leaked if an NPAPI plugin fails to release a reference to it before being destroyed. The NPObjectWrapper was added to ensure that NPAPI scripting could not touch the real window script object after it had been deallocated, by providing the plugin with a small wrapper which will leak if the plugin fails to dereference it.

This patch removes NPObjectWrapper and instead drops the window script NPObject's reference to the underlying V8Object in ScriptController::clearScriptObjects(). If a plugin fails to dereference the object then the NPV8Object wrapper will be leaked but the DOM objects it references will not.

Reviewed by Nate Chapin.

Test: plugins/npruntime/leak-window-scriptable-object.html

* WebCore.gypi:
* bindings/v8/NPObjectWrapper.cpp: Removed.
* bindings/v8/NPObjectWrapper.h: Removed.
* bindings/v8/NPV8Object.cpp:
(WebCore::disposeUnderlyingV8Object):
(WebCore):
(WebCore::freeV8NPObject):
(_NPN_Invoke):
(_NPN_InvokeDefault):
(_NPN_EvaluateHelper):
(_NPN_GetProperty):
(_NPN_SetProperty):
(_NPN_RemoveProperty):
(_NPN_HasProperty):
(_NPN_HasMethod):
(_NPN_Enumerate):
(_NPN_Construct):
* bindings/v8/NPV8Object.h:
(WebCore):
* bindings/v8/ScriptController.cpp:
(WebCore::ScriptController::ScriptController):
(WebCore::ScriptController::clearScriptObjects):
(WebCore::ScriptController::windowScriptNPObject):
* bindings/v8/ScriptController.h:
(ScriptController):

2012-06-29 Adam Barth <abarth@webkit.org>

Update complex fonts on Android to use fonts from a newer SDK
@@ -2143,8 +2143,6 @@
'bindings/v8/IsolatedWorld.h',
'bindings/v8/JavaScriptCallFrame.cpp',
'bindings/v8/JavaScriptCallFrame.h',
'bindings/v8/NPObjectWrapper.cpp',
'bindings/v8/NPObjectWrapper.h',
'bindings/v8/NPV8Object.cpp',
'bindings/v8/NPV8Object.h',
'bindings/v8/Dictionary.cpp',

This file was deleted.

This file was deleted.

0 comments on commit 6b45390

Please sign in to comment.