Reload force-installed extensions on crash/force-close

chrome/browser/policy/policy_browsertest.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 <algorithm>
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_service.h"
 #include "base/run_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/test_file_util.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/autocomplete/autocomplete_controller.h"
+#include "chrome/browser/background/background_contents_service.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/devtools/devtools_window.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/extensions/crx_installer.h"
+#include "chrome/browser/extensions/extension_host.h"
+#include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/unpacked_installer.h"
 #include "chrome/browser/extensions/updater/extension_updater.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/media/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/media_stream_devices_controller.h"
 #include "chrome/browser/metrics/variations/variations_service.h"
 #include "chrome/browser/net/url_request_mock_util.h"
 #include "chrome/browser/plugins/plugin_prefs.h"
@@ skipping 33 matching lines @@
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/test_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/browser_child_process_host_iterator.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_data.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/plugin_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_paths.h"
 #include "content/public/common/page_transition_types.h"
 #include "content/public/common/process_type.h"
+#include "content/public/common/result_codes.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/common/webplugininfo.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/download_test_observer.h"
 #include "content/public/test/mock_notification_observer.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "content/test/net/url_request_failed_job.h"
 #include "content/test/net/url_request_mock_http_job.h"
 #include "grit/generated_resources.h"
@@ skipping 331 matching lines @@
     ++output_mute_changed_count_;
   }
 
  private:
   int output_mute_changed_count_;
 
   DISALLOW_COPY_AND_ASSIGN(TestAudioObserver);
 };
 #endif
 
+// This class is a customised version of WindowedNotificationObserver to allow

[bartfab (slow), 2013/08/26 13:26:53]

Nit: s/WindowedNotificationObserver/content::WindowedNotificationObserver/

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+// us to register and wait until one of two simple events has occurred. See

[bartfab (slow), 2013/08/26 13:26:53]

Nit: "simple events" is nomenclature I introduced in
WindowedNotificationObserver only. It is not what you would normally use in
Chrome. The comment should more be something along the lines of:

