Try again to land: http://codereview.chromium.org/160483

Ever closer. Extract a client interface out of CrxInstaller
and use it to implement ExtensionInstallUI.

There is (still) no dialog here. But the next CL will, um,
definitely have it.

Also, fixed the issue with theme preview infobars not updating if you install a theme while another one is
already previewed.

TBR=mpcomplete@chromium.org
BUG=17932

Review URL: http://codereview.chromium.org/160501

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@22244 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 363 insertions, 187 deletions

diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 6b6c732..60fe23e 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -14,38 +14,48 @@
 #include "chrome/common/extensions/extension_error_reporter.h"
 #include "grit/chromium_strings.h"
 
-#if defined(OS_WIN)
-#include "app/win_util.h"
-#elif defined(OS_MACOSX)
-#include "base/scoped_cftyperef.h"
-#include "base/sys_string_conversions.h"
-#include <CoreFoundation/CFUserNotification.h>
-#endif
+namespace {
+  // Helper function to delete files. This is used to avoid ugly casts which
+  // would be necessary with PostMessage since file_util::Delete is overloaded.
+  static void DeleteFileHelper(const FilePath& path, bool recursive) {
+    file_util::Delete(path, recursive);
+  }
+}
+
+void CrxInstaller::Start(const FilePath& crx_path,
+                         const FilePath& install_directory,
+                         Extension::Location install_source,
+                         const std::string& expected_id,
+                         bool delete_crx,
+                         MessageLoop* file_loop,
+                         ExtensionsService* frontend,
+                         CrxInstallerClient* client) {
+  // Note: We don't keep a reference because this object manages its own
+  // lifetime.
+  new CrxInstaller(crx_path, install_directory, install_source, expected_id,
+                   delete_crx, file_loop, frontend, client);
+}
 
 CrxInstaller::CrxInstaller(const FilePath& crx_path,
                            const FilePath& install_directory,
                            Extension::Location install_source,
                            const std::string& expected_id,
-                           bool extensions_enabled,
-                           bool is_from_gallery,
-                           bool show_prompts,
                            bool delete_crx,
                            MessageLoop* file_loop,
-                           ExtensionsService* frontend)
+                           ExtensionsService* frontend,
+                           CrxInstallerClient* client)
     : crx_path_(crx_path),
       install_directory_(install_directory),
       install_source_(install_source),
       expected_id_(expected_id),
-      extensions_enabled_(extensions_enabled),
-      is_from_gallery_(is_from_gallery),
-      show_prompts_(show_prompts),
       delete_crx_(delete_crx),
       file_loop_(file_loop),
-      ui_loop_(MessageLoop::current()) {
+      ui_loop_(MessageLoop::current()),
+      frontend_(frontend),
+      client_(client) {
+
+  extensions_enabled_ = frontend_->extensions_enabled();
 
-  // Note: this is a refptr so that we keep the frontend alive long enough to
-  // get our response.
-  frontend_ = frontend;
   unpacker_ = new SandboxedExtensionUnpacker(
       crx_path, g_browser_process->resource_dispatcher_host(), this);
 
@@ -53,31 +63,40 @@ CrxInstaller::CrxInstaller(const FilePath& crx_path,
       &SandboxedExtensionUnpacker::Start));
 }
 
