diff options
author | rob <rob@robwu.nl> | 2016-03-18 18:05:01 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-03-19 01:06:14 +0000 |
commit | 43ea0649d4b70fdcf3e9fa5c03aee1bbba0b04bb (patch) | |
tree | a8fbdfa955350a4dd67b87c9253f082303ccb71a | |
parent | b5e56e738b16ea36601bb9281f6f785da583ecae (diff) | |
download | chromium_src-43ea0649d4b70fdcf3e9fa5c03aee1bbba0b04bb.zip chromium_src-43ea0649d4b70fdcf3e9fa5c03aee1bbba0b04bb.tar.gz chromium_src-43ea0649d4b70fdcf3e9fa5c03aee1bbba0b04bb.tar.bz2 |
Deal with frame removal by content scripts
Blink and the RenderFrame implementations are currently not prepared to deal
with frame detachments in their callbacks. Consequently, extension code
(content scripts, chrome.app.window.create) that run arbitrary code in the
"document element created" and "document loaded" notifications may result in
unexpected invalidation of memory, resulting in a UAF.
This patch fixes the bug by moving all code that runs untrusted code from
observers to dedicated callbacks, which are only run at a safe point.
All document parsers in Blink have been modified to make sure that they still
work even when the document creation is interrupted by frame removal.
An extensive set of tests for all different kinds of documents, frame removal
methods (e.g. synchronously / in mutation events / ...) and injection points
(document start/end) have been added to avoid regressions.
BUG=582008
Review URL: https://codereview.chromium.org/1642283002
Cr-Commit-Position: refs/heads/master@{#382162}
62 files changed, 799 insertions, 34 deletions
diff --git a/chrome/browser/extensions/execute_script_apitest.cc b/chrome/browser/extensions/execute_script_apitest.cc index 7f81895..91d19df 100644 --- a/chrome/browser/extensions/execute_script_apitest.cc +++ b/chrome/browser/extensions/execute_script_apitest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/strings/string_number_conversions.h" #include "build/build_config.h" #include "chrome/browser/extensions/extension_apitest.h" #include "net/dns/mock_host_resolver.h" @@ -137,3 +138,80 @@ IN_PROC_BROWSER_TEST_F(ExecuteScriptApiTest, RemovedFrames) { ASSERT_TRUE(StartEmbeddedTestServer()); ASSERT_TRUE(RunExtensionTest("executescript/removed_frames")) << message_; } + +// If tests time out because it takes too long to run them, then this value can +// be increased to split the DestructiveScriptTest tests in approximately equal +// parts. Each part takes approximately the same time to run. +const int kDestructiveScriptTestBucketCount = 1; + +class DestructiveScriptTest : public ExecuteScriptApiTest, + public testing::WithParamInterface<int> { + protected: + // The test extension selects the sub test based on the host name. + bool RunSubtest(const std::string& test_host) { + host_resolver()->AddRule(test_host, "127.0.0.1"); + return RunExtensionSubtest( + "executescript/destructive", + "test.html?" + test_host + + "#bucketcount=" + base::IntToString(kDestructiveScriptTestBucketCount) + + "&bucketindex=" + base::IntToString(GetParam())); + } +}; + +// Removes the frame as soon as the content script is executed. +IN_PROC_BROWSER_TEST_P(DestructiveScriptTest, SynchronousRemoval) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunSubtest("synchronous")) << message_; +} + +// Removes the frame at the frame's first scheduled microtask. +IN_PROC_BROWSER_TEST_P(DestructiveScriptTest, MicrotaskRemoval) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunSubtest("microtask")) << message_; +} + +// Removes the frame at the frame's first scheduled macrotask. +IN_PROC_BROWSER_TEST_P(DestructiveScriptTest, MacrotaskRemoval) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunSubtest("macrotask")) << message_; +} + +// Removes the frame at the first DOMNodeInserted event. +IN_PROC_BROWSER_TEST_P(DestructiveScriptTest, DOMNodeInserted1) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunSubtest("domnodeinserted1")) << message_; +} + +// Removes the frame at the second DOMNodeInserted event. +IN_PROC_BROWSER_TEST_P(DestructiveScriptTest, DOMNodeInserted2) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunSubtest("domnodeinserted2")) << message_; +} + +// Removes the frame at the third DOMNodeInserted event. +IN_PROC_BROWSER_TEST_P(DestructiveScriptTest, DOMNodeInserted3) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunSubtest("domnodeinserted3")) << message_; +} + +// Removes the frame at the first DOMSubtreeModified event. +IN_PROC_BROWSER_TEST_P(DestructiveScriptTest, DOMSubtreeModified1) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunSubtest("domsubtreemodified1")) << message_; +} + +// Removes the frame at the second DOMSubtreeModified event. +IN_PROC_BROWSER_TEST_P(DestructiveScriptTest, DOMSubtreeModified2) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunSubtest("domsubtreemodified2")) << message_; +} + +// Removes the frame at the third DOMSubtreeModified event. +IN_PROC_BROWSER_TEST_P(DestructiveScriptTest, DOMSubtreeModified3) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunSubtest("domsubtreemodified3")) << message_; +} + +INSTANTIATE_TEST_CASE_P(ExecuteScriptApiTest, + DestructiveScriptTest, + ::testing::Range(0, kDestructiveScriptTestBucketCount)); diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc index 5fa4a89..a7d0931 100644 --- a/chrome/renderer/chrome_content_renderer_client.cc +++ b/chrome/renderer/chrome_content_renderer_client.cc @@ -1392,6 +1392,24 @@ void ChromeContentRendererClient::AddImageContextMenuProperties( } } +void ChromeContentRendererClient::RunScriptsAtDocumentStart( + content::RenderFrame* render_frame) { +#if defined(ENABLE_EXTENSIONS) + ChromeExtensionsRendererClient::GetInstance()->RunScriptsAtDocumentStart( + render_frame); + // |render_frame| might be dead by now. +#endif +} + +void ChromeContentRendererClient::RunScriptsAtDocumentEnd( + content::RenderFrame* render_frame) { +#if defined(ENABLE_EXTENSIONS) + ChromeExtensionsRendererClient::GetInstance()->RunScriptsAtDocumentEnd( + render_frame); + // |render_frame| might be dead by now. +#endif +} + void ChromeContentRendererClient::DidInitializeServiceWorkerContextOnWorkerThread( v8::Local<v8::Context> context, diff --git a/chrome/renderer/chrome_content_renderer_client.h b/chrome/renderer/chrome_content_renderer_client.h index 97ee060..653cded 100644 --- a/chrome/renderer/chrome_content_renderer_client.h +++ b/chrome/renderer/chrome_content_renderer_client.h @@ -145,6 +145,8 @@ class ChromeContentRendererClient : public content::ContentRendererClient { void AddImageContextMenuProperties( const blink::WebURLResponse& response, std::map<std::string, std::string>* properties) override; + void RunScriptsAtDocumentStart(content::RenderFrame* render_frame) override; + void RunScriptsAtDocumentEnd(content::RenderFrame* render_frame) override; void DidInitializeServiceWorkerContextOnWorkerThread( v8::Local<v8::Context> context, const GURL& url) override; diff --git a/chrome/renderer/extensions/chrome_extensions_renderer_client.cc b/chrome/renderer/extensions/chrome_extensions_renderer_client.cc index 0f3d463..78e7b2e 100644 --- a/chrome/renderer/extensions/chrome_extensions_renderer_client.cc +++ b/chrome/renderer/extensions/chrome_extensions_renderer_client.cc @@ -304,3 +304,13 @@ ChromeExtensionsRendererClient::CreateBrowserPluginDelegate( return new extensions::MimeHandlerViewContainer(render_frame, mime_type, original_url); } + +void ChromeExtensionsRendererClient::RunScriptsAtDocumentStart( + content::RenderFrame* render_frame) { + extension_dispatcher_->RunScriptsAtDocumentStart(render_frame); +} + +void ChromeExtensionsRendererClient::RunScriptsAtDocumentEnd( + content::RenderFrame* render_frame) { + extension_dispatcher_->RunScriptsAtDocumentEnd(render_frame); +} diff --git a/chrome/renderer/extensions/chrome_extensions_renderer_client.h b/chrome/renderer/extensions/chrome_extensions_renderer_client.h index 2559b82b..801b8fd 100644 --- a/chrome/renderer/extensions/chrome_extensions_renderer_client.h +++ b/chrome/renderer/extensions/chrome_extensions_renderer_client.h @@ -72,6 +72,9 @@ class ChromeExtensionsRendererClient const std::string& mime_type, const GURL& original_url); + void RunScriptsAtDocumentStart(content::RenderFrame* render_frame); + void RunScriptsAtDocumentEnd(content::RenderFrame* render_frame); + extensions::Dispatcher* extension_dispatcher() { return extension_dispatcher_.get(); } diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/about_blank_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/about_blank_frame.html new file mode 100644 index 0000000..d319a3d --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/about_blank_frame.html @@ -0,0 +1 @@ +<iframe src="about:blank"></iframe> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/audio.oga b/chrome/test/data/extensions/api_test/executescript/destructive/audio.oga new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/audio.oga diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/audio.oga.mock-http-headers b/chrome/test/data/extensions/api_test/executescript/destructive/audio.oga.mock-http-headers new file mode 100644 index 0000000..ee58d88 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/audio.oga.mock-http-headers @@ -0,0 +1,4 @@ +HTTP/1.0 200 OK +Content-Type: audio/ogg +Content-Length: 0 +X-Comment: The presence of audio/ogg activates rendering as a media document. diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/audio_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/audio_frame.html new file mode 100644 index 0000000..d37ed80 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/audio_frame.html @@ -0,0 +1 @@ +<iframe src="audio.oga?child"></iframe> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/empty.html b/chrome/test/data/extensions/api_test/executescript/destructive/empty.html new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/empty.html diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/empty_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/empty_frame.html new file mode 100644 index 0000000..feb0d54 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/empty_frame.html @@ -0,0 +1 @@ +<iframe src="empty.html?child"></iframe> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/flag_document_end.js b/chrome/test/data/extensions/api_test/executescript/destructive/flag_document_end.js new file mode 100644 index 0000000..2fa3c80 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/flag_document_end.js @@ -0,0 +1,7 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// remove_self.js runs at both document_start and document_end, so we use this +// script (that only runs at document_end) to distinguish between the two. +window.didRunAtDocumentEnd = true; diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/image.png b/chrome/test/data/extensions/api_test/executescript/destructive/image.png Binary files differnew file mode 100644 index 0000000..709f7c3 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/image.png diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/image_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/image_frame.html new file mode 100644 index 0000000..1663361 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/image_frame.html @@ -0,0 +1 @@ +<iframe src="image.png?child"></iframe> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/manifest.json b/chrome/test/data/extensions/api_test/executescript/destructive/manifest.json new file mode 100644 index 0000000..1bdd34f --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/manifest.json @@ -0,0 +1,33 @@ +{ + "name": "Content scripts that destroy their frame", + "manifest_version": 2, + "version": "1", + "content_scripts": [{ + "run_at": "document_start", + "all_frames": true, + "match_about_blank": true, + "js": ["remove_self.js"], + // URL of the parent frame. + "matches": ["*://*/*.html?blankstart*"] + }, { + "run_at": "document_end", + "all_frames": true, + "match_about_blank": true, + "js": ["flag_document_end.js", "remove_self.js"], + // URL of the parent frame. + "matches": ["*://*/*.html?blankend*"] + }, { + "run_at": "document_start", + "all_frames": true, + "js": ["remove_self.js"], + // 1st URL for the parent frame, second URL for the child frame. + "matches": ["*://*/*.html?start*", "*://*/*?child*"] + }, { + "run_at": "document_end", + "all_frames": true, + "js": ["flag_document_end.js", "remove_self.js"], + // 1st URL for the parent frame, second URL for the child frame. + "matches": ["*://*/*.html?end*", "*://*/*?child*"] + }], + "permissions": ["*://*/*", "webNavigation"] +} diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/no_url_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/no_url_frame.html new file mode 100644 index 0000000..fcb672a --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/no_url_frame.html @@ -0,0 +1 @@ +<iframe></iframe> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/plain.txt b/chrome/test/data/extensions/api_test/executescript/destructive/plain.txt new file mode 100644 index 0000000..8253d6b --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/plain.txt @@ -0,0 +1 @@ +Plain text document diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/plugin_file.pdf b/chrome/test/data/extensions/api_test/executescript/destructive/plugin_file.pdf new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/plugin_file.pdf diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/plugin_file.pdf.mock-http-headers b/chrome/test/data/extensions/api_test/executescript/destructive/plugin_file.pdf.mock-http-headers new file mode 100644 index 0000000..0c12e3f --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/plugin_file.pdf.mock-http-headers @@ -0,0 +1,4 @@ +HTTP/1.0 200 OK +Content-Type: application/pdf +Content-Length: 0 +X-Comment: This resource ought to be rendered as a plugin document. diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/plugin_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/plugin_frame.html new file mode 100644 index 0000000..852e3ee --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/plugin_frame.html @@ -0,0 +1 @@ +<iframe src="plugin_file.pdf?child"></iframe> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/remove_self.js b/chrome/test/data/extensions/api_test/executescript/destructive/remove_self.js new file mode 100644 index 0000000..bf90db4 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/remove_self.js @@ -0,0 +1,93 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +if (top === window) { + testTop(); +} else if (!parent.location.search.includes('end') || + window.didRunAtDocumentEnd) { + testChild(); +} + +function testTop() { + var testMessage = {}; + function reportFrames() { + testMessage.frameCount = frames.length; + testMessage.frameHTML = window.frameHTML; + chrome.runtime.sendMessage(testMessage); + } + + if (window.frameHTML) { // Set by child frame... + // about:blank frames are synchronously parsed, so their document_end script + // injection happens before the main frame's injection. + var expectChildBeforeMain = location.search.includes('?blankend'); + if (!expectChildBeforeMain) { + // Add a message to the test notification to cause the test to fail, + // with some useful information for diagnostics. + testMessage.warning = 'Content script in child frame was executed ' + + 'before the main frame\'s content script!'; + } + reportFrames(); + } else { + window.frameHTML = '(not set)'; + window.onmessage = function(event) { + chrome.test.assertEq('toBeRemoved', event.data); + reportFrames(); + }; + } +} + +function testChild() { + var TEST_HOST = parent.location.hostname; + + if (TEST_HOST === 'synchronous') { + doRemove(); + } else if (TEST_HOST === 'microtask') { + Promise.resolve().then(doRemove); + } else if (TEST_HOST === 'macrotask') { + setTimeout(doRemove, 0); + } else if (TEST_HOST.startsWith('domnodeinserted')) { + removeOnEvent('DOMNodeInserted'); + } else if (TEST_HOST.startsWith('domsubtreemodified')) { + removeOnEvent('DOMSubtreeModified'); + } else { + console.error('Unexpected test: ' + TEST_HOST); + chrome.test.fail(); + } + + function doRemove() { + parent.frameHTML = document.documentElement.outerHTML || '(no outerHTML)'; + parent.postMessage('toBeRemoved', '*'); + // frameElement = <iframe> element in parent document. + frameElement.remove(); + } + + function removeOnEvent(eventName) { + var expected = parseInt(TEST_HOST.match(/\d+/)[0]); + document.addEventListener(eventName, function() { + // Synchronously remove the frame in the mutation event. + if (--expected === 0) + doRemove(); + }); + + // Fallback in case the mutation events are not triggered. + new Promise(function(resolve) { + // The window.onload event signals that the document and its resources + // have finished loading, so we don't expect any other parser-initiated + // DOM mutations after that point. + if (document.readyState === 'complete') + resolve(); + else + window.addEventListener('load', resolve); + }).then(function() { + if (expected > 0) { + expected = 0; + // Print this message to make it clear that the expected condition + // (mutation event |eventName| triggered XXX times) did not happen. + console.log('Mutation condition not triggered: ' + TEST_HOST); + doRemove(); + } + }); + + } +} diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/srcdoc_html_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/srcdoc_html_frame.html new file mode 100644 index 0000000..fb581fe --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/srcdoc_html_frame.html @@ -0,0 +1 @@ +<iframe srcdoc="<br>"></iframe> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/srcdoc_text_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/srcdoc_text_frame.html new file mode 100644 index 0000000..8c66fe8 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/srcdoc_text_frame.html @@ -0,0 +1 @@ +<iframe srcdoc="text"></iframe> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/test.html b/chrome/test/data/extensions/api_test/executescript/destructive/test.html new file mode 100644 index 0000000..46f4d74 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/test.html @@ -0,0 +1 @@ +<script src="test.js"></script> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/test.js b/chrome/test/data/extensions/api_test/executescript/destructive/test.js new file mode 100644 index 0000000..c0d4848 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/test.js @@ -0,0 +1,241 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// This script is run multiple times with different configurations. The content +// scripts need to determine synchronously which subtest is being run. This +// state is communicated via the URL. +var TEST_HOST = location.search.slice(1); +chrome.test.assertTrue( + TEST_HOST !== '', + 'The subtest type must be specified in the query string.'); + +var config; +var tabId; + +function getTestUrl(page) { + return 'http://' + TEST_HOST + ':' + config.testServer.port + + '/extensions/api_test/executescript/destructive/' + page; +} + +chrome.test.getConfig(function(config) { + window.config = config; + + chrome.tabs.create({url: 'about:blank'}, function(tab) { + tabId = tab.id; + startTest(); + }); +}); + +function startTest() { + // These tests load a page that contains a frame, where a content script will + // be run (and remove the frame). The injection point depends on the URL: + // - ?blankstart = Load a script in the blank frame at document_start. + // - ?blankend = Load a script in the blank frame at document_end. + // - ?start = Load a script in the non-blank frame at document_start. + // - ?end = Load a script in the non-blank frame at document_end. + + var kEmptyHtmlBodyPattern = /<body><\/body>/; + + var allTests = [ + // Empty document. + function removeHttpFrameAtDocumentStart() { + testRemoveSelf('empty_frame.html?start'); + }, + + function removeHttpFrameAtDocumentEnd() { + testRemoveSelf('empty_frame.html?end', kEmptyHtmlBodyPattern); + }, + + // about:blank + function removeAboutBlankAtDocumentStart() { + testRemoveSelf('about_blank_frame.html?blankstart'); + }, + + function removeAboutBlankAtDocumentEnd() { + testRemoveSelf('about_blank_frame.html?blankend', kEmptyHtmlBodyPattern); + }, + + // srcdoc frame with a HTML tag + function removeAboutSrcdocHtmlAtDocumentStart() { + testRemoveSelf('srcdoc_html_frame.html?blankstart'); + }, + + function removeAboutSrcdocHtmlAtDocumentEnd() { + testRemoveSelf('srcdoc_html_frame.html?blankend', /<br>/); + }, + + // srcdoc frame with text + function removeAboutSrcdocTextOnlyAtDocumentStart() { + testRemoveSelf('srcdoc_text_frame.html?blankstart'); + }, + + function removeAboutSrcdocTextOnlyAtDocumentEnd() { + testRemoveSelf('srcdoc_text_frame.html?blankend', /text/); + }, + + // <iframe></iframe> (no URL) + function removeFrameWithoutUrlAtDocumentStart() { + testRemoveSelf('no_url_frame.html?blankstart'); + }, + + function removeFrameWithoutUrlAtDocumentEnd() { + testRemoveSelf('no_url_frame.html?blankend', kEmptyHtmlBodyPattern); + }, + + // An image. + function removeImageAtDocumentStart() { + testRemoveSelf('image_frame.html?start'); + }, + + function removeImageAtDocumentEnd() { + testRemoveSelf('image_frame.html?end', /<img/); + }, + + // Audio (media). + function removeAudioAtDocumentStart() { + testRemoveSelf('audio_frame.html?start'); + }, + + function removeAudioAtDocumentEnd() { + testRemoveSelf('audio_frame.html?end', /<video.+ type="audio\//); + }, + + // Video (media). + function removeVideoAtDocumentStart() { + testRemoveSelf('video_frame.html?start'); + }, + + function removeVideoAtDocumentEnd() { + testRemoveSelf('video_frame.html?end', /<video.+ type="video\//); + }, + + // Plugins + function removePluginAtDocumentStart() { + if (maybeSkipPluginTest()) + return; + testRemoveSelf('plugin_frame.html?start'); + }, + + function removePluginAtDocumentEnd() { + if (maybeSkipPluginTest()) + return; + testRemoveSelf('plugin_frame.html?end', /<embed/); + }, + + // Plain text + function removePlainTextAtDocumentStart() { + testRemoveSelf('txt_frame.html?start'); + }, + + function removePlainTextAtDocumentEnd() { + testRemoveSelf('txt_frame.html?end', /<pre/); + }, + + // XHTML + function removeXhtmlAtDocumentStart() { + testRemoveSelf( + 'xhtml_frame.html?start', + /<html xmlns="http:\/\/www\.w3\.org\/1999\/xhtml"><\/html>/); + }, + + function removeXhtmlAtDocumentEnd() { + testRemoveSelf( + 'xhtml_frame.html?end', + /<html xmlns="http:\/\/www\.w3\.org\/1999\/xhtml">/); + }, + + // XML + function removeXmlAtDocumentStart() { + testRemoveSelf('xml_frame.html?start', /<root\/>/); + }, + + function removeXmlAtDocumentEnd() { + testRemoveSelf('xml_frame.html?end', /<root><child\/><\/root>/); + }, + ]; + + // Parameters defined in execute_script_apitest.cc. + var testParams = new URLSearchParams(location.hash.slice(1)); + var kBucketCount = parseInt(testParams.get('bucketcount')); + var kBucketIndex = parseInt(testParams.get('bucketindex')); + var kTestsPerBucket = Math.ceil(allTests.length / kBucketCount); + + chrome.test.assertTrue(kBucketCount * kTestsPerBucket >= allTests.length, + 'To cover all tests, the number of buckets multiplied by the number of ' + + 'tests per bucket must be at least as big as the number of tests.'); + chrome.test.assertTrue(0 <= kBucketIndex >= 0 && kBucketIndex < kBucketCount, + 'There are only ' + kBucketCount + ' buckets, so the bucket index must ' + + 'be between 0 and ' + kBucketCount + ', but it was ' + kBucketIndex); + + // Every run except for the last run contains |kTestsPerBucket| tests. The + // last run (i.e. |kBucketIndex| = |kBucketCount| - 1) may contain fewer tests + // if there are not enough remaining tests, since the total number of tests is + // not necessarily divisible by |kTestsPerBucket|. + var filteredTests = + allTests.slice(kBucketIndex * kTestsPerBucket).slice(0, kTestsPerBucket); + + // At the document_end stage, the parser will not modify the DOM any more, so + // we can skip those tests that wait for DOM mutations to save time. + if (TEST_HOST.startsWith('dom')) { + filteredTests = filteredTests.filter(function(testfn) { + // Omit the *Documentend tests = keep the *DocumentStart tests. + return !testfn.name.endsWith('DocumentEnd'); + }); + } + chrome.test.runTests(filteredTests); +} + +// |page| must be an existing page in this directory, and the URL must be +// matched by one content script. Otherwise the test will time out. +// +// If the regular expression |pattern| is specified, then the serialization of +// the frame content must match the pattern. This ensures that the tests are +// still testing the expected document in the future. +function testRemoveSelf(page, pattern) { + // By default, the serialization of the document must be non-empty. + var kDefaultPattern = /</; + + if (page.includes('start')) { + pattern = pattern || /^<html><\/html>$/; + pattern = TEST_HOST === 'synchronous' ? pattern : kDefaultPattern; + } else if (page.includes('end')) { + pattern = pattern || kDefaultPattern; + } else { + chrome.test.fail('URL must contain "start" or "end": ' + page); + } + + chrome.test.listenOnce(chrome.runtime.onMessage, function(msg, sender) { + chrome.test.assertEq(tabId, sender.tab && sender.tab.id); + chrome.test.assertEq(0, sender.frameId); + + var frameHTML = msg.frameHTML; + delete msg.frameHTML; + chrome.test.assertEq({frameCount: 0}, msg); + + chrome.test.assertTrue( + pattern.test(frameHTML), + 'The pattern ' + pattern + ' should be matched by: ' + frameHTML); + }); + chrome.tabs.update(tabId, {url: getTestUrl(page)}); +} + +// The plugin test requires a plugin to be installed. Skip the test if plugins +// are not supported. +function maybeSkipPluginTest() { + // This MIME-type should be handled by a browser plugin. + var kPluginMimeType = 'application/pdf'; + for (var i = 0; i < navigator.plugins.length; ++i) { + var plugin = navigator.plugins[i]; + for (var j = 0; j < plugin.length; ++j) { + var mimeType = plugin[j]; + if (mimeType.type === kPluginMimeType) + return false; + } + } + var kMessage = 'Plugin not found for ' + kPluginMimeType + ', skipping test.'; + console.log(kMessage); + chrome.test.log(kMessage); + chrome.test.succeed(); + return true; +} diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/txt_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/txt_frame.html new file mode 100644 index 0000000..c16f9b2 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/txt_frame.html @@ -0,0 +1 @@ +<iframe src="plain.txt?child"></iframe> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/video.ogv b/chrome/test/data/extensions/api_test/executescript/destructive/video.ogv new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/video.ogv diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/video.ogv.mock-http-headers b/chrome/test/data/extensions/api_test/executescript/destructive/video.ogv.mock-http-headers new file mode 100644 index 0000000..e8edf86 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/video.ogv.mock-http-headers @@ -0,0 +1,4 @@ +HTTP/1.0 200 OK +Content-Type: video/ogg +Content-Length: 0 +X-Comment: The presence of video/ogg activates rendering as a media document. diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/video_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/video_frame.html new file mode 100644 index 0000000..9ec3d79 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/video_frame.html @@ -0,0 +1 @@ +<iframe src="video.ogv?child"></iframe> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/xhtml_document.xhtml b/chrome/test/data/extensions/api_test/executescript/destructive/xhtml_document.xhtml new file mode 100644 index 0000000..c81cb59 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/xhtml_document.xhtml @@ -0,0 +1,9 @@ +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> +<html xmlns="http://www.w3.org/1999/xhtml"> +<head> + <title>Some valid XHTML document</title> +</head> +<body> + This is a XHTML document. +</body> +</html> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/xhtml_document.xhtml.mock-http-headers b/chrome/test/data/extensions/api_test/executescript/destructive/xhtml_document.xhtml.mock-http-headers new file mode 100644 index 0000000..ca1c4b2 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/xhtml_document.xhtml.mock-http-headers @@ -0,0 +1,2 @@ +HTTP/1.0 200 OK +Content-Type: application/xhtml+xml diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/xhtml_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/xhtml_frame.html new file mode 100644 index 0000000..2f557fa --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/xhtml_frame.html @@ -0,0 +1 @@ +<iframe src="xhtml_document.xhtml?child"></iframe> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/xml_document.xml b/chrome/test/data/extensions/api_test/executescript/destructive/xml_document.xml new file mode 100644 index 0000000..471275d --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/xml_document.xml @@ -0,0 +1 @@ +<?xml version="1.0"?><?xml-stylesheet type="text/css"?><root><child/></root> diff --git a/chrome/test/data/extensions/api_test/executescript/destructive/xml_frame.html b/chrome/test/data/extensions/api_test/executescript/destructive/xml_frame.html new file mode 100644 index 0000000..1c92b95 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/destructive/xml_frame.html @@ -0,0 +1 @@ +<iframe src="xml_document.xml?child"></iframe> diff --git a/content/public/renderer/content_renderer_client.h b/content/public/renderer/content_renderer_client.h index 880550c..b5c05cd 100644 --- a/content/public/renderer/content_renderer_client.h +++ b/content/public/renderer/content_renderer_client.h @@ -14,7 +14,6 @@ #include "base/bind.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/memory/weak_ptr.h" #include "base/strings/string16.h" #include "content/public/common/content_client.h" #include "third_party/WebKit/public/platform/WebPageVisibilityState.h" @@ -312,6 +311,15 @@ class CONTENT_EXPORT ContentRendererClient { const blink::WebURLResponse& response, std::map<std::string, std::string>* properties) {} + // Notifies that a document element has been inserted in the frame's document. + // This may be called multiple times for the same document. This method may + // invalidate the frame. + virtual void RunScriptsAtDocumentStart(RenderFrame* render_frame) {} + + // Notifies that the DOM is ready in the frame's document. + // This method may invalidate the frame. + virtual void RunScriptsAtDocumentEnd(RenderFrame* render_frame) {} + // Notifies that a service worker context has been created. This function // is called from the worker thread. virtual void DidInitializeServiceWorkerContextOnWorkerThread( diff --git a/content/renderer/mojo_bindings_controller.cc b/content/renderer/mojo_bindings_controller.cc index 2214254..84a61b2 100644 --- a/content/renderer/mojo_bindings_controller.cc +++ b/content/renderer/mojo_bindings_controller.cc @@ -71,17 +71,17 @@ void MojoBindingsController::WillReleaseScriptContext( DestroyContextState(context); } -void MojoBindingsController::DidFinishDocumentLoad() { +void MojoBindingsController::RunScriptsAtDocumentStart() { + CreateContextState(); +} + +void MojoBindingsController::RunScriptsAtDocumentReady() { v8::HandleScope handle_scope(blink::mainThreadIsolate()); MojoContextState* state = GetContextState(); if (state) state->Run(); } -void MojoBindingsController::DidCreateDocumentElement() { - CreateContextState(); -} - void MojoBindingsController::DidClearWindowObject() { // NOTE: this function may be called early on twice. From the constructor // mainWorldScriptContext() may trigger this to be called. If we are created diff --git a/content/renderer/mojo_bindings_controller.h b/content/renderer/mojo_bindings_controller.h index 9d4ac9b..d26f17a 100644 --- a/content/renderer/mojo_bindings_controller.h +++ b/content/renderer/mojo_bindings_controller.h @@ -29,6 +29,8 @@ class MojoBindingsController public RenderFrameObserverTracker<MojoBindingsController> { public: MojoBindingsController(RenderFrame* render_frame, bool for_layout_tests); + void RunScriptsAtDocumentStart(); + void RunScriptsAtDocumentReady(); private: ~MojoBindingsController() override; @@ -40,8 +42,6 @@ class MojoBindingsController // RenderFrameObserver overrides: void WillReleaseScriptContext(v8::Local<v8::Context> context, int world_id) override; - void DidFinishDocumentLoad() override; - void DidCreateDocumentElement() override; void DidClearWindowObject() override; const bool for_layout_tests_; diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index f61dce0..a41cdb2 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -3293,6 +3293,23 @@ void RenderFrameImpl::didCreateDocumentElement(blink::WebLocalFrame* frame) { DidCreateDocumentElement(frame)); } +void RenderFrameImpl::runScriptsAtDocumentElementAvailable( + blink::WebLocalFrame* frame) { + DCHECK(!frame_ || frame_ == frame); + base::WeakPtr<RenderFrameImpl> weak_self = weak_factory_.GetWeakPtr(); + + MojoBindingsController* mojo_bindings_controller = + MojoBindingsController::Get(this); + if (mojo_bindings_controller) + mojo_bindings_controller->RunScriptsAtDocumentStart(); + + if (!weak_self.get()) + return; + + GetContentClient()->renderer()->RunScriptsAtDocumentStart(this); + // Do not use |this| or |frame|! ContentClient might have deleted them by now! +} + void RenderFrameImpl::didReceiveTitle(blink::WebLocalFrame* frame, const blink::WebString& title, blink::WebTextDirection direction) { @@ -3368,6 +3385,21 @@ void RenderFrameImpl::didFinishDocumentLoad(blink::WebLocalFrame* frame, } } +void RenderFrameImpl::runScriptsAtDocumentReady(blink::WebLocalFrame* frame) { + base::WeakPtr<RenderFrameImpl> weak_self = weak_factory_.GetWeakPtr(); + + MojoBindingsController* mojo_bindings_controller = + MojoBindingsController::Get(this); + if (mojo_bindings_controller) + mojo_bindings_controller->RunScriptsAtDocumentReady(); + + if (!weak_self.get()) + return; + + GetContentClient()->renderer()->RunScriptsAtDocumentEnd(this); + // Do not use |this| or |frame|! ContentClient might have deleted them by now! +} + void RenderFrameImpl::didHandleOnloadEvents(blink::WebLocalFrame* frame) { DCHECK_EQ(frame_, frame); if (!frame->parent()) { diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h index a21024d..8aa008d 100644 --- a/content/renderer/render_frame_impl.h +++ b/content/renderer/render_frame_impl.h @@ -489,6 +489,8 @@ class CONTENT_EXPORT RenderFrameImpl void didCreateNewDocument(blink::WebLocalFrame* frame) override; void didClearWindowObject(blink::WebLocalFrame* frame) override; void didCreateDocumentElement(blink::WebLocalFrame* frame) override; + void runScriptsAtDocumentElementAvailable( + blink::WebLocalFrame* frame) override; void didReceiveTitle(blink::WebLocalFrame* frame, const blink::WebString& title, blink::WebTextDirection direction) override; @@ -496,6 +498,7 @@ class CONTENT_EXPORT RenderFrameImpl blink::WebIconURL::Type icon_type) override; void didFinishDocumentLoad(blink::WebLocalFrame* frame, bool document_is_empty) override; + void runScriptsAtDocumentReady(blink::WebLocalFrame* frame) override; void didHandleOnloadEvents(blink::WebLocalFrame* frame) override; void didFailLoad(blink::WebLocalFrame* frame, const blink::WebURLError& error, diff --git a/extensions/renderer/dispatcher.cc b/extensions/renderer/dispatcher.cc index 5ff2616e..4bca09c 100644 --- a/extensions/renderer/dispatcher.cc +++ b/extensions/renderer/dispatcher.cc @@ -500,6 +500,24 @@ void Dispatcher::DidCreateDocumentElement(blink::WebLocalFrame* frame) { } } +void Dispatcher::RunScriptsAtDocumentStart(content::RenderFrame* render_frame) { + ExtensionFrameHelper* frame_helper = ExtensionFrameHelper::Get(render_frame); + if (!frame_helper) + return; // The frame is invisible to extensions. + + frame_helper->RunScriptsAtDocumentStart(); + // |frame_helper| and |render_frame| might be dead by now. +} + +void Dispatcher::RunScriptsAtDocumentEnd(content::RenderFrame* render_frame) { + ExtensionFrameHelper* frame_helper = ExtensionFrameHelper::Get(render_frame); + if (!frame_helper) + return; // The frame is invisible to extensions. + + frame_helper->RunScriptsAtDocumentEnd(); + // |frame_helper| and |render_frame| might be dead by now. +} + void Dispatcher::OnExtensionResponse(int request_id, bool success, const base::ListValue& response, diff --git a/extensions/renderer/dispatcher.h b/extensions/renderer/dispatcher.h index 40194a0..eb4372c 100644 --- a/extensions/renderer/dispatcher.h +++ b/extensions/renderer/dispatcher.h @@ -108,8 +108,14 @@ class Dispatcher : public content::RenderProcessObserver, v8::Local<v8::Context> v8_context, const GURL& url); + // This method is not allowed to run JavaScript code in the frame. void DidCreateDocumentElement(blink::WebLocalFrame* frame); + // These methods may run (untrusted) JavaScript code in the frame, and + // cause |render_frame| to become invalid. + void RunScriptsAtDocumentStart(content::RenderFrame* render_frame); + void RunScriptsAtDocumentEnd(content::RenderFrame* render_frame); + void OnExtensionResponse(int request_id, bool success, const base::ListValue& response, diff --git a/extensions/renderer/extension_frame_helper.cc b/extensions/renderer/extension_frame_helper.cc index 300e16e..ba47a7c 100644 --- a/extensions/renderer/extension_frame_helper.cc +++ b/extensions/renderer/extension_frame_helper.cc @@ -58,6 +58,22 @@ bool RenderFrameMatches(const ExtensionFrameHelper* frame_helper, return true; } +// Runs every callback in |callbacks_to_be_run_and_cleared| while |frame_helper| +// is valid, and clears |callbacks_to_be_run_and_cleared|. +void RunCallbacksWhileFrameIsValid( + base::WeakPtr<ExtensionFrameHelper> frame_helper, + std::vector<base::Closure>* callbacks_to_be_run_and_cleared) { + // The JavaScript code can cause re-entrancy. To avoid a deadlock, don't run + // callbacks that are added during the iteration. + std::vector<base::Closure> callbacks; + callbacks_to_be_run_and_cleared->swap(callbacks); + for (auto& callback : callbacks) { + callback.Run(); + if (!frame_helper.get()) + return; // Frame and ExtensionFrameHelper invalidated by callback. + } +} + } // namespace ExtensionFrameHelper::ExtensionFrameHelper(content::RenderFrame* render_frame, @@ -68,7 +84,8 @@ ExtensionFrameHelper::ExtensionFrameHelper(content::RenderFrame* render_frame, tab_id_(-1), browser_window_id_(-1), extension_dispatcher_(extension_dispatcher), - did_create_current_document_element_(false) { + did_create_current_document_element_(false), + weak_ptr_factory_(this) { g_frame_helpers.Get().insert(this); } @@ -123,6 +140,29 @@ void ExtensionFrameHelper::DidCreateNewDocument() { did_create_current_document_element_ = false; } +void ExtensionFrameHelper::RunScriptsAtDocumentStart() { + DCHECK(did_create_current_document_element_); + RunCallbacksWhileFrameIsValid(weak_ptr_factory_.GetWeakPtr(), + &document_element_created_callbacks_); + // |this| might be dead by now. +} + +void ExtensionFrameHelper::RunScriptsAtDocumentEnd() { + RunCallbacksWhileFrameIsValid(weak_ptr_factory_.GetWeakPtr(), + &document_load_finished_callbacks_); + // |this| might be dead by now. +} + +void ExtensionFrameHelper::ScheduleAtDocumentStart( + const base::Closure& callback) { + document_element_created_callbacks_.push_back(callback); +} + +void ExtensionFrameHelper::ScheduleAtDocumentEnd( + const base::Closure& callback) { + document_load_finished_callbacks_.push_back(callback); +} + void ExtensionFrameHelper::DidMatchCSS( const blink::WebVector<blink::WebString>& newly_matching_selectors, const blink::WebVector<blink::WebString>& stopped_matching_selectors) { diff --git a/extensions/renderer/extension_frame_helper.h b/extensions/renderer/extension_frame_helper.h index a710d4f..57ac3b6 100644 --- a/extensions/renderer/extension_frame_helper.h +++ b/extensions/renderer/extension_frame_helper.h @@ -8,6 +8,7 @@ #include <vector> #include "base/macros.h" +#include "base/memory/weak_ptr.h" #include "content/public/common/console_message_level.h" #include "content/public/renderer/render_frame_observer.h" #include "content/public/renderer/render_frame_observer_tracker.h" @@ -61,6 +62,24 @@ class ExtensionFrameHelper return did_create_current_document_element_; } + // Called when the document element has been inserted in this frame. This + // method may invoke untrusted JavaScript code that invalidate the frame and + // this ExtensionFrameHelper. + void RunScriptsAtDocumentStart(); + + // Called after the DOMContentLoaded event has fired. + void RunScriptsAtDocumentEnd(); + + // Schedule a callback, to be run at the next RunScriptsAtDocumentStart + // notification. Only call this when you are certain that there will be such a + // notification, e.g. from RenderFrameObserver::DidCreateDocumentElement. + // Otherwise the callback is never invoked, or invoked for a document that you + // were not expecting. + void ScheduleAtDocumentStart(const base::Closure& callback); + + // Schedule a callback, to be run at the next RunScriptsAtDocumentEnd call. + void ScheduleAtDocumentEnd(const base::Closure& callback); + private: // RenderFrameObserver implementation. void DidCreateDocumentElement() override; @@ -113,6 +132,14 @@ class ExtensionFrameHelper // Whether or not the current document element has been created. bool did_create_current_document_element_; + // Callbacks to be run at the next RunScriptsAtDocumentStart notification. + std::vector<base::Closure> document_element_created_callbacks_; + + // Callbacks to be run at the next RunScriptsAtDocumentEnd notification. + std::vector<base::Closure> document_load_finished_callbacks_; + + base::WeakPtrFactory<ExtensionFrameHelper> weak_ptr_factory_; + DISALLOW_COPY_AND_ASSIGN(ExtensionFrameHelper); }; diff --git a/extensions/renderer/render_frame_observer_natives.cc b/extensions/renderer/render_frame_observer_natives.cc index 1c1f06e8..7c507b3 100644 --- a/extensions/renderer/render_frame_observer_natives.cc +++ b/extensions/renderer/render_frame_observer_natives.cc @@ -24,12 +24,11 @@ class LoadWatcher : public content::RenderFrameObserver { : content::RenderFrameObserver(frame), callback_(callback) {} void DidCreateDocumentElement() override { - // The callback must be run as soon as the root element is available. - // Running the callback may trigger DidCreateDocumentElement or - // DidFailProvisionalLoad, so delete this before running the callback. - base::Callback<void(bool)> callback = callback_; + // Defer the callback instead of running it now to avoid re-entrancy caused + // by the JavaScript callback. + ExtensionFrameHelper::Get(render_frame()) + ->ScheduleAtDocumentStart(base::Bind(callback_, true)); delete this; - callback.Run(true); } void DidFailProvisionalLoad(const blink::WebURLError& error) override { diff --git a/extensions/renderer/script_injection_manager.cc b/extensions/renderer/script_injection_manager.cc index aa0f5d3..0e234d0 100644 --- a/extensions/renderer/script_injection_manager.cc +++ b/extensions/renderer/script_injection_manager.cc @@ -88,6 +88,8 @@ class ScriptInjectionManager::RFOHelper : public content::RenderFrameObserver { // document_idle. void RunIdle(); + void StartInjectScripts(UserScript::RunLocation run_location); + // Indicate that the frame is no longer valid because it is starting // a new load or closing. void InvalidateAndResetFrame(); @@ -136,7 +138,10 @@ void ScriptInjectionManager::RFOHelper::DidCreateNewDocument() { } void ScriptInjectionManager::RFOHelper::DidCreateDocumentElement() { - manager_->StartInjectScripts(render_frame(), UserScript::DOCUMENT_START); + ExtensionFrameHelper::Get(render_frame()) + ->ScheduleAtDocumentStart( + base::Bind(&ScriptInjectionManager::RFOHelper::StartInjectScripts, + weak_factory_.GetWeakPtr(), UserScript::DOCUMENT_START)); } void ScriptInjectionManager::RFOHelper::DidFailProvisionalLoad( @@ -162,7 +167,11 @@ void ScriptInjectionManager::RFOHelper::DidFailProvisionalLoad( void ScriptInjectionManager::RFOHelper::DidFinishDocumentLoad() { DCHECK(content::RenderThread::Get()); - manager_->StartInjectScripts(render_frame(), UserScript::DOCUMENT_END); + ExtensionFrameHelper::Get(render_frame()) + ->ScheduleAtDocumentEnd( + base::Bind(&ScriptInjectionManager::RFOHelper::StartInjectScripts, + weak_factory_.GetWeakPtr(), UserScript::DOCUMENT_END)); + // We try to run idle in two places: here and DidFinishLoad. // DidFinishDocumentLoad() corresponds to completing the document's load, // whereas DidFinishLoad corresponds to completing the document and all @@ -231,6 +240,11 @@ void ScriptInjectionManager::RFOHelper::RunIdle() { } } +void ScriptInjectionManager::RFOHelper::StartInjectScripts( + UserScript::RunLocation run_location) { + manager_->StartInjectScripts(render_frame(), run_location); +} + void ScriptInjectionManager::RFOHelper::InvalidateAndResetFrame() { // Invalidate any pending idle injections, and reset the frame inject on idle. weak_factory_.InvalidateWeakPtrs(); diff --git a/extensions/shell/renderer/shell_content_renderer_client.cc b/extensions/shell/renderer/shell_content_renderer_client.cc index 7dc2aaa..92b7149 100644 --- a/extensions/shell/renderer/shell_content_renderer_client.cc +++ b/extensions/shell/renderer/shell_content_renderer_client.cc @@ -133,6 +133,16 @@ ShellContentRendererClient::CreateBrowserPluginDelegate( } } +void ShellContentRendererClient::RunScriptsAtDocumentStart( + content::RenderFrame* render_frame) { + extension_dispatcher_->RunScriptsAtDocumentStart(render_frame); +} + +void ShellContentRendererClient::RunScriptsAtDocumentEnd( + content::RenderFrame* render_frame) { + extension_dispatcher_->RunScriptsAtDocumentEnd(render_frame); +} + ExtensionsClient* ShellContentRendererClient::CreateExtensionsClient() { return new ShellExtensionsClient; } diff --git a/extensions/shell/renderer/shell_content_renderer_client.h b/extensions/shell/renderer/shell_content_renderer_client.h index a928e10..639da37 100644 --- a/extensions/shell/renderer/shell_content_renderer_client.h +++ b/extensions/shell/renderer/shell_content_renderer_client.h @@ -47,6 +47,8 @@ class ShellContentRendererClient : public content::ContentRendererClient { content::RenderFrame* render_frame, const std::string& mime_type, const GURL& original_url) override; + void RunScriptsAtDocumentStart(content::RenderFrame* render_frame) override; + void RunScriptsAtDocumentEnd(content::RenderFrame* render_frame) override; protected: // app_shell embedders may need custom extensions client interfaces. diff --git a/third_party/WebKit/Source/core/html/ImageDocument.cpp b/third_party/WebKit/Source/core/html/ImageDocument.cpp index 842ceb1..a612769 100644 --- a/third_party/WebKit/Source/core/html/ImageDocument.cpp +++ b/third_party/WebKit/Source/core/html/ImageDocument.cpp @@ -146,9 +146,7 @@ void ImageDocumentParser::appendBytes(const char* data, size_t length) document()->cachedImage()->appendData(data, length); } - // TODO(esprehn): These null checks on Document don't make sense, document() - // will ASSERT if it was null. Do these want to check isDetached() ? - if (document()) + if (!isDetached()) document()->imageUpdated(); } @@ -171,14 +169,14 @@ void ImageDocumentParser::finish() if (fileName.isEmpty()) fileName = document()->url().host(); document()->setTitle(imageTitle(fileName, size)); + if (isDetached()) + return; } document()->imageUpdated(); } - // TODO(esprehn): These null checks on Document don't make sense, document() - // will ASSERT if it was null. Do these want to check isDetached() ? - if (document()) + if (!isDetached()) document()->finishedParsing(); } @@ -207,8 +205,10 @@ void ImageDocument::createDocumentStructure() appendChild(rootElement); rootElement->insertedByParser(); - if (frame()) - frame()->loader().dispatchDocumentElementAvailable(); + frame()->loader().dispatchDocumentElementAvailable(); + frame()->loader().runScriptsAtDocumentElementAvailable(); + if (isStopped()) + return; // runScriptsAtDocumentElementAvailable can detach the frame. RefPtrWillBeRawPtr<HTMLHeadElement> head = HTMLHeadElement::create(*this); RefPtrWillBeRawPtr<HTMLMetaElement> meta = HTMLMetaElement::create(*this); @@ -219,8 +219,7 @@ void ImageDocument::createDocumentStructure() RefPtrWillBeRawPtr<HTMLBodyElement> body = HTMLBodyElement::create(*this); body->setAttribute(styleAttr, "margin: 0px;"); - if (frame()) - frame()->loader().client()->dispatchWillInsertBody(); + frame()->loader().client()->dispatchWillInsertBody(); m_imageElement = HTMLImageElement::create(*this); m_imageElement->setAttribute(styleAttr, "-webkit-user-select: none"); @@ -405,8 +404,13 @@ void ImageDocument::windowSizeChanged(ScaleType type) ImageResource* ImageDocument::cachedImage() { - if (!m_imageElement) + if (!m_imageElement) { createDocumentStructure(); + if (isStopped()) { + m_imageElement = nullptr; + return nullptr; + } + } return m_imageElement->cachedImage(); } diff --git a/third_party/WebKit/Source/core/html/MediaDocument.cpp b/third_party/WebKit/Source/core/html/MediaDocument.cpp index 525d869..3435ebd 100644 --- a/third_party/WebKit/Source/core/html/MediaDocument.cpp +++ b/third_party/WebKit/Source/core/html/MediaDocument.cpp @@ -75,8 +75,10 @@ void MediaDocumentParser::createDocumentStructure() rootElement->insertedByParser(); document()->appendChild(rootElement); - if (document()->frame()) - document()->frame()->loader().dispatchDocumentElementAvailable(); + document()->frame()->loader().dispatchDocumentElementAvailable(); + document()->frame()->loader().runScriptsAtDocumentElementAvailable(); + if (isDetached()) + return; // runScriptsAtDocumentElementAvailable can detach the frame. RefPtrWillBeRawPtr<HTMLHeadElement> head = HTMLHeadElement::create(*document()); RefPtrWillBeRawPtr<HTMLMetaElement> meta = HTMLMetaElement::create(*document()); diff --git a/third_party/WebKit/Source/core/html/PluginDocument.cpp b/third_party/WebKit/Source/core/html/PluginDocument.cpp index 91cc810..4dc2ec5 100644 --- a/third_party/WebKit/Source/core/html/PluginDocument.cpp +++ b/third_party/WebKit/Source/core/html/PluginDocument.cpp @@ -93,10 +93,15 @@ void PluginDocumentParser::createDocumentStructure() rootElement->insertedByParser(); document()->appendChild(rootElement); frame->loader().dispatchDocumentElementAvailable(); + frame->loader().runScriptsAtDocumentElementAvailable(); + if (isStopped()) + return; // runScriptsAtDocumentElementAvailable can detach the frame. RefPtrWillBeRawPtr<HTMLBodyElement> body = HTMLBodyElement::create(*document()); body->setAttribute(styleAttr, "background-color: rgb(38,38,38); height: 100%; width: 100%; overflow: hidden; margin: 0"); rootElement->appendChild(body); + if (isStopped()) + return; // Possibly detached by a mutation event listener installed in runScriptsAtDocumentElementAvailable. m_embedElement = HTMLEmbedElement::create(*document()); m_embedElement->setAttribute(widthAttr, "100%"); @@ -106,6 +111,8 @@ void PluginDocumentParser::createDocumentStructure() m_embedElement->setAttribute(srcAttr, AtomicString(document()->url().getString())); m_embedElement->setAttribute(typeAttr, document()->loader()->mimeType()); body->appendChild(m_embedElement); + if (isStopped()) + return; // Possibly detached by a mutation event listener installed in runScriptsAtDocumentElementAvailable. toPluginDocument(document())->setPluginNode(m_embedElement.get()); @@ -115,8 +122,11 @@ void PluginDocumentParser::createDocumentStructure() // below so flush the layout tasks now instead of waiting on the timer. frame->view()->flushAnyPendingPostLayoutTasks(); // Focus the plugin here, as the line above is where the plugin is created. - if (frame->isMainFrame()) + if (frame->isMainFrame()) { m_embedElement->focus(); + if (isStopped()) + return; // Possibly detached by a focus event listener installed in runScriptsAtDocumentElementAvailable. + } if (PluginView* view = pluginView()) view->didReceiveResponse(document()->loader()->response()); @@ -124,8 +134,11 @@ void PluginDocumentParser::createDocumentStructure() void PluginDocumentParser::appendBytes(const char* data, size_t length) { - if (!m_embedElement) + if (!m_embedElement) { createDocumentStructure(); + if (isStopped()) + return; + } if (!length) return; diff --git a/third_party/WebKit/Source/core/html/parser/HTMLConstructionSite.cpp b/third_party/WebKit/Source/core/html/parser/HTMLConstructionSite.cpp index cc5344d..be72275 100644 --- a/third_party/WebKit/Source/core/html/parser/HTMLConstructionSite.cpp +++ b/third_party/WebKit/Source/core/html/parser/HTMLConstructionSite.cpp @@ -383,8 +383,11 @@ PassRefPtrWillBeRawPtr<HTMLFormElement> HTMLConstructionSite::takeForm() void HTMLConstructionSite::dispatchDocumentElementAvailableIfNeeded() { ASSERT(m_document); - if (m_document->frame() && !m_isParsingFragment) + if (m_document->frame() && !m_isParsingFragment) { m_document->frame()->loader().dispatchDocumentElementAvailable(); + m_document->frame()->loader().runScriptsAtDocumentElementAvailable(); + // runScriptsAtDocumentElementAvailable might have invalidated m_document. + } } void HTMLConstructionSite::insertHTMLHtmlStartTagBeforeHTML(AtomicHTMLToken* token) diff --git a/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp b/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp index 2ce0a69..372389c 100644 --- a/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp +++ b/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp @@ -188,6 +188,11 @@ public: skipLeading<isNotHTMLSpace<UChar>>(); } + void skipRemaining() + { + m_current = m_end; + } + String takeRemaining() { ASSERT(!isEmpty()); @@ -2288,6 +2293,10 @@ ReprocessBuffer: if (buffer.isEmpty()) return; defaultForBeforeHTML(); + if (m_parser->isStopped()) { + buffer.skipRemaining(); + return; + } // Fall through. } case BeforeHeadMode: { diff --git a/third_party/WebKit/Source/core/html/parser/TextDocumentParser.cpp b/third_party/WebKit/Source/core/html/parser/TextDocumentParser.cpp index d6b8d15..bc1c0c3 100644 --- a/third_party/WebKit/Source/core/html/parser/TextDocumentParser.cpp +++ b/third_party/WebKit/Source/core/html/parser/TextDocumentParser.cpp @@ -60,6 +60,8 @@ void TextDocumentParser::insertFakePreElement() attributes.append(Attribute(styleAttr, "word-wrap: break-word; white-space: pre-wrap;")); AtomicHTMLToken fakePre(HTMLToken::StartTag, preTag.localName(), attributes); treeBuilder()->constructTree(&fakePre); + if (isStopped()) + return; // The document could have been detached by an extension while the tree was being constructed. // Normally we would skip the first \n after a <pre> element, but we don't // want to skip the first \n for text documents! diff --git a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp index dc4df7c..8af960e 100644 --- a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp +++ b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp @@ -319,6 +319,9 @@ void DocumentLoader::finishedLoading(double finishTime) commitData(0, 0); } + if (!m_frame) + return; + m_applicationCacheHost->finishedLoadingMainResource(); endWriting(m_writer.get()); if (m_state < MainResourceDone) diff --git a/third_party/WebKit/Source/core/loader/EmptyClients.h b/third_party/WebKit/Source/core/loader/EmptyClients.h index 5b9c4ff..2f0ff6d 100644 --- a/third_party/WebKit/Source/core/loader/EmptyClients.h +++ b/third_party/WebKit/Source/core/loader/EmptyClients.h @@ -255,6 +255,8 @@ public: void didCreateNewDocument() override {} void dispatchDidClearWindowObjectInMainWorld() override {} void documentElementAvailable() override {} + void runScriptsAtDocumentElementAvailable() override {} + void runScriptsAtDocumentReady() override {} void didCreateScriptContext(v8::Local<v8::Context>, int extensionGroup, int worldId) override {} void willReleaseScriptContext(v8::Local<v8::Context>, int worldId) override {} diff --git a/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/third_party/WebKit/Source/core/loader/FrameLoader.cpp index e6d0494..aa26da1 100644 --- a/third_party/WebKit/Source/core/loader/FrameLoader.cpp +++ b/third_party/WebKit/Source/core/loader/FrameLoader.cpp @@ -84,6 +84,7 @@ #include "core/xml/parser/XMLDocumentParser.h" #include "platform/Logging.h" #include "platform/PluginScriptForbiddenScope.h" +#include "platform/ScriptForbiddenScope.h" #include "platform/UserGestureIndicator.h" #include "platform/network/HTTPParsers.h" #include "platform/network/ResourceRequest.h" @@ -507,8 +508,13 @@ void FrameLoader::finishedParsing() m_progressTracker->finishedParsing(); - if (client()) + if (client()) { + ScriptForbiddenScope forbidScripts; client()->dispatchDidFinishDocumentLoad(m_documentLoader ? m_documentLoader->isCommittedButEmpty() : true); + } + + if (client()) + client()->runScriptsAtDocumentReady(); checkCompleted(); @@ -1511,9 +1517,16 @@ bool FrameLoader::shouldTreatURLAsSrcdocDocument(const KURL& url) const void FrameLoader::dispatchDocumentElementAvailable() { + ScriptForbiddenScope forbidScripts; client()->documentElementAvailable(); } +void FrameLoader::runScriptsAtDocumentElementAvailable() +{ + client()->runScriptsAtDocumentElementAvailable(); + // The frame might be detached at this point. +} + void FrameLoader::dispatchDidClearDocumentOfWindowObject() { if (!m_frame->script().canExecuteScripts(NotAboutToExecuteScript)) diff --git a/third_party/WebKit/Source/core/loader/FrameLoader.h b/third_party/WebKit/Source/core/loader/FrameLoader.h index b948aac..2fa071d 100644 --- a/third_party/WebKit/Source/core/loader/FrameLoader.h +++ b/third_party/WebKit/Source/core/loader/FrameLoader.h @@ -136,6 +136,7 @@ public: void dispatchDidClearWindowObjectInMainWorld(); void dispatchDidClearDocumentOfWindowObject(); void dispatchDocumentElementAvailable(); + void runScriptsAtDocumentElementAvailable(); // The following sandbox flags will be forced, regardless of changes to // the sandbox attribute of any parent frames. diff --git a/third_party/WebKit/Source/core/loader/FrameLoaderClient.h b/third_party/WebKit/Source/core/loader/FrameLoaderClient.h index 34c16c7..65f542f 100644 --- a/third_party/WebKit/Source/core/loader/FrameLoaderClient.h +++ b/third_party/WebKit/Source/core/loader/FrameLoaderClient.h @@ -169,6 +169,8 @@ public: virtual void didCreateNewDocument() = 0; virtual void dispatchDidClearWindowObjectInMainWorld() = 0; virtual void documentElementAvailable() = 0; + virtual void runScriptsAtDocumentElementAvailable() = 0; + virtual void runScriptsAtDocumentReady() = 0; virtual v8::Local<v8::Value> createTestInterface(const AtomicString& name) = 0; diff --git a/third_party/WebKit/Source/core/xml/parser/XMLDocumentParser.cpp b/third_party/WebKit/Source/core/xml/parser/XMLDocumentParser.cpp index 9f79b05..c189c33 100644 --- a/third_party/WebKit/Source/core/xml/parser/XMLDocumentParser.cpp +++ b/third_party/WebKit/Source/core/xml/parser/XMLDocumentParser.cpp @@ -1071,8 +1071,11 @@ void XMLDocumentParser::startElementNs(const AtomicString& localName, const Atom if (isHTMLHtmlElement(*newElement)) toHTMLHtmlElement(*newElement).insertedByParser(); - if (!m_parsingFragment && isFirstElement && document()->frame()) + if (!m_parsingFragment && isFirstElement && document()->frame()) { document()->frame()->loader().dispatchDocumentElementAvailable(); + document()->frame()->loader().runScriptsAtDocumentElementAvailable(); + // runScriptsAtDocumentElementAvailable might have invalidated the document. + } } void XMLDocumentParser::endElementNs() diff --git a/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp b/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp index 6a74bb2..1c199ad 100644 --- a/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp +++ b/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp @@ -187,6 +187,20 @@ void FrameLoaderClientImpl::documentElementAvailable() m_webFrame->viewImpl()->mainFrameDocumentElementAvailable(); } +void FrameLoaderClientImpl::runScriptsAtDocumentElementAvailable() +{ + if (m_webFrame->client()) + m_webFrame->client()->runScriptsAtDocumentElementAvailable(m_webFrame); + // The callback might have deleted the frame, do not use |this|! +} + +void FrameLoaderClientImpl::runScriptsAtDocumentReady() +{ + if (m_webFrame->client()) + m_webFrame->client()->runScriptsAtDocumentReady(m_webFrame); + // The callback might have deleted the frame, do not use |this|! +} + void FrameLoaderClientImpl::didCreateScriptContext(v8::Local<v8::Context> context, int extensionGroup, int worldId) { if (m_webFrame->client()) diff --git a/third_party/WebKit/Source/web/FrameLoaderClientImpl.h b/third_party/WebKit/Source/web/FrameLoaderClientImpl.h index 38bab18..4fbff228 100644 --- a/third_party/WebKit/Source/web/FrameLoaderClientImpl.h +++ b/third_party/WebKit/Source/web/FrameLoaderClientImpl.h @@ -60,6 +60,8 @@ public: // parsing begins. void dispatchDidClearWindowObjectInMainWorld() override; void documentElementAvailable() override; + void runScriptsAtDocumentElementAvailable() override; + void runScriptsAtDocumentReady() override; void didCreateScriptContext(v8::Local<v8::Context>, int extensionGroup, int worldId) override; void willReleaseScriptContext(v8::Local<v8::Context>, int worldId) override; diff --git a/third_party/WebKit/public/web/WebFrameClient.h b/third_party/WebKit/public/web/WebFrameClient.h index 2beb12e..575d49a 100644 --- a/third_party/WebKit/public/web/WebFrameClient.h +++ b/third_party/WebKit/public/web/WebFrameClient.h @@ -306,8 +306,13 @@ public: virtual void didClearWindowObject(WebLocalFrame* frame) { } // The document element has been created. + // This method may not invalidate the frame, nor execute JavaScript code. virtual void didCreateDocumentElement(WebLocalFrame*) { } + // Like |didCreateDocumentElement|, except this method may run JavaScript + // code (and possibly invalidate the frame). + virtual void runScriptsAtDocumentElementAvailable(WebLocalFrame*) { } + // The page title is available. virtual void didReceiveTitle(WebLocalFrame* frame, const WebString& title, WebTextDirection direction) { } @@ -315,8 +320,13 @@ public: virtual void didChangeIcon(WebLocalFrame*, WebIconURL::Type) { } // The frame's document finished loading. + // This method may not execute JavaScript code. virtual void didFinishDocumentLoad(WebLocalFrame*, bool documentIsEmpty) { } + // Like |didFinishDocumentLoad|, except this method may run JavaScript + // code (and possibly invalidate the frame). + virtual void runScriptsAtDocumentReady(WebLocalFrame*) { } + // The 'load' event was dispatched. virtual void didHandleOnloadEvents(WebLocalFrame*) { } |