[Extensions] chrome.tabs.update() no longer supports JavaScript URLs.

Bug: 827288
Change-Id: Ic500608906e7c7c428e3042c5ffcf29f31899beb
Reviewed-on: https://chromium-review.googlesource.com/1207576
Commit-Queue: David Bertoni <dbertoni@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594805}

chrome/browser/extensions/api/tabs/tabs_api.cc

@@ -1271,7 +1271,6 @@ ExtensionFunction::ResponseAction TabsUpdateFunction::Run() {
   // -favIconUrl
 
   // Navigate the tab to a new location if the url is different.
-  bool is_async = false;
   if (params->update_properties.url.get()) {
     std::string updated_url = *params->update_properties.url;
     if (browser->profile()->GetProfileType() == Profile::INCOGNITO_PROFILE &&
@@ -1279,7 +1278,7 @@ ExtensionFunction::ResponseAction TabsUpdateFunction::Run() {
       return RespondNow(Error(ErrorUtils::FormatErrorMessage(
           tabs_constants::kURLsNotAllowedInIncognitoError, updated_url)));
     }
-    if (!UpdateURL(updated_url, tab_id, &is_async, &error))
+    if (!UpdateURL(updated_url, tab_id, &error))
       return RespondNow(Error(error));
   }
 
@@ -1360,15 +1359,11 @@ ExtensionFunction::ResponseAction TabsUpdateFunction::Run() {
         ->SetAutoDiscardable(state);
   }
 
-  if (!is_async)
-    return RespondNow(GetResult());
-
-  return RespondLater();
+  return RespondNow(GetResult());
 }
 
 bool TabsUpdateFunction::UpdateURL(const std::string& url_string,
                                    int tab_id,
-                                   bool* is_async,
                                    std::string* error) {
   GURL url =
       ExtensionTabUtil::ResolvePossiblyRelativeURL(url_string, extension());
@@ -1388,32 +1383,10 @@ bool TabsUpdateFunction::UpdateURL(const std::string& url_string,
   const bool is_javascript_scheme = url.SchemeIs(url::kJavaScriptScheme);
   UMA_HISTOGRAM_BOOLEAN("Extensions.ApiTabUpdateJavascript",
                         is_javascript_scheme);
-  // JavaScript URLs can do the same kinds of things as cross-origin XHR, so
-  // we need to check host permissions before allowing them.
+  // JavaScript URLs are forbidden in chrome.tabs.update().
   if (is_javascript_scheme) {
-    if (!extension()->permissions_data()->CanAccessPage(web_contents_->GetURL(),
-                                                        tab_id, error)) {
-      return false;
-    }
-
-    TabHelper::FromWebContents(web_contents_)
-        ->script_executor()
-        ->ExecuteScript(
-            HostID(HostID::EXTENSIONS, extension_id()),
-            ScriptExecutor::JAVASCRIPT,
-            net::UnescapeURLComponent(
-                url.GetContent(),
-                net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-                    net::UnescapeRule::PATH_SEPARATORS |
-                    net::UnescapeRule::SPACES),
-            ScriptExecutor::SINGLE_FRAME, ExtensionApiFrameIdMap::kTopFrameId,
-            ScriptExecutor::DONT_MATCH_ABOUT_BLANK, UserScript::DOCUMENT_IDLE,
-            ScriptExecutor::MAIN_WORLD, ScriptExecutor::DEFAULT_PROCESS, GURL(),
-            GURL(), user_gesture(), base::nullopt, ScriptExecutor::NO_RESULT,
-            base::Bind(&TabsUpdateFunction::OnExecuteCodeFinished, this));
-
-    *is_async = true;
-    return true;
+    *error = tabs_constants::kJavaScriptUrlsNotAllowedInTabsUpdate;
+    return false;
   }
 
   bool use_renderer_initiated = false;
@@ -1427,14 +1400,8 @@ bool TabsUpdateFunction::UpdateURL(const std::string& url_string,
   load_params.is_renderer_initiated = use_renderer_initiated;
   web_contents_->GetController().LoadURLWithParams(load_params);
 
-  // The URL of a tab contents never actually changes to a JavaScript URL, so
-  // this check only makes sense in other cases.
-  if (!url.SchemeIs(url::kJavaScriptScheme)) {
-    // The URL should be present in the pending entry, though it may not be
-    // visible in the omnibox until it commits.
-    DCHECK_EQ(
-        url, web_contents_->GetController().GetPendingEntry()->GetVirtualURL());
-  }
+  DCHECK_EQ(url,
+            web_contents_->GetController().GetPendingEntry()->GetVirtualURL());
 
   return true;
 }

chrome/browser/extensions/api/tabs/tabs_api.h

@@ -145,7 +145,6 @@ class TabsUpdateFunction : public UIThreadExtensionFunction {
   ~TabsUpdateFunction() override {}
   bool UpdateURL(const std::string& url,
                  int tab_id,
-                 bool* is_async,
                  std::string* error);
   ResponseValue GetResult();
 

chrome/browser/extensions/api/tabs/tabs_api_unittest.cc

@@ -315,4 +315,45 @@ TEST_F(TabsApiUnitTest, ExecuteScriptNoTabIsNonFatalError) {
   EXPECT_EQ(tabs_constants::kNoTabInBrowserWindowError, error);
 }
 
+// Tests that calling chrome.tabs.update with a JavaScript URL results
+// in an error.
+TEST_F(TabsApiUnitTest, TabsUpdateJavaScriptUrlNotAllowed) {
+  // An extension with access to www.example.com.
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder()
+          .SetManifest(
+              DictionaryBuilder()
+                  .Set("name", "Extension with a host permission")
+                  .Set("version", "1.0")
+                  .Set("manifest_version", 2)
+                  .Set("permissions",
+                       ListBuilder().Append("http://www.example.com/*").Build())
+                  .Build())
+          .Build();
+  auto function = base::MakeRefCounted<TabsUpdateFunction>();
+  function->set_extension(extension);
+
+  // Add a web contents to the browser.
+  std::unique_ptr<content::WebContents> contents(
+      content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
+  content::WebContents* raw_contents = contents.get();
+  browser()->tab_strip_model()->AppendWebContents(std::move(contents), true);
+  EXPECT_EQ(browser()->tab_strip_model()->GetActiveWebContents(), raw_contents);
+  content::WebContentsTester* web_contents_tester =
+      content::WebContentsTester::For(raw_contents);
+  web_contents_tester->NavigateAndCommit(GURL("http://www.example.com"));
+  SessionTabHelper::CreateForWebContents(raw_contents);
+  int tab_id = SessionTabHelper::IdForTab(raw_contents).id();
+
+  static constexpr char kFormatArgs[] = R"([%d, {"url": "%s"}])";
+  const std::string args = base::StringPrintf(
+      kFormatArgs, tab_id, "javascript:void(document.title = 'Won't work')");
+  std::string error = extension_function_test_utils::RunFunctionAndReturnError(
+      function.get(), args, browser(), api_test_utils::NONE);
+  EXPECT_EQ(tabs_constants::kJavaScriptUrlsNotAllowedInTabsUpdate, error);
+
+  // Clean up.
+  browser()->tab_strip_model()->CloseAllTabs();
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/tabs/tabs_constants.cc

@@ -114,6 +114,9 @@ const char kCannotDetermineLanguageOfUnloadedTab[] =
 const char kMissingLockWindowFullscreenPrivatePermission[] =
     "Cannot lock window to fullscreen or close a locked fullscreen window "
     "without lockWindowFullscreenPrivate manifest permission";
+const char kJavaScriptUrlsNotAllowedInTabsUpdate[] =
+    "JavaScript URLs are not allowed in chrome.tabs.update. Use "
+    "chrome.tabs.executeScript instead.";
 
 }  // namespace tabs_constants
 }  // namespace extensions

chrome/browser/extensions/api/tabs/tabs_constants.h

@@ -105,6 +105,7 @@ extern const char kCannotUpdateMuteDisabled[];
 extern const char kCannotUpdateMuteCaptured[];
 extern const char kCannotDetermineLanguageOfUnloadedTab[];
 extern const char kMissingLockWindowFullscreenPrivatePermission[];
+extern const char kJavaScriptUrlsNotAllowedInTabsUpdate[];
 
 };  // namespace tabs_constants
 };  // namespace extensions

chrome/browser/extensions/execute_script_apitest.cc

@@ -74,11 +74,6 @@ IN_PROC_BROWSER_TEST_F(ExecuteScriptApiTest,
                                   "execute_script.html")) << message_;
 }
 