+CrxInstaller::~CrxInstaller() {
+  // Delete the temp directory and crx file as necessary. Note that the
+  // destructor might be called on any thread, so we post a task to the file
+  // thread to make sure the delete happens there.
+  if (!temp_dir_.value().empty()) {
+    file_loop_->PostTask(FROM_HERE, NewRunnableFunction(&DeleteFileHelper,
+        temp_dir_, true));  // recursive delete
+  }
+
+  if (delete_crx_) {
+    file_loop_->PostTask(FROM_HERE, NewRunnableFunction(&DeleteFileHelper,
+        crx_path_, false));  // non-recursive delete
+  }
+}
+
 void CrxInstaller::OnUnpackFailure(const std::string& error_message) {
+  DCHECK(MessageLoop::current() == file_loop_);
   ReportFailureFromFileThread(error_message);
 }
 
 void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir,
                                    const FilePath& extension_dir,
                                    Extension* extension) {
+  DCHECK(MessageLoop::current() == file_loop_);
+
   // Note: We take ownership of |extension| and |temp_dir|.
   extension_.reset(extension);
   temp_dir_ = temp_dir;
 
-  // temp_dir_deleter is stack allocated instead of a member of CrxInstaller, so
-  // that delete always happens on the file thread.
-  ScopedTempDir temp_dir_deleter;
-  temp_dir_deleter.Set(temp_dir);
-
   // The unpack dir we don't have to delete explicity since it is a child of
   // the temp dir.
   unpacked_extension_root_ = extension_dir;
   DCHECK(file_util::ContainsPath(temp_dir_, unpacked_extension_root_));
 
-  // If we were supposed to delete the source file, we can do that now.
-  if (delete_crx_)
-    file_util::Delete(crx_path_, false);  // non-recursive
-
   // Determine whether to allow installation. We always allow themes and
   // external installs.
   if (!extensions_enabled_ && !extension->IsTheme() &&
@@ -89,59 +108,36 @@ void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir,
   // Make sure the expected id matches.
   // TODO(aa): Also support expected version?
   if (!expected_id_.empty() && expected_id_ != extension->id()) {
-    ReportFailureFromFileThread(
-        StringPrintf("ID in new extension manifest (%s) does not match "
-                     "expected id (%s)",
-                     extension->id().c_str(),
-                     expected_id_.c_str()));
+    ReportFailureFromFileThread(StringPrintf(
+        "ID in new extension manifest (%s) does not match expected id (%s)",
+        extension->id().c_str(),
+        expected_id_.c_str()));
     return;
   }
 
-  // Show the confirm UI if necessary.
-  // NOTE: We also special case themes to not have a dialog, because we show
-  // a special infobar UI for them instead.
-  if (show_prompts_ && !extension->IsTheme()) {
-    if (!ConfirmInstall())
-      return;  // error reported by ConfirmInstall()
+  if (client_.get()) {
+    ui_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
+        &CrxInstaller::ConfirmInstall));
+  } else {
+    CompleteInstall();
   }
-
-  CompleteInstall();
 }
 
-bool CrxInstaller::ConfirmInstall() {
-#if defined(OS_WIN)
-  if (win_util::MessageBox(GetForegroundWindow(),
-          L"Are you sure you want to install this extension?\n\n"
-          L"You should only install extensions from sources you trust.",
-          l10n_util::GetString(IDS_PRODUCT_NAME).c_str(),
-          MB_OKCANCEL) != IDOK) {
-    ReportFailureFromFileThread("User did not allow extension to be "
-                                "installed.");
-    return false;
-  }
-#elif defined(OS_MACOSX)
-  // Using CoreFoundation to do this dialog is unimaginably lame but will do
-  // until the UI is redone.
-  scoped_cftyperef<CFStringRef> product_name(
-      base::SysWideToCFStringRef(l10n_util::GetString(IDS_PRODUCT_NAME)));
-  CFOptionFlags response;
-  if (kCFUserNotificationAlternateResponse == CFUserNotificationDisplayAlert(
-      0, kCFUserNotificationCautionAlertLevel, NULL, NULL, NULL,
-      product_name,
-      CFSTR("Are you sure you want to install this extension?\n\n"
-           "This is a temporary message and it will be removed when "
-           "extensions UI is finalized."),
-      NULL, CFSTR("Cancel"), NULL, &response)) {
-    ReportFailureFromFileThread("User did not allow extension to be "
-                                "installed.");
-    return false;
+void CrxInstaller::ConfirmInstall() {
+  if (!client_->ConfirmInstall(extension_.get())) {
+    // We're done. Since we don't post any more tasks to ourselves, our ref
+    // count should go to zero and we die. The destructor will clean up the temp
+    // dir.
+    return;
   }
-#endif  // OS_*
 
-  return true;
+  file_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
+      &CrxInstaller::CompleteInstall));
 }
 
 void CrxInstaller::CompleteInstall() {
+  DCHECK(MessageLoop::current() == file_loop_);
+
   FilePath version_dir;
   Extension::InstallType install_type = Extension::INSTALL_ERROR;
   std::string error_msg;
@@ -173,22 +169,57 @@ void CrxInstaller::CompleteInstall() {
 }
 
 void CrxInstaller::ReportFailureFromFileThread(const std::string& error) {
+  DCHECK(MessageLoop::current() == file_loop_);
   ui_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
       &CrxInstaller::ReportFailureFromUIThread, error));
 }
 
 void CrxInstaller::ReportFailureFromUIThread(const std::string& error) {
-  ExtensionErrorReporter::GetInstance()->ReportError(error, show_prompts_);
+  DCHECK(MessageLoop::current() == ui_loop_);
+
+  // This isn't really necessary, it is only used because unit tests expect to
+  // see errors get reported via this interface.
+  //
+  // TODO(aa): Need to go through unit tests and clean them up too, probably get
+  // rid of this line.
+  ExtensionErrorReporter::GetInstance()->ReportError(error, false);  // quiet
+
+  if (client_)
+    client_->OnInstallFailure(error);
 }
 
 void CrxInstaller::ReportOverinstallFromFileThread() {
-  ui_loop_->PostTask(FROM_HERE, NewRunnableMethod(frontend_.get(),
-      &ExtensionsService::OnExtensionOverinstallAttempted, extension_->id()));
+  DCHECK(MessageLoop::current() == file_loop_);
+  ui_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
+      &CrxInstaller::ReportOverinstallFromUIThread));
+}
+
+void CrxInstaller::ReportOverinstallFromUIThread() {
+  DCHECK(MessageLoop::current() == ui_loop_);
+
+  if (client_.get())
+    client_->OnOverinstallAttempted(extension_.get());
+
+  frontend_->OnExtensionOverinstallAttempted(extension_->id());
 }
 
 void CrxInstaller::ReportSuccessFromFileThread() {
+  DCHECK(MessageLoop::current() == file_loop_);
+  ui_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
+      &CrxInstaller::ReportSuccessFromUIThread));
+}
+
+void CrxInstaller::ReportSuccessFromUIThread() {
+  DCHECK(MessageLoop::current() == ui_loop_);
+
+  // If there is a client, tell the client about installation.
+  if (client_.get())
+    client_->OnInstallSuccess(extension_.get());
+
   // Tell the frontend about the installation and hand off ownership of
   // extension_ to it.
-  ui_loop_->PostTask(FROM_HERE, NewRunnableMethod(frontend_.get(),
-      &ExtensionsService::OnExtensionInstalled, extension_.release()));
+  frontend_->OnExtensionInstalled(extension_.release());
+
+  // We're done. We don't post any more tasks to ourselves so we are deleted
+  // soon.
 }
