Add new permission string for extensions that override new tab pages.

chrome/browser/extensions/extension_loading_browsertest.cc

 // Copyright 2013 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 file contains tests for extension loading, reloading, and
 // unloading behavior.
 
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/version.h"
 #include "build/build_config.h"
 #include "chrome/browser/extensions/devtools_util.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/test_extension_dir.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/test/browser_test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/process_manager.h"
+#include "extensions/common/permissions/permissions_data.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
 #endif
 
 namespace extensions {
 namespace {
@@ skipping 60 matching lines @@
   AddTabAtIndex(browser()->tab_strip_model()->count(),
                 GURL("http://www.google.com/"),
                 ui::PAGE_TRANSITION_TYPED);
 
   // Check that the extension hasn't crashed.
   ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
   EXPECT_EQ(0U, registry->terminated_extensions().size());
   EXPECT_TRUE(registry->enabled_extensions().Contains(new_tab_extension->id()));
 }
 
+IN_PROC_BROWSER_TEST_F(ExtensionLoadingTest,
+                       UpgradeAddingNewTabPagePermissionNoPrompt) {
+  embedded_test_server()->ServeFilesFromDirectory(
+      base::FilePath(FILE_PATH_LITERAL("chrome/test/data")));
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  TestExtensionDir extension_dir;
+  const char manifest_template[] =
+      "{"
+      "  'name': 'Overrides New Tab',"
+      "  'version': '%d',"
+      "  'description': 'Will override New Tab soon',"
+      "  'manifest_version': 2,"
+      "  %s"  // Placeholder for future NTP url override block.
+      "  'background': {"
+      "    'persistent': false,"
+      "    'scripts': ['event.js']"
+      "  }"
+      "}";
+  extension_dir.WriteManifestWithSingleQuotes(
+      base::StringPrintf(manifest_template, 1, ""));
+  extension_dir.WriteFile(FILE_PATH_LITERAL("event.js"), "");
+  extension_dir.WriteFile(FILE_PATH_LITERAL("newtab.html"),
+                          "<h1>Overridden New Tab Page</h1>");
+
+  const Extension* new_tab_extension =
+      InstallExtension(extension_dir.Pack(), 1 /*new install*/);
+  ASSERT_NE(nullptr, new_tab_extension);

[Devlin, 2017/07/05 15:45:02]

optional: I think ASSERT_TRUE(new_tab_extension) is more common, but either way
works.

[robertshield, 2017/07/18 19:46:20]

Done.

+
+  EXPECT_FALSE(new_tab_extension->permissions_data()->HasAPIPermission(
+      APIPermission::kNewTabPageOverride));
+
+  // Navigate that tab to a non-extension URL to swap out the extension's
+  // renderer.
+  const GURL test_link_from_NTP =

[Devlin, 2017/07/05 15:45:02]

nit: ignore capitals in variable names, so just |test_link_from_ntp|.

[robertshield, 2017/07/18 19:46:20]

Done.

+      embedded_test_server()->GetURL("/README.chromium");
+  EXPECT_THAT(test_link_from_NTP.spec(), testing::EndsWith("/README.chromium"))
+      << "Check that the test server started.";
+  NavigateInRenderer(browser()->tab_strip_model()->GetActiveWebContents(),
+                     test_link_from_NTP);
+
+  // Increase the extension's version and add the NTP url override which will
+  // add the kNewTabPageOverride permission.
+  const char ntp_override_string[] =
+      "  'chrome_url_overrides': {"
+      "    'newtab': 'newtab.html'"
+      "  },";
+  extension_dir.WriteManifestWithSingleQuotes(
+      base::StringPrintf(manifest_template, 2, ntp_override_string));
+
+  // Upgrade the extension, ensure that the upgrade 'worked' in the sense that
+  // the extension is still present and not disabled and that it now has the

[Devlin, 2017/07/05 15:45:02]

Is this desired?  It means that if I don't want the user to see that I change
the NTP, I could have them install the extension at v1 and then upgrade to v2 to
silently swap out the NTP for all existing users.  I assume this is just to
allow for the migration process?

[robertshield, 2017/07/18 19:46:20]

Yes, it's intended for migration. The intent is not to re-prompt users of
existing ntp override extensions so this does allow exactly the strategy you
describe. 

In practice updating an extension that does not replace the NTP to start doing
so is likely a policy violation so may be addressable through other means.

+  // new API permission.
+  new_tab_extension = UpdateExtension(
+      new_tab_extension->id(), extension_dir.Pack(), 0 /*expected upgrade*/);
+  ASSERT_NE(nullptr, new_tab_extension);
+
+  EXPECT_TRUE(new_tab_extension->permissions_data()->HasAPIPermission(
+      APIPermission::kNewTabPageOverride));
+  EXPECT_THAT(new_tab_extension->version()->components(),
+              testing::ElementsAre(2));
+}
+
 // Tests the behavior described in http://crbug.com/532088.
 IN_PROC_BROWSER_TEST_F(ExtensionLoadingTest,
                        KeepAliveWithDevToolsOpenOnReload) {
   embedded_test_server()->ServeFilesFromDirectory(
       base::FilePath(FILE_PATH_LITERAL("chrome/test/data")));
   ASSERT_TRUE(embedded_test_server()->Start());
 
   TestExtensionDir extension_dir;
   const char manifest_contents[] =
       "{"
@@ skipping 116 matching lines @@
 
   // Now check whether the extension runtime is valid (see kTargetJs).
   bool is_valid = false;
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
       bg_contents, "domAutomationController.send(is_valid);", &is_valid));
   EXPECT_TRUE(is_valid);
 }
 
 }  // namespace
 }  // namespace extensions