This is a customized version of content::WindowedNotificationObserver that waits
until either of the two provided notification types is obseved.

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+// WindowedNotificationObserver for further documentation.
+class OneOfTwoNotificationsObserver : public content::NotificationObserver {
+ public:
+  // Set up to wait for one of two conditions. The condition is met when a

[bartfab (slow), 2013/08/26 13:26:53]

Nit: As above, "condition" is not the right term here. "Notification" is.

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+  // notification of either |notification_type1| or |notification_type1| is
+  // received.
+  OneOfTwoNotificationsObserver(int notification_type1, int notification_type2);
+

[bartfab (slow), 2013/08/26 13:26:53]

Nit: Remove this blank line.

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+  virtual ~OneOfTwoNotificationsObserver();
+
+  // Wait until the specified condition is met. If the condition is already met

[bartfab (slow), 2013/08/26 13:26:53]

Nit: As above, "notification" instead of "condition".

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+  // (that is, the expected notification has already been received), Wait()
+  // returns immediately.
+  void Wait();
+
+  // NotificationObserver:

[bartfab (slow), 2013/08/26 13:26:53]

Nit: s/NotificationObserver/content::NotificationObserver/

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+ private:
+  bool seen_;
+  bool running_;
+  content::NotificationRegistrar registrar_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(OneOfTwoNotificationsObserver);
+};
+
+OneOfTwoNotificationsObserver::OneOfTwoNotificationsObserver(
+    int notification_type1, int notification_type2)
+    : seen_(false), running_(false) {
+  registrar_.Add(this, notification_type1,
+                 content::NotificationService::AllSources());
+  registrar_.Add(this, notification_type2,
+                 content::NotificationService::AllSources());
+}
+
+OneOfTwoNotificationsObserver::~OneOfTwoNotificationsObserver() {}
+
+void OneOfTwoNotificationsObserver::Wait() {
+  if (seen_)
+      return;

[bartfab (slow), 2013/08/26 13:26:53]

Nit: Indent 2 spaces, not 4.

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+  running_ = true;
+  message_loop_runner_ = new content::MessageLoopRunner;
+  message_loop_runner_->Run();
+  EXPECT_TRUE(seen_);
+}
+
+// NotificationObserver:
+void OneOfTwoNotificationsObserver::Observe(int type,
+                     const content::NotificationSource& source,
+                     const content::NotificationDetails& details) OVERRIDE {
+  seen_ = true;
+  if (!running_)
+    return;
+  message_loop_runner_->Quit();
+  running_ = false;
+}
+
 }  // namespace
 
 class PolicyTest : public InProcessBrowserTest {
  protected:
   PolicyTest() {}
   virtual ~PolicyTest() {}
 
   virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
     CommandLine::ForCurrentProcess()->AppendSwitch("noerrdialogs");
     EXPECT_CALL(provider_, IsInitializationComplete(_))
@@ skipping 923 matching lines @@
 
   // Updating the force-installed extension.
   extensions::ExtensionUpdater* updater = service->updater();
   extensions::ExtensionUpdater::CheckParams params;
   params.install_immediately = true;
   content::WindowedNotificationObserver update_observer(
       chrome::NOTIFICATION_EXTENSION_INSTALLED,
       content::NotificationService::AllSources());
   updater->CheckNow(params);
   update_observer.Wait();
-

[bartfab (slow), 2013/08/26 13:26:53]

Nit: Why remove the blank line?

[anitawoodruff, 2013/08/26 21:45:23]

Done.

   const base::Version* new_version =
       service->GetExtensionById(kGoodCrxId, true)->version();
   ASSERT_TRUE(new_version->IsValid());
   base::Version old_version(old_version_number);
   ASSERT_TRUE(old_version.IsValid());
 
   EXPECT_EQ(1, new_version->CompareTo(old_version));
 
   EXPECT_EQ(0u, interceptor.GetPendingSize());
+
+  // Wait for all the extension render view hosts that exist to finish loading.

[bartfab (slow), 2013/08/26 13:26:53]

Nit: Technically, this comment is correct. But it does not explain why you are
waiting for these RVHs to finish loading. Better use something like:

// Wait until any background pages belonging to force-installed extensions have
been loaded.

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+  ExtensionProcessManager* manager =
+      extensions::ExtensionSystem::Get(browser()->profile())->process_manager();
+  ExtensionProcessManager::ViewSet all_views = manager->GetAllViews();
+  for (ExtensionProcessManager::ViewSet::const_iterator iter =
+           all_views.begin();
+       iter != all_views.end();) {
+    if (!(*iter)->IsLoading()) {
+      ++iter;
+    } else {
+      OneOfTwoNotificationsObserver(
+          content::NOTIFICATION_LOAD_STOP,
+          content::NOTIFICATION_WEB_CONTENTS_DESTROYED).Wait();
+
+      // Test activity may have modified the set of extension processes during
+      // message processing, so re-start the iteration to catch added/removed
+      // processes.
+      all_views = manager->GetAllViews();
+      iter = all_views.begin();
+    }
+  }
+  // Test policy-installed extensions are reloaded when killed.

[bartfab (slow), 2013/08/26 13:26:53]

Nit: Add a blank line above.

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+  BackgroundContentsService::SetCrashDelaysForTesting(
+      base::TimeDelta::FromSeconds(0), base::TimeDelta::FromSeconds(0));
+  content::WindowedNotificationObserver extension_crashed_observer(
+      chrome::NOTIFICATION_EXTENSION_PROCESS_TERMINATED,
+      content::NotificationService::AllSources());
+  content::WindowedNotificationObserver extension_loaded_observer(
+      chrome::NOTIFICATION_EXTENSION_LOADED,
+      content::NotificationService::AllSources());
+  extensions::ExtensionHost* extension_host =
+      extensions::ExtensionSystem::Get(browser()->profile())->
+          process_manager()->GetBackgroundHostForExtension(kGoodCrxId);
+  base::KillProcess(extension_host->render_process_host()->GetHandle(),
+                    content::RESULT_CODE_KILLED, false);
+  extension_crashed_observer.Wait();
+  extension_loaded_observer.Wait();
 }
 
 IN_PROC_BROWSER_TEST_F(PolicyTest, ExtensionAllowedTypes) {
   // Verifies that extensions are blocked if policy specifies an allowed types
   // list and the extension's type is not on that list.
   ExtensionService* service = extension_service();
   ASSERT_FALSE(service->GetExtensionById(kGoodCrxId, true));
   ASSERT_FALSE(service->GetExtensionById(kHostedAppCrxId, true));
 
   base::ListValue allowed_types;
@@ skipping 1087 matching lines @@
       chrome_variations::VariationsService::GetVariationsServerURL(
           g_browser_process->local_state());
   EXPECT_TRUE(StartsWithASCII(url.spec(), default_variations_url, true));
   std::string value;
   EXPECT_TRUE(net::GetValueForKeyInQuery(url, "restrict", &value));
   EXPECT_EQ("restricted", value);
 }
 #endif
 
 }  // namespace policy