diff --git a/chrome/browser/extensions/crx_installer.h b/chrome/browser/extensions/crx_installer.h
index 9e5ff7c..5c373d9 100644
--- a/chrome/browser/extensions/crx_installer.h
+++ b/chrome/browser/extensions/crx_installer.h
@@ -14,6 +14,28 @@
 #include "chrome/browser/extensions/sandboxed_extension_unpacker.h"
 #include "chrome/common/extensions/extension.h"
 
+// Classes that want to know about install completion, or that want to have an
+// opportunity to reject the unpacked extension before installation, should
+// implement this interface.
+class CrxInstallerClient
+    : public base::RefCountedThreadSafe<CrxInstallerClient> {
+ public:
+  virtual ~CrxInstallerClient() {}
+
+  // Return true to indicate that installation should proceed, false otherwise.
+  virtual bool ConfirmInstall(Extension* extension) = 0;
+
+  // Installation was successful.
+  virtual void OnInstallSuccess(Extension* extension) = 0;
+
+  // Intallation failed.
+  virtual void OnInstallFailure(const std::string& error) = 0;
+
+  // The install was rejected because the same extension/version is already
+  // installed.
+  virtual void OnOverinstallAttempted(Extension* extension) = 0;
+};
+
 // This class installs a crx file into a profile.
 //
 // Installing a CRX is a multi-step process, including unpacking the crx,
@@ -22,7 +44,6 @@
 // necessary to do its job. (This also minimizes external dependencies for
 // easier testing).
 //
-//
 // Lifetime management:
 //
 // This class is ref-counted by each call it makes to itself on another thread,
@@ -30,46 +51,54 @@
 //
 // Additionally, we hold a reference to our own client so that it lives at least
 // long enough to receive the result of unpacking.
-// 
-//
-// NOTE: This class is rather huge at the moment because it is handling all
-// types of installation (external, autoupdate, and manual). In the future,
-// manual installation will probably pulled out of it.
 //
