From 5979cf5a11ec1665217f115749ab1b6dd7394eff Mon Sep 17 00:00:00 2001 From: "erikkay@chromium.org" <erikkay@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> Date: Tue, 15 Sep 2009 21:22:56 +0000 Subject: [PATCH] Handle mole/toolstrip URLs properly. * expand/collapse to chrome-extension://crashme no longer crashes * expand/collapse to a relative URL now works BUG=20412,21905 TEST=browser_tests ExtensionApi.Toolstrip (note that the test doesn't actually exercise these changes due to 21905) Review URL: http://codereview.chromium.org/195093 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@26266 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/extensions/extension_host.cc | 30 +++++++++++++++++-- .../extensions/extension_toolstrip_api.cc | 9 +++--- .../api_test/toolstrip/folder/relative.html | 8 +++++ .../api_test/toolstrip/manifest.json | 2 +- .../extensions/api_test/toolstrip/test.html | 30 ++++++++++++++++++- .../extensions/api_test/toolstrip/test2.html | 9 ++++++ 6 files changed, 79 insertions(+), 9 deletions(-) create mode 100755 chrome/test/data/extensions/api_test/toolstrip/folder/relative.html create mode 100755 chrome/test/data/extensions/api_test/toolstrip/test2.html diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc index 2a19667dc2830..0bafeecf7c4d6 100644 --- a/chrome/browser/extensions/extension_host.cc +++ b/chrome/browser/extensions/extension_host.cc @@ -101,6 +101,15 @@ void ExtensionHost::CreateRenderView(RenderWidgetHostView* host_view) { } void ExtensionHost::NavigateToURL(const GURL& url) { + // Prevent explicit navigation to another extension id's pages. + // This method is only called by some APIs, so we still need to protect + // DidNavigate below (location = ""). + if (url.SchemeIs(chrome::kExtensionScheme) && + url.host() != extension_->id()) { + // TODO(erikkay) communicate this back to the caller? + return; + } + url_ = url; if (!is_background_page() && !extension_->GetBackgroundPageReady()) { @@ -144,11 +153,28 @@ void ExtensionHost::DidNavigate(RenderViewHost* render_view_host, return; } - url_ = params.url; - if (!url_.SchemeIs(chrome::kExtensionScheme)) { + if (!params.url.SchemeIs(chrome::kExtensionScheme)) { + extension_function_dispatcher_.reset(NULL); + url_ = params.url; + return; + } + + // This catches two bogus use cases: + // (1) URLs that look like chrome-extension://somethingbogus or + // chrome-extension://nosuchid/, in other words, no Extension would + // be found. + // (2) URLs that refer to a different extension than this one. + // In both cases, we preserve the old URL and reset the EFD to NULL. This + // will leave the host in kind of a bad state with poor UI and errors, but + // it's better than the alternative. + // TODO(erikkay) Perhaps we should display log errors or display a big 404 + // in the toolstrip or something like that. + if (params.url.host() != extension_->id()) { extension_function_dispatcher_.reset(NULL); return; } + + url_ = params.url; extension_function_dispatcher_.reset( new ExtensionFunctionDispatcher(render_view_host_, this, url_)); } diff --git a/chrome/browser/extensions/extension_toolstrip_api.cc b/chrome/browser/extensions/extension_toolstrip_api.cc index a502dcaa7196b..829d44357bff2 100644 --- a/chrome/browser/extensions/extension_toolstrip_api.cc +++ b/chrome/browser/extensions/extension_toolstrip_api.cc @@ -82,14 +82,13 @@ bool ToolstripExpandFunction::RunImpl() { return false; } - GURL url; if (args->HasKey(keys::kUrlKey)) { std::string url_string; EXTENSION_FUNCTION_VALIDATE(args->GetString(keys::kUrlKey, &url_string)); - url = GURL(url_string); - if (!url.is_valid() && !url.is_empty()) { + url = dispatcher()->url().Resolve(url_string); + if (!url.is_valid()) { error_ = kInvalidURLError; return false; } @@ -117,8 +116,8 @@ bool ToolstripCollapseFunction::RunImpl() { std::string url_string; EXTENSION_FUNCTION_VALIDATE(args->GetString(keys::kUrlKey, &url_string)); - url = GURL(url_string); - if (!url.is_valid() && !url.is_empty()) { + url = dispatcher()->url().Resolve(url_string); + if (!url.is_valid()) { error_ = kInvalidURLError; return false; } diff --git a/chrome/test/data/extensions/api_test/toolstrip/folder/relative.html b/chrome/test/data/extensions/api_test/toolstrip/folder/relative.html new file mode 100755 index 0000000000000..9be5b62f1c34e --- /dev/null +++ b/chrome/test/data/extensions/api_test/toolstrip/folder/relative.html @@ -0,0 +1,8 @@ +<script> +/* +chrome.toolstrip.onExpanded.addListener(function() { + chrome.test.log("relative expanded"); +});*/ +</script> +relative + diff --git a/chrome/test/data/extensions/api_test/toolstrip/manifest.json b/chrome/test/data/extensions/api_test/toolstrip/manifest.json index 36ed6e883822b..278ffbf05b2f1 100755 --- a/chrome/test/data/extensions/api_test/toolstrip/manifest.json +++ b/chrome/test/data/extensions/api_test/toolstrip/manifest.json @@ -2,5 +2,5 @@ "name": "chrome.toolstrip", "version": "0.1", "description": "end-to-end browser test for chrome.toolstrip API", - "toolstrips": ["test.html"] + "toolstrips": ["test.html", "test2.html"] } diff --git a/chrome/test/data/extensions/api_test/toolstrip/test.html b/chrome/test/data/extensions/api_test/toolstrip/test.html index 237612dfba710..985e751c78ff5 100755 --- a/chrome/test/data/extensions/api_test/toolstrip/test.html +++ b/chrome/test/data/extensions/api_test/toolstrip/test.html @@ -9,6 +9,34 @@ chrome.test.runTests([ function collapse() { chrome.test.listenOnce(chrome.toolstrip.onCollapsed, function(){}); chrome.toolstrip.collapse({}, chrome.test.callbackPass(function(){})); - } + }, + + /* TODO(mpcomplete): this is failing in part due to bug 21905 + function expandOther() { + var orig = window.location; + var relative = "folder/relative.html"; + + // Get the other toolstrip. + var toolstrips = chrome.extension.getToolstrips(); + var other = (toolstrips[0].location == location) ? + toolstrips[1] : toolstrips[0]; + + other.chrome.toolstrip.expand({height:200, url:relative}, + chrome.test.callbackPass(function() { + // TODO(mpcomplete) this never gets called + chrome.test.log(other.location.toString()); + chrome.test.listenOnce(other.onLoad, function() { + chrome.test.log(other.location.toString()); + chrome.test.assertEq(other.location.toString(), + chrome.extension.getURL(relative)); + other.chrome.toolstrip.collapse({url:"../test2.html"}, + chrome.test.callbackPass(function() { + chrome.test.assertEq(other.location.toString(), orig); + })); + }); + })); + }, + */ ]); </script> +test \ No newline at end of file diff --git a/chrome/test/data/extensions/api_test/toolstrip/test2.html b/chrome/test/data/extensions/api_test/toolstrip/test2.html new file mode 100755 index 0000000000000..957f4e443e8e9 --- /dev/null +++ b/chrome/test/data/extensions/api_test/toolstrip/test2.html @@ -0,0 +1,9 @@ +<script> +chrome.toolstrip.onExpanded.addListener(function() { + chrome.test.log("test2 expanded"); +}); +chrome.toolstrip.onCollapsed.addListener(function() { + chrome.test.log("test2 collapsed"); +}); +</script> +test2 -- GitLab