-IN_PROC_BROWSER_TEST_F(ExecuteScriptApiTest, NavigationRaceJavaScriptURL) {
-  ASSERT_TRUE(RunExtensionSubtest("executescript/navigation_race",
-                                  "javascript_url.html")) << message_;
-}
-
 // If failing, mark disabled and update http://crbug.com/92105.
 IN_PROC_BROWSER_TEST_F(ExecuteScriptApiTest, ExecuteScriptFrameAfterLoad) {
   ASSERT_TRUE(RunExtensionTest("executescript/frame_after_load")) << message_;

chrome/browser/extensions/extension_javascript_url_apitest.cc

-// Copyright (c) 2012 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"
-#include "net/dns/mock_host_resolver.h"
-
-class JavscriptApiTest : public extensions::ExtensionApiTest {
- public:
-  void SetUpOnMainThread() override {
-    extensions::ExtensionApiTest::SetUpOnMainThread();
-    host_resolver()->AddRule("*", "127.0.0.1");
-    ASSERT_TRUE(StartEmbeddedTestServer());
-  }
-};
-
-// If crashing, mark disabled and update http://crbug.com/63589.
-IN_PROC_BROWSER_TEST_F(JavscriptApiTest, JavaScriptURLPermissions) {
-  ASSERT_TRUE(RunExtensionTest("tabs/javascript_url_permissions")) << message_;
-}
-
-IN_PROC_BROWSER_TEST_F(JavscriptApiTest, JavasScriptEncodedURL) {
-  ASSERT_TRUE(RunExtensionTest("tabs/javascript_url_encoded")) << message_;
-}

chrome/common/extensions/api/tabs.json

@@ -578,7 +578,7 @@
               "url": {
                 "type": "string",
                 "optional": true,
-                "description": "A URL to navigate the tab to."
+                "description": "A URL to navigate the tab to. JavaScript URLs are not supported; use $(ref:tabs.executeScript) instead."
               },
               "active": {
                 "type": "boolean",

chrome/test/BUILD.gn

@@ -1357,7 +1357,6 @@ test("browser_tests") {
         "../browser/extensions/extension_install_prompt_test_helper.cc",
         "../browser/extensions/extension_install_prompt_test_helper.h",
         "../browser/extensions/extension_install_ui_browsertest.cc",
-        "../browser/extensions/extension_javascript_url_apitest.cc",
         "../browser/extensions/extension_loading_browsertest.cc",
         "../browser/extensions/extension_management_test_util.cc",
         "../browser/extensions/extension_management_test_util.h",

chrome/test/data/extensions/api_test/executescript/navigation_race/javascript_url.html

-<!--
- * Copyright (c) 2011 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.
--->
-<script src="test.js"></script>
-<script src="javascript_url.js"></script>
\ No newline at end of file

chrome/test/data/extensions/api_test/executescript/navigation_race/javascript_url.js

-// Copyright (c) 2011 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 executeCodeInTab(tabId, callback) {
-  chrome.tabs.update(
-      tabId,
-      {url: "javascript:void(document.title = 'hi, I\\'m on ' + location)"},
-      callback);
-}