-// TODO(aa): Pull out the manual installation bits.
 // TODO(aa): Pull out a frontend interface for testing?
 class CrxInstaller : public SandboxedExtensionUnpackerClient {
  public:
+  // Starts the installation of the crx file in |crx_path| into
+  // |install_directory|.
+  //
+  // Other params:
+  //  install_source: The source of the install (external, --load-extension, etc
+  //  expected_id: Optional. If the caller knows what the ID of this extension
+  //               should be after unpacking, it can be specified here as a
+  //               sanity check.
+  //  delete_crx: Whether the crx should be deleted on completion.
+  //  file_loop: The message loop to do file IO on.
+  //  frontend: The ExtensionsService to report the successfully installed
+  //            extension to.
+  //  client: Optional. If specified, will be used to confirm installation and
+  //          also notified of success/fail. Note that we hold a reference to
+  //          this, so it can outlive its creator (eg the UI).
+  static void Start(const FilePath& crx_path,
+                    const FilePath& install_directory,
+                    Extension::Location install_source,
+                    const std::string& expected_id,
+                    bool delete_crx,
+                    MessageLoop* file_loop,
+                    ExtensionsService* frontend,
+                    CrxInstallerClient* client);
+
+ private:
   CrxInstaller(const FilePath& crx_path,
                const FilePath& install_directory,
                Extension::Location install_source,
                const std::string& expected_id,
-               bool extensions_enabled,
-               bool is_from_gallery,
-               bool show_prompts,
                bool delete_crx,
                MessageLoop* file_loop,
-               ExtensionsService* frontend);
-  ~CrxInstaller() {
-    // This is only here for debugging purposes, as a convenient place to set
-    // breakpoints.
-  }
+               ExtensionsService* frontend,
+               CrxInstallerClient* client);
+  ~CrxInstaller();
 
- private:
   // SandboxedExtensionUnpackerClient
   virtual void OnUnpackFailure(const std::string& error_message);
   virtual void OnUnpackSuccess(const FilePath& temp_dir,
                                const FilePath& extension_dir,
                                Extension* extension);
 
-  // Confirm with the user that it is OK to install this extension.
-  //
-  // Note that this runs on the file thread. It happens to be OK to do this on
-  // Windows and Mac, and although ugly, we leave it because this is all getting
-  // pulled out soon, anyway.
-  //
-  // TODO(aa): Pull this up, closer to the UI layer.
-  bool ConfirmInstall();
+  // Runs on the UI thread. Confirms with the user (via CrxInstallerClient) that
+  // it is OK to install this extension.
+  void ConfirmInstall();
 
   // Runs on File thread. Install the unpacked extension into the profile and
   // notify the frontend.
