diff options
author | asargent@chromium.org <asargent@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-02-24 22:34:06 +0000 |
---|---|---|
committer | asargent@chromium.org <asargent@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-02-24 22:34:06 +0000 |
commit | f6c2459d185424c66e4ef99eed7dc966039715e0 (patch) | |
tree | 6e2ce58cadb57edeb57c8bfa2127191164fb3222 | |
parent | a6e82fccc2b73736c7a99e6293c6dfc04c6957ae (diff) | |
download | chromium_src-f6c2459d185424c66e4ef99eed7dc966039715e0.zip chromium_src-f6c2459d185424c66e4ef99eed7dc966039715e0.tar.gz chromium_src-f6c2459d185424c66e4ef99eed7dc966039715e0.tar.bz2 |
Don't re-run content scripts on fragment navigations.
This regression was inadvertently introduced by a fix to make executeScript
run after a fragment navigation (bug 29541). That patch was:
http://codereview.chromium.org/566041
The problem is that on frame navigations (didChangeLocationWithinPage), we end
up creating a new UserScriptIdleScheduler, so this patch keeps track of the
previous has_run state and propagates that to the new UserScriptIdleScheduler.
BUG=35924
TEST=Steps to verify are outlined in bug report
Review URL: http://codereview.chromium.org/646017
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@39939 0039d316-1c4b-4281-b951-d872f2087c98
12 files changed, 216 insertions, 12 deletions
diff --git a/chrome/browser/extensions/fragment_navigation_apitest.cc b/chrome/browser/extensions/fragment_navigation_apitest.cc new file mode 100644 index 0000000..7524c91 --- /dev/null +++ b/chrome/browser/extensions/fragment_navigation_apitest.cc @@ -0,0 +1,19 @@ +// Copyright (c) 2010 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. + +#include "chrome/browser/extensions/extension_apitest.h" + + +IN_PROC_BROWSER_TEST_F(ExtensionApiTest, ContentScriptFragmentNavigation) { + StartHTTPServer(); + const char* extension_name = "content_scripts/fragment"; + ASSERT_TRUE(RunExtensionTest(extension_name)) << message_; +} + +IN_PROC_BROWSER_TEST_F(ExtensionApiTest, ExecuteScriptFragmentNavigation) { + StartHTTPServer(); + const char* extension_name = "executescript/fragment"; + ASSERT_TRUE(RunExtensionTest(extension_name)) << message_; +} + diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index d377b95f..c1ba1c6 100755 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1161,6 +1161,7 @@ 'browser/extensions/extension_tabs_apitest.cc', 'browser/extensions/extension_toolbar_model_unittest.cc', 'browser/extensions/extension_toolstrip_apitest.cc', + 'browser/extensions/fragment_navigation_apitest.cc', 'browser/extensions/incognito_noscript_apitest.cc', 'browser/extensions/isolated_world_apitest.cc', 'browser/extensions/page_action_apitest.cc', diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc index 860e909..79fa816 100644 --- a/chrome/renderer/render_view.cc +++ b/chrome/renderer/render_view.cc @@ -2681,6 +2681,13 @@ void RenderView::didFinishLoad(WebFrame* frame) { void RenderView::didChangeLocationWithinPage( WebFrame* frame, bool is_new_navigation) { + + // Determine if the UserScriptIdleScheduler already ran scripts on this page, + // since a new one gets created by didCreateDataSource. + NavigationState* state = + NavigationState::FromDataSource(frame->dataSource()); + bool idle_scheduler_ran = state->user_script_idle_scheduler()->has_run(); + // If this was a reference fragment navigation that we initiated, then we // could end up having a non-null pending navigation state. We just need to // update the ExtraData on the datasource so that others who read the @@ -2690,13 +2697,16 @@ void RenderView::didChangeLocationWithinPage( // DidCreateDataSource conveniently takes care of this for us. didCreateDataSource(frame, frame->dataSource()); + if (idle_scheduler_ran) { + // Update the new UserScriptIdleScheduler so we don't re-run scripts. + NavigationState* new_state = + NavigationState::FromDataSource(frame->dataSource()); + new_state->user_script_idle_scheduler()->set_has_run(true); + } + didCommitProvisionalLoad(frame, is_new_navigation); UpdateTitle(frame, frame->view()->mainFrame()->dataSource()->pageTitle()); - - NavigationState* navigation_state = NavigationState::FromDataSource( - frame->dataSource()); - navigation_state->user_script_idle_scheduler()->DidChangeLocationWithinPage(); } void RenderView::didUpdateCurrentHistoryItem(WebFrame* frame) { diff --git a/chrome/renderer/user_script_idle_scheduler.cc b/chrome/renderer/user_script_idle_scheduler.cc index 94efbe7..9336a9d 100644 --- a/chrome/renderer/user_script_idle_scheduler.cc +++ b/chrome/renderer/user_script_idle_scheduler.cc @@ -31,10 +31,6 @@ void UserScriptIdleScheduler::DidFinishLoad() { method_factory_.NewRunnableMethod(&UserScriptIdleScheduler::MaybeRun)); } -void UserScriptIdleScheduler::DidChangeLocationWithinPage() { - DidFinishLoad(); -} - void UserScriptIdleScheduler::Cancel() { view_ = NULL; frame_ = NULL; diff --git a/chrome/renderer/user_script_idle_scheduler.h b/chrome/renderer/user_script_idle_scheduler.h index d4825a5..25b75eb 100644 --- a/chrome/renderer/user_script_idle_scheduler.h +++ b/chrome/renderer/user_script_idle_scheduler.h @@ -22,22 +22,21 @@ class WebFrame; // // The intent of this mechanism is to prevent user scripts from slowing down // fast pages (run after load), while still allowing them to run relatively -// timelily for pages with lots of slow subresources. +// timely for pages with lots of slow subresources. class UserScriptIdleScheduler { public: UserScriptIdleScheduler(RenderView* view, WebKit::WebFrame* frame); bool has_run() { return has_run_; } + void set_has_run(bool has_run) { has_run_ = has_run; } + // Called when the DOM has been completely constructed. void DidFinishDocumentLoad(); // Called when the document has completed loading. void DidFinishLoad(); - // Called when the document has navigated to a fragment. - void DidChangeLocationWithinPage(); - // Called when the client has gone away and we should no longer run scripts. void Cancel(); diff --git a/chrome/test/data/extensions/api_test/content_scripts/fragment/background.html b/chrome/test/data/extensions/api_test/content_scripts/fragment/background.html new file mode 100644 index 0000000..065cf24 --- /dev/null +++ b/chrome/test/data/extensions/api_test/content_scripts/fragment/background.html @@ -0,0 +1,67 @@ +<script> +var failed = false; + +function did_fail() { + return failed; +} + +function set_failed(val) { + failed = val; +} + +function fail() { + set_failed(true); + if (!did_fail()) { + chrome.test.fail(); + } +} + +var test_url = "http://localhost:1337/files/extensions/test_file.html"; + +// For running in normal chrome (ie outside of the browser_tests environment), +// set debug to 1 here. +var debug = 0; +if (debug) { + test_url = "http://www.google.com"; + chrome.test.log = function(msg) { console.log(msg) }; + chrome.test.runTests = function(tests) { + for (var i in tests) { + tests[i](); + } + }; + chrome.test.succeed = function(){ console.log("succeed"); }; + chrome.test.fail = function(){ console.log("fail"); }; +} + +chrome.test.runTests([ + function test1() { + chrome.extension.onRequest.addListener(function(req, sender) { + chrome.test.log("got request: " + JSON.stringify(req)); + if (req == "fail") { + fail(); + } else if (req == "content_script_start") { + var tab = sender.tab; + if (tab.url.indexOf("#") != -1) { + fail(); + } else { + chrome.tabs.update(tab.id, {"url": tab.url + "#foo"}); + } + } + }); + chrome.tabs.onUpdated.addListener(function(tabid, info, tab) { + chrome.test.log("onUpdated status: " + info.status + " url:" + tab.url); + if (info.status == "complete" && tab.url.indexOf("#foo") != -1) { + setTimeout(function() { + if (!did_fail()) { + chrome.test.succeed(); + } + }, 750); + } + }); + chrome.test.log("creating tab"); + chrome.tabs.create({"url": test_url}); + } +]); + + +</script> diff --git a/chrome/test/data/extensions/api_test/content_scripts/fragment/content_script.js b/chrome/test/data/extensions/api_test/content_scripts/fragment/content_script.js new file mode 100644 index 0000000..adf73d2 --- /dev/null +++ b/chrome/test/data/extensions/api_test/content_scripts/fragment/content_script.js @@ -0,0 +1,14 @@ +// Copyright (c) 2010 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. + +// Tests that we don't re-inject scripts after fragment navigation. + +// The background page should only see this once - it will then use tab.update +// to navigate this page to #foo. +chrome.extension.sendRequest("content_script_start"); + +if (location.href.indexOf("#foo") != -1) { + // This means the content script ran again. + chrome.extension.sendRequest("fail"); +} diff --git a/chrome/test/data/extensions/api_test/content_scripts/fragment/manifest.json b/chrome/test/data/extensions/api_test/content_scripts/fragment/manifest.json new file mode 100644 index 0000000..c2328f6 --- /dev/null +++ b/chrome/test/data/extensions/api_test/content_scripts/fragment/manifest.json @@ -0,0 +1,10 @@ +{ + "name": "content_scripts_fragment_navigation", + "version": "1.0", + "description": "Tests content scripts running with navigation to fragments within a page", + "background_page": "background.html", + "permissions": ["tabs"], + "content_scripts": [ + { "matches": ["http://*/*"], "js": ["content_script.js"] } + ] +} diff --git a/chrome/test/data/extensions/api_test/executescript/fragment/background.html b/chrome/test/data/extensions/api_test/executescript/fragment/background.html new file mode 100644 index 0000000..673a56f --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/fragment/background.html @@ -0,0 +1,59 @@ +<script> +var got_request = false; + +var test_url = "http://localhost:1337/files/extensions/test_file.html"; + +// For running in normal chrome (ie outside of the browser_tests environment), +// set debug to 1 here. +var debug = 0; +if (debug) { + test_url = "http://www.google.com"; + chrome.test.log = function(msg) { console.log(msg) }; + chrome.test.runTests = function(tests) { + for (var i in tests) { + tests[i](); + } + }; + chrome.test.succeed = function(){ console.log("succeed"); }; + chrome.test.fail = function(){ console.log("fail"); }; +} + +function navigate_to_fragment(tab, callback) { + var new_url = test_url + "#foo"; + chrome.test.log("navigating tab to " + new_url); + chrome.tabs.update(tab.id, {"url": new_url}, callback); +} + +var succeeded = false; + +function do_execute(tab) { + chrome.tabs.executeScript(tab.id, {"file": "execute_script.js"}); + setTimeout(function() { + if (!succeeded) { + chrome.test.fail("timed out"); + } + }, 10000); +} + + +chrome.test.runTests([ + // When the tab is created, a content script will send a request letting + // know the onload has fired. Then we navigate to a fragment, and try + // running chrome.tabs.executeScript. + function test1() { + chrome.extension.onRequest.addListener(function(req, sender) { + chrome.test.log("got request: " + JSON.stringify(req)); + if (req == "content_script") { + navigate_to_fragment(sender.tab, do_execute); + } else if (req == "execute_script") { + suceeded = true; + chrome.test.succeed(); + } + }); + chrome.test.log("creating tab"); + chrome.tabs.create({"url": test_url}); + } +]); + + +</script> diff --git a/chrome/test/data/extensions/api_test/executescript/fragment/content_script.js b/chrome/test/data/extensions/api_test/executescript/fragment/content_script.js new file mode 100644 index 0000000..9f44deb --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/fragment/content_script.js @@ -0,0 +1,13 @@ +// Copyright (c) 2010 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. + +function notify() { + chrome.extension.sendRequest("content_script"); +} + +if (document.readyState) { + notify(); +} else { + document.onload = notify; +} diff --git a/chrome/test/data/extensions/api_test/executescript/fragment/execute_script.js b/chrome/test/data/extensions/api_test/executescript/fragment/execute_script.js new file mode 100644 index 0000000..dc6b7ba --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/fragment/execute_script.js @@ -0,0 +1,6 @@ +// Copyright (c) 2010 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. + +// Notify the background page that we ran. +chrome.extension.sendRequest("execute_script"); diff --git a/chrome/test/data/extensions/api_test/executescript/fragment/manifest.json b/chrome/test/data/extensions/api_test/executescript/fragment/manifest.json new file mode 100644 index 0000000..91c6e68 --- /dev/null +++ b/chrome/test/data/extensions/api_test/executescript/fragment/manifest.json @@ -0,0 +1,10 @@ +{ + "name": "execute_script_fragment_navigation", + "version": "1.0", + "description": "Tests execute script running after navigation to a fragments within a page", + "background_page": "background.html", + "permissions": ["tabs", "http://*/*"], + "content_scripts": [ + {"matches": ["http://*/*"], "js": ["content_script.js"]} + ] +} |