[MD settings] listen for ntp extension changes

This CL observes changes to extensions that may control the NTP and
sends that information to the OnStartup settings page.

Bug: 740779
Change-Id: Icdc539f0fcb64eeb6f0dc6f8eb9ade061d4e870e
Reviewed-on: https://chromium-review.googlesource.com/567419
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486645}

chrome/browser/resources/settings/on_startup_page/startup_urls_page.js

@@ -63,6 +63,10 @@ Polymer({
   /** @override */
   attached: function() {
     this.getNtpExtension_();
+    this.addWebUIListener('update-ntp-extension', function(ntpExtension) {
+      // Note that |ntpExtension| is empty if there is no NTP extension.
+      this.ntpExtension_ = ntpExtension;
+    }.bind(this));
 
     this.browserProxy_ = settings.StartupUrlsPageBrowserProxyImpl.getInstance();
     this.addWebUIListener('update-startup-pages', function(startupPages) {

chrome/browser/ui/webui/settings/md_settings_ui.cc

@@ -6,7 +6,9 @@
 
 #include <stddef.h>
 
+#include <memory>
 #include <string>
+#include <utility>
 
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
@@ -134,7 +136,7 @@ MdSettingsUI::MdSettingsUI(content::WebUI* web_ui, const GURL& url)
 #if defined(GOOGLE_CHROME_BUILD) && !defined(OS_CHROMEOS)
   AddSettingsPageUIHandler(base::MakeUnique<MetricsReportingHandler>());
 #endif
-  AddSettingsPageUIHandler(base::MakeUnique<OnStartupHandler>());
+  AddSettingsPageUIHandler(base::MakeUnique<OnStartupHandler>(profile));
   AddSettingsPageUIHandler(base::MakeUnique<PeopleHandler>(profile));
   AddSettingsPageUIHandler(base::MakeUnique<ProfileInfoHandler>(profile));
   AddSettingsPageUIHandler(base::MakeUnique<ProtocolHandlersHandler>());

chrome/browser/ui/webui/settings/on_startup_handler.cc

@@ -4,12 +4,15 @@
 
 #include "chrome/browser/ui/webui/settings/on_startup_handler.h"
 
+#include <string>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "chrome/browser/extensions/settings_api_helpers.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/webui/settings_utils.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_ui.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/management_policy.h"
@@ -17,9 +20,25 @@
 
 namespace settings {
 
-OnStartupHandler::OnStartupHandler() {}
+// static
+const char OnStartupHandler::kOnStartupNtpExtensionEventName[] =
+    "update-ntp-extension";
+
+OnStartupHandler::OnStartupHandler(Profile* profile)
+    : extension_registry_observer_(this), profile_(profile) {
+  DCHECK(profile);
+}
 OnStartupHandler::~OnStartupHandler() {}
 
+void OnStartupHandler::OnJavascriptAllowed() {
+  extension_registry_observer_.Add(
+      extensions::ExtensionRegistry::Get(profile_));
+}
+
+void OnStartupHandler::OnJavascriptDisallowed() {
+  extension_registry_observer_.RemoveAll();
+}
+
 void OnStartupHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "getNtpExtension", base::Bind(&OnStartupHandler::HandleGetNtpExtension,
@@ -30,41 +49,52 @@ void OnStartupHandler::RegisterMessages() {
                  base::Unretained(this)));
 }
 
-void OnStartupHandler::HandleGetNtpExtension(const base::ListValue* args) {
-  const base::Value* callback_id;
-  CHECK(args->Get(0, &callback_id));
+void OnStartupHandler::OnExtensionUnloaded(
+    content::BrowserContext* browser_context,
+    const extensions::Extension* extension,
+    extensions::UnloadedExtensionReason reason) {
+  FireWebUIListener(kOnStartupNtpExtensionEventName, *GetNtpExtension());
+}
 
-  AllowJavascript();
+void OnStartupHandler::OnExtensionReady(
+    content::BrowserContext* browser_context,
+    const extensions::Extension* extension) {
+  FireWebUIListener(kOnStartupNtpExtensionEventName, *GetNtpExtension());
+}
 
-  Profile* profile = Profile::FromWebUI(web_ui());
+std::unique_ptr<base::Value> OnStartupHandler::GetNtpExtension() {
   const extensions::Extension* ntp_extension =
-      extensions::GetExtensionOverridingNewTabPage(profile);
-
+      extensions::GetExtensionOverridingNewTabPage(profile_);
   if (!ntp_extension) {
-    ResolveJavascriptCallback(*callback_id, base::Value());
-    return;
+    std::unique_ptr<base::Value> none(new base::Value);
+    return none;
   }
 
-  base::DictionaryValue dict;
-  dict.SetString("id", ntp_extension->id());
-  dict.SetString("name", ntp_extension->name());
-  dict.SetBoolean("canBeDisabled",
-                  !extensions::ExtensionSystem::Get(profile)
-                       ->management_policy()
-                       ->MustRemainEnabled(ntp_extension, nullptr));
-  ResolveJavascriptCallback(*callback_id, dict);
+  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
+  dict->SetString("id", ntp_extension->id());
+  dict->SetString("name", ntp_extension->name());
+  dict->SetBoolean("canBeDisabled",
+                   !extensions::ExtensionSystem::Get(profile_)
+                        ->management_policy()
+                        ->MustRemainEnabled(ntp_extension, nullptr));
+  return dict;
 }
 
-void OnStartupHandler::HandleValidateStartupPage(const base::ListValue* args) {
+void OnStartupHandler::HandleGetNtpExtension(const base::ListValue* args) {
+  const base::Value* callback_id;
+  CHECK(args->Get(0, &callback_id));
   AllowJavascript();
 
-  CHECK_EQ(args->GetSize(), 2U);
+  ResolveJavascriptCallback(*callback_id, *GetNtpExtension());
+}
 
+void OnStartupHandler::HandleValidateStartupPage(const base::ListValue* args) {
+  CHECK_EQ(args->GetSize(), 2U);
   const base::Value* callback_id;
   CHECK(args->Get(0, &callback_id));
-
   std::string url_string;
   CHECK(args->GetString(1, &url_string));
+  AllowJavascript();
 
   bool valid = settings_utils::FixupAndValidateStartupPage(url_string, nullptr);
   ResolveJavascriptCallback(*callback_id, base::Value(valid));

chrome/browser/ui/webui/settings/on_startup_handler.h

@@ -5,8 +5,15 @@
 #ifndef CHROME_BROWSER_UI_WEBUI_SETTINGS_ON_STARTUP_HANDLER_H_
 #define CHROME_BROWSER_UI_WEBUI_SETTINGS_ON_STARTUP_HANDLER_H_
 
+#include <memory>
+
 #include "base/macros.h"
+#include "base/scoped_observer.h"
 #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/extension_registry_observer.h"
+
+class Profile;
 
 namespace base {
 class ListValue;
@@ -14,17 +21,29 @@ class ListValue;
 
 namespace settings {
 
-class OnStartupHandler : public SettingsPageUIHandler {
+class OnStartupHandler : public SettingsPageUIHandler,
+                         public extensions::ExtensionRegistryObserver {
  public:
-  OnStartupHandler();
+  static const char kOnStartupNtpExtensionEventName[];
+
+  explicit OnStartupHandler(Profile* profile);
   ~OnStartupHandler() override;
 
   // SettingsPageUIHandler:
+  void OnJavascriptAllowed() override;
+  void OnJavascriptDisallowed() override;
   void RegisterMessages() override;
-  void OnJavascriptAllowed() override {}
-  void OnJavascriptDisallowed() override {}
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(OnStartupHandlerTest, HandleGetNtpExtension);
+  FRIEND_TEST_ALL_PREFIXES(OnStartupHandlerTest,
+                           HandleValidateStartupPage_Valid);
+  FRIEND_TEST_ALL_PREFIXES(OnStartupHandlerTest,
+                           HandleValidateStartupPage_Invalid);
+
+  // Info for extension controlling the NTP or empty value.
+  std::unique_ptr<base::Value> GetNtpExtension();
+
   // Handler for the "getNtpExtension" message. No arguments.
   void HandleGetNtpExtension(const base::ListValue* /*args*/);
 
@@ -32,6 +51,20 @@ class OnStartupHandler : public SettingsPageUIHandler {
   // valid startup page.
   void HandleValidateStartupPage(const base::ListValue* args);
 
+  // extensions::ExtensionRegistryObserver.
+  void OnExtensionUnloaded(content::BrowserContext* browser_context,
+                           const extensions::Extension* extension,
+                           extensions::UnloadedExtensionReason reason) override;
+  void OnExtensionReady(content::BrowserContext* browser_context,
+                        const extensions::Extension* extension) override;
+
+  // Listen to extension unloaded notifications.
+  ScopedObserver<extensions::ExtensionRegistry,
+                 extensions::ExtensionRegistryObserver>
+      extension_registry_observer_;
+
+  Profile* profile_;
+
   DISALLOW_COPY_AND_ASSIGN(OnStartupHandler);
 };
 

chrome/browser/ui/webui/settings/on_startup_handler_unittest.cc

+// Copyright 2017 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/ui/webui/settings/on_startup_handler.h"
+
+#include <string>
+
+#include "base/macros.h"
+#include "base/values.h"
+#include "build/build_config.h"
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
+#include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"
+#endif
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_web_ui.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+const char kCallbackId[] = "test-on-startup-callback-id";
+
+class TestOnStartupHandler : public settings::OnStartupHandler {
+ public:
+  explicit TestOnStartupHandler(Profile* profile)
+      : settings::OnStartupHandler(profile) {}
+
+  using settings::OnStartupHandler::set_web_ui;
+};
+
+}  // namespace
+
+namespace settings {
+
+class OnStartupHandlerTest : public testing::Test {
+ public:
+  OnStartupHandlerTest()
+      : profile_manager_(TestingBrowserProcess::GetGlobal()),
+        profile_(nullptr) {}
+
+  void SetUp() override {
+    ASSERT_TRUE(profile_manager_.SetUp());
+
+#if defined(OS_CHROMEOS)
+    chromeos::FakeChromeUserManager* fake_user_manager =
+        new chromeos::FakeChromeUserManager;
+    user_manager_enabler_.reset(
+        new chromeos::ScopedUserManagerEnabler(fake_user_manager));
+    constexpr char kFakeEmail[] = "fake_id@gmail.com";
+    profile_ = profile_manager_.CreateTestingProfile(kFakeEmail);
+    fake_user_manager->AddUser(AccountId::FromUserEmail(kFakeEmail));
+#else
+    profile_ = profile_manager_.CreateTestingProfile("Profile 1");
+#endif
+
+    handler_.reset(new TestOnStartupHandler(profile_));
+    handler_->set_web_ui(&web_ui_);
+  }
+
+  TestOnStartupHandler* handler() { return handler_.get(); }
+  Profile* profile() const { return profile_; }
+  content::TestWebUI* web_ui() { return &web_ui_; }
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+  TestingProfileManager profile_manager_;
+  std::unique_ptr<TestOnStartupHandler> handler_;
+  Profile* profile_;
+#if defined(OS_CHROMEOS)
+  std::unique_ptr<chromeos::ScopedUserManagerEnabler> user_manager_enabler_;
+#endif
+  content::TestWebUI web_ui_;
+};
+
+TEST_F(OnStartupHandlerTest, HandleGetNtpExtension) {
+  base::ListValue list_args;
+  list_args.AppendString(kCallbackId);
+  handler()->HandleGetNtpExtension(&list_args);
+
+  EXPECT_EQ(1U, web_ui()->call_data().size());
+
+  const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
+  EXPECT_EQ("cr.webUIResponse", data.function_name());
+
+  std::string callback_id;
+  ASSERT_TRUE(data.arg1()->GetAsString(&callback_id));
+  EXPECT_EQ(kCallbackId, callback_id);
+
+  bool success = false;
+  ASSERT_TRUE(data.arg2()->GetAsBoolean(&success));
+  EXPECT_TRUE(success);
+}
+
+TEST_F(OnStartupHandlerTest, HandleValidateStartupPage_Valid) {
+  base::ListValue list_args;
+  list_args.AppendString(kCallbackId);
+  list_args.AppendString("http://example.com");
+  handler()->HandleValidateStartupPage(&list_args);
+
+  EXPECT_EQ(1U, web_ui()->call_data().size());
+
+  const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
+  EXPECT_EQ("cr.webUIResponse", data.function_name());
+
+  std::string callback_id;
+  ASSERT_TRUE(data.arg1()->GetAsString(&callback_id));
+  EXPECT_EQ(kCallbackId, callback_id);
+
+  bool success = false;
+  ASSERT_TRUE(data.arg2()->GetAsBoolean(&success));
+  EXPECT_TRUE(success);
+
+  bool is_valid = false;
+  ASSERT_TRUE(data.arg2()->GetAsBoolean(&is_valid));
+  EXPECT_TRUE(is_valid);
+}
+
+TEST_F(OnStartupHandlerTest, HandleValidateStartupPage_Invalid) {
+  base::ListValue list_args;
+  list_args.AppendString(kCallbackId);
+  list_args.AppendString("@");
+  handler()->HandleValidateStartupPage(&list_args);
+
+  EXPECT_EQ(1U, web_ui()->call_data().size());
+
+  const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
+  EXPECT_EQ("cr.webUIResponse", data.function_name());
+
+  std::string callback_id;
+  ASSERT_TRUE(data.arg1()->GetAsString(&callback_id));
+  EXPECT_EQ(kCallbackId, callback_id);
+
+  bool success = false;
+  ASSERT_TRUE(data.arg2()->GetAsBoolean(&success));
+  EXPECT_TRUE(success);
+
+  bool is_valid = false;
+  ASSERT_TRUE(data.arg3()->GetAsBoolean(&is_valid));
+  EXPECT_FALSE(is_valid);
+}
+
+}  // namespace settings

chrome/test/BUILD.gn

@@ -3723,6 +3723,7 @@ test("unit_tests") {
       "../browser/ui/webui/settings/downloads_handler_unittest.cc",
       "../browser/ui/webui/settings/md_settings_ui_unittest.cc",
       "../browser/ui/webui/settings/metrics_reporting_handler_unittest.cc",
+      "../browser/ui/webui/settings/on_startup_handler_unittest.cc",
       "../browser/ui/webui/settings/people_handler_unittest.cc",
       "../browser/ui/webui/settings/profile_info_handler_unittest.cc",
       "../browser/ui/webui/settings/reset_settings_handler_unittest.cc",