@@ -79,7 +108,9 @@ class CrxInstaller : public SandboxedExtensionUnpackerClient {
   void ReportFailureFromFileThread(const std::string& error);
   void ReportFailureFromUIThread(const std::string& error);
   void ReportOverinstallFromFileThread();
+  void ReportOverinstallFromUIThread();
   void ReportSuccessFromFileThread();
+  void ReportSuccessFromUIThread();
 
   // The crx file we're installing.
   FilePath crx_path_;
@@ -96,18 +127,11 @@ class CrxInstaller : public SandboxedExtensionUnpackerClient {
   // extension to contain.
   std::string expected_id_;
 
-  // Whether extension installation is set. We can't just check this before
-  // trying to install because themes are special-cased to always be allowed.
+  // Whether manual extension installation is enabled. We can't just check this
+  // before trying to install because themes are special-cased to always be
+  // allowed.
   bool extensions_enabled_;
 
-  // Whether this installation was initiated from the gallery. We trust it more
-  // and have special UI if it was.
-  bool is_from_gallery_;
-
-  // Whether we shoud should show prompts. This is sometimes false for testing
-  // and autoupdate.
-  bool show_prompts_;
-
   // Whether we're supposed to delete the source crx file on destruction.
   bool delete_crx_;
 
@@ -128,6 +152,10 @@ class CrxInstaller : public SandboxedExtensionUnpackerClient {
   // The frontend we will report results back to.
   scoped_refptr<ExtensionsService> frontend_;
 
+  // The client we will work with to do the installation. This can be NULL, in
+  // which case the install is silent.
+  scoped_refptr<CrxInstallerClient> client_;
+
   // The root of the unpacked extension directory. This is a subdirectory of
   // temp_dir_, so we don't have to delete it explicitly.
   FilePath unpacked_extension_root_;
diff --git a/chrome/browser/extensions/extension_install_ui.cc b/chrome/browser/extensions/extension_install_ui.cc
new file mode 100644
index 0000000..4416701
--- /dev/null
+++ b/chrome/browser/extensions/extension_install_ui.cc
@@ -0,0 +1,118 @@
+// Copyright (c) 2009 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_install_ui.h"
+
+#include "app/l10n_util.h"
+#include "grit/chromium_strings.h"
+#include "chrome/browser/browser_list.h"
+#include "chrome/browser/extensions/theme_preview_infobar_delegate.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
+
+#if defined(OS_WIN)
+#include "app/win_util.h"
+#elif defined(OS_MACOSX)
+#include "base/scoped_cftyperef.h"
+#include "base/sys_string_conversions.h"
+#include <CoreFoundation/CFUserNotification.h>
+#endif
+
+ExtensionInstallUI::ExtensionInstallUI(Profile* profile)
+    : profile_(profile), ui_loop_(MessageLoop::current()) {
+}
+
+bool ExtensionInstallUI::ConfirmInstall(Extension* extension) {
+  DCHECK(ui_loop_ == MessageLoop::current());
+
+  // We special-case themes to not show any confirm UI. Instead they are
+  // immediately installed, and then we show an infobar (see OnInstallSuccess)
+  // to allow the user to revert if they don't like it.
+  if (extension->IsTheme())
+    return true;
+
+#if defined(OS_WIN)
+  if (win_util::MessageBox(GetForegroundWindow(),
+          L"Are you sure you want to install this extension?\n\n"
+          L"You should only install extensions from sources you trust.",
+          l10n_util::GetString(IDS_PRODUCT_NAME).c_str(),
+          MB_OKCANCEL) != IDOK) {
+    return false;
+  }
+#elif defined(OS_MACOSX)
+  // Using CoreFoundation to do this dialog is unimaginably lame but will do
+  // until the UI is redone.
+  scoped_cftyperef<CFStringRef> product_name(
+      base::SysWideToCFStringRef(l10n_util::GetString(IDS_PRODUCT_NAME)));
+  CFOptionFlags response;
+  if (kCFUserNotificationAlternateResponse == CFUserNotificationDisplayAlert(
+      0, kCFUserNotificationCautionAlertLevel, NULL, NULL, NULL,
+      product_name,
+      CFSTR("Are you sure you want to install this extension?\n\n"
+           "This is a temporary message and it will be removed when "
+           "extensions UI is finalized."),
+      NULL, CFSTR("Cancel"), NULL, &response)) {
+    return false;
+  }
+#else
+  NOTREACHED();
+#endif  // OS_*
+
+  return true;
+}
+
+void ExtensionInstallUI::OnInstallSuccess(Extension* extension) {
+  ShowThemeInfoBar(extension);
+}
+
+void ExtensionInstallUI::OnInstallFailure(const std::string& error) {
+  DCHECK(ui_loop_ == MessageLoop::current());
+
+#if defined(OS_WIN)
+  win_util::MessageBox(NULL, UTF8ToWide(error), L"Extension Install Error",
+      MB_OK | MB_SETFOREGROUND);
+#elif defined(OS_MACOSX)
+  // There must be a better way to do this, for all platforms.
+  scoped_cftyperef<CFStringRef> message_cf(
+      base::SysUTF8ToCFStringRef(error));
+  CFOptionFlags response;
+  CFUserNotificationDisplayAlert(
+      0, kCFUserNotificationCautionAlertLevel, NULL, NULL, NULL,
+      CFSTR("Extension Install Error"), message_cf,
+      NULL, NULL, NULL, &response);
+#else
+  LOG(ERROR) << "Extension install failed: " << error.c_str();
+  NOTREACHED();
+#endif
+}
+
+void ExtensionInstallUI::OnOverinstallAttempted(Extension* extension) {
+  ShowThemeInfoBar(extension);
+}
+
+void ExtensionInstallUI::ShowThemeInfoBar(Extension* extension) {
+  if (!extension->IsTheme())
+    return;
+
+  Browser* browser = BrowserList::GetLastActiveWithProfile(profile_);
+  if (!browser)
+    return;
+
+  TabContents* tab_contents = browser->GetSelectedTabContents();
+  if (!tab_contents)
+    return;
+
+  // First remove any previous theme preview infobar.
+  for (int i = 0; i < tab_contents->infobar_delegate_count(); ++i) {
+    InfoBarDelegate* delegate = tab_contents->GetInfoBarDelegateAt(i);
+    if (delegate->AsThemePreviewInfobarDelegate()) {
+      tab_contents->RemoveInfoBar(delegate);
+      break;
+    }
+  }
+
+  // Now add the new one.
+  tab_contents->AddInfoBar(new ThemePreviewInfobarDelegate(
+      tab_contents, extension->name()));
+}
diff --git a/chrome/browser/extensions/extension_install_ui.h b/chrome/browser/extensions/extension_install_ui.h
new file mode 100644
index 0000000..1fc0379
--- /dev/null
+++ b/chrome/browser/extensions/extension_install_ui.h
@@ -0,0 +1,39 @@
+// Copyright (c) 2009 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.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALL_UI_H_
+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALL_UI_H_
+
+#include "base/file_path.h"
+#include "base/gfx/native_widget_types.h"
+#include "base/ref_counted.h"
+#include "chrome/browser/extensions/crx_installer.h"
+
+class ExtensionsService;
+class MessageLoop;
+class Profile;
+class SandboxedExtensionUnpacker;
+
+// Displays all the UI around extension installation.
+//
+// TODO(aa): This will become a view and move to browser/views/extensions in the
+// future.
+class ExtensionInstallUI : public CrxInstallerClient {
+ public:
+  ExtensionInstallUI(Profile* profile);
+
+ private:
+  // CrxInstallerClient
+  virtual bool ConfirmInstall(Extension* extension);
+  virtual void OnInstallSuccess(Extension* extension);
+  virtual void OnInstallFailure(const std::string& error);
+  virtual void OnOverinstallAttempted(Extension* extension);
+
+  void ShowThemeInfoBar(Extension* extension);
+
+  Profile* profile_;
+  MessageLoop* ui_loop_;
+};
+
+#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALL_UI_H_
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 3302f4d..bf657c7 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -14,9 +14,7 @@
 #include "chrome/browser/extensions/extension_updater.h"
 #include "chrome/browser/extensions/external_extension_provider.h"
 #include "chrome/browser/extensions/external_pref_extension_provider.h"
-#include "chrome/browser/extensions/theme_preview_infobar_delegate.h"
 #include "chrome/browser/profile.h"
-#include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_error_reporter.h"
@@ -34,10 +32,16 @@
 const char* ExtensionsService::kInstallDirectoryName = "Extensions";
 const char* ExtensionsService::kCurrentVersionFileName = "Current Version";
 
+/*
 const char* ExtensionsService::kGalleryDownloadURLPrefix =
     "https://dl-ssl.google.com/chrome/";
 const char* ExtensionsService::kGalleryURLPrefix =
     "https://tools.google.com/chrome/";
+*/
+const char* ExtensionsService::kGalleryDownloadURLPrefix =
+    "http://www.corp.google.com/~glen/chrome/";
+const char* ExtensionsService::kGalleryURLPrefix =
+    "http://www.corp.google.com/~glen/chrome/";
 
 // static
 bool ExtensionsService::IsDownloadFromGallery(const GURL& download_url,
@@ -108,20 +112,12 @@ void ExtensionsService::Init() {
 }
 
 void ExtensionsService::InstallExtension(const FilePath& extension_path) {
-  InstallExtension(extension_path, GURL(), GURL());
-}
-
-void ExtensionsService::InstallExtension(const FilePath& extension_path,
-                                         const GURL& download_url,
-                                         const GURL& referrer_url) {
-  new CrxInstaller(extension_path, install_directory_, Extension::INTERNAL,
-                   "",   // no expected id
-                   extensions_enabled_,
-                   IsDownloadFromGallery(download_url, referrer_url),
-                   show_extensions_prompts(),
-                   false,  // don't delete crx when complete
-                   backend_loop_,
-                   this);
+  CrxInstaller::Start(extension_path, install_directory_, Extension::INTERNAL,
+                      "",   // no expected id
+                      false,  // don't delete crx when complete
+                      backend_loop_,
+                      this,
+                      NULL);  // no client (silent install)
 }
 
 void ExtensionsService::UpdateExtension(const std::string& id,
@@ -132,13 +128,12 @@ void ExtensionsService::UpdateExtension(const std::string& id,
     return;
   }
 
-  new CrxInstaller(extension_path, install_directory_, Extension::INTERNAL,
-                   id, extensions_enabled_,
-                   false,  // not from gallery
-                   show_extensions_prompts(),
-                   true,  // delete crx when complete
-                   backend_loop_,
-                   this);
+  CrxInstaller::Start(extension_path, install_directory_, Extension::INTERNAL,
+                      id,
+                      true,  // delete crx when complete
+                      backend_loop_,
+                      this,
+                      NULL);  // no client (silent install)
 }
 
 void ExtensionsService::ReloadExtension(const std::string& extension_id) {
@@ -311,7 +306,6 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension) {
   // If the extension is a theme, tell the profile (and therefore ThemeProvider)
   // to apply it.
   if (extension->IsTheme()) {
-    ShowThemePreviewInfobar(extension);
     NotificationService::current()->Notify(
         NotificationType::THEME_INSTALLED,
         Source<ExtensionsService>(this),
@@ -333,7 +327,6 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension) {
 void ExtensionsService::OnExtensionOverinstallAttempted(const std::string& id) {
   Extension* extension = GetExtensionById(id);
   if (extension && extension->IsTheme()) {
-    ShowThemePreviewInfobar(extension);
     NotificationService::current()->Notify(
         NotificationType::THEME_INSTALLED,
         Source<ExtensionsService>(this),
@@ -368,23 +361,6 @@ void ExtensionsService::SetProviderForTesting(
       location, test_provider));
 }
 
-bool ExtensionsService::ShowThemePreviewInfobar(Extension* extension) {
-  if (!profile_)
-    return false;
-
-  Browser* browser = BrowserList::GetLastActiveWithProfile(profile_);
-  if (!browser)
-    return false;
-
-  TabContents* tab_contents = browser->GetSelectedTabContents();
-  if (!tab_contents)
-    return false;
-
-  tab_contents->AddInfoBar(new ThemePreviewInfobarDelegate(tab_contents,
-                                                           extension->name()));
-  return true;
-}
-
 void ExtensionsService::OnExternalExtensionFound(const std::string& id,
                                                  const std::string& version,
                                                  const FilePath& path,
@@ -409,12 +385,11 @@ void ExtensionsService::OnExternalExtensionFound(const std::string& id,
     }
   }
 
-  new CrxInstaller(path, install_directory_, location, id, extensions_enabled_,
-                   false,  // not from gallery
-                   show_extensions_prompts(),
-                   false,  // don't delete crx when complete
-                   backend_loop_,
-                   this);
+  CrxInstaller::Start(path, install_directory_, location, id,
+                      false,  // don't delete crx when complete
+                      backend_loop_,
+                      this,
+                      NULL);  // no client (silent install)
 }
 
 
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index 67f7c4a..640700e 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -89,6 +89,8 @@ class ExtensionsService
     return &extensions_;
   }
 
+  const FilePath& install_directory() const { return install_directory_; }
+
   // Initialize and start all installed extensions.
   void Init();
 
@@ -96,16 +98,14 @@ class ExtensionsService
   // update if an older version is already installed.
   // For fresh installs, this method also causes the extension to be
   // immediately loaded.
+  // TODO(aa): This method can be removed. It is only used by the unit tests,
+  // and they could use CrxInstaller directly instead.
   void InstallExtension(const FilePath& extension_path);
 
-  // XXX Hack: This is a temporary nasty hack to get theme installation working
-  // without a dialog. Will be fixed by making ExtensionsService more modular.
-  void InstallExtension(const FilePath& extension_path,
-                        const GURL& download_url,
-                        const GURL& referrer_url);
-
   // Updates a currently-installed extension with the contents from
   // |extension_path|.
+  // TODO(aa): This method can be removed. ExtensionUpdater could use
+  // CrxInstaller directly instead.
   virtual void UpdateExtension(const std::string& id,
                                const FilePath& extension_path);
 
@@ -195,12 +195,6 @@ class ExtensionsService
   bool is_ready() { return ready_; }
 
  private:
-  // Show a confirm installation infobar on the currently active tab.
-  // TODO(aa): This should be moved up into the UI and attached to the tab it
-  // actually occured in. This requires some modularization of
-  // ExtensionsService.
-  bool ShowThemePreviewInfobar(Extension* extension);
-
   // The profile this ExtensionsService is part of.
   Profile* profile_;
 
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
index 8048ff2..46a8382 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
@@ -28,11 +28,15 @@ const char SandboxedExtensionUnpacker::kExtensionHeaderMagic[] = "Cr24";
 SandboxedExtensionUnpacker::SandboxedExtensionUnpacker(
     const FilePath& crx_path, ResourceDispatcherHost* rdh,
     SandboxedExtensionUnpackerClient* client)
-      : crx_path_(crx_path), client_loop_(MessageLoop::current()), rdh_(rdh),
-        client_(client), got_response_(false) {
+      : crx_path_(crx_path), file_loop_(NULL), rdh_(rdh), client_(client),
+        got_response_(false) {
 }
 
 void SandboxedExtensionUnpacker::Start() {
+  // We assume that we are started on the thread that the client wants us to do
+  // file IO on.
+  file_loop_ = MessageLoop::current();
+
   // Create a temporary directory to work in.
   if (!temp_dir_.CreateUniqueTempDir()) {
     ReportFailure("Could not create temporary directory.");
@@ -72,13 +76,13 @@ void SandboxedExtensionUnpacker::Start() {
 
 void SandboxedExtensionUnpacker::StartProcessOnIOThread(
     const FilePath& temp_crx_path) {
-  UtilityProcessHost* host = new UtilityProcessHost(rdh_, this,
-                                                    MessageLoop::current());
+  UtilityProcessHost* host = new UtilityProcessHost(rdh_, this, file_loop_);
   host->StartExtensionUnpacker(temp_crx_path);
 }
 
 void SandboxedExtensionUnpacker::OnUnpackExtensionSucceeded(
     const DictionaryValue& manifest) {
+  DCHECK(file_loop_ == MessageLoop::current());
   got_response_ = true;
 
   ExtensionUnpacker::DecodedImages images;
@@ -164,6 +168,7 @@ void SandboxedExtensionUnpacker::OnUnpackExtensionSucceeded(
 
 void SandboxedExtensionUnpacker::OnUnpackExtensionFailed(
     const std::string& error) {
+  DCHECK(file_loop_ == MessageLoop::current());
   got_response_ = true;
   ReportFailure(error);
 }
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.h b/chrome/browser/extensions/sandboxed_extension_unpacker.h
index 02b34fb..e6fe25c 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.h
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.h
@@ -124,7 +124,7 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client {
   void ReportSuccess();
 
   FilePath crx_path_;
-  MessageLoop* client_loop_;
+  MessageLoop* file_loop_;
   ResourceDispatcherHost* rdh_;
   scoped_refptr<SandboxedExtensionUnpackerClient> client_;
   ScopedTempDir temp_dir_;
diff --git a/chrome/browser/extensions/theme_preview_infobar_delegate.cc b/chrome/browser/extensions/theme_preview_infobar_delegate.cc
index 75c1ce8..d22ebda 100644
--- a/chrome/browser/extensions/theme_preview_infobar_delegate.cc
+++ b/chrome/browser/extensions/theme_preview_infobar_delegate.cc
@@ -16,19 +16,6 @@ ThemePreviewInfobarDelegate::ThemePreviewInfobarDelegate(
            profile_(tab_contents->profile()), name_(name) {
 }
 
-bool ThemePreviewInfobarDelegate::EqualsDelegate(InfoBarDelegate* delegate)
-    const {
-  // If another infobar of this type is showing, this will prevent us adding
-  // a new one, we only care if they're the same type, as pressing undo always
-  // has the same result each time. This does mean that the text continues
-  // to refer to the old theme, but this is good enough for beta.
-  // http://crbug.com/17932
-  if (delegate->AsThemePreviewInfobarDelegate())
-    return true;
-
-  return false;
-}
-
 void ThemePreviewInfobarDelegate::InfoBarClosed() {
   delete this;
 }
diff --git a/chrome/browser/extensions/theme_preview_infobar_delegate.h b/chrome/browser/extensions/theme_preview_infobar_delegate.h
index 0d2b564..54a8507 100644
--- a/chrome/browser/extensions/theme_preview_infobar_delegate.h
+++ b/chrome/browser/extensions/theme_preview_infobar_delegate.h
@@ -18,7 +18,6 @@ class ThemePreviewInfobarDelegate : public ConfirmInfoBarDelegate {
  public:
   ThemePreviewInfobarDelegate(TabContents* tab_contents,
                               const std::string& name);
-  virtual bool EqualsDelegate(InfoBarDelegate* delegate) const;
   virtual void InfoBarClosed();
   virtual std::wstring GetMessageText() const;
   virtual SkBitmap* GetIcon() const;