Make use of the list of DLLs returned by each BrowserDistribution instance, rather than the hard-coded list built at startup.

BUG=61609
TEST=Chrome Frame and CEEE continue to be registered normally.

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

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

3 files changed, 90 insertions, 55 deletions

diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc
index 90f7361..880f3e1 100644
--- a/chrome/installer/setup/install.cc
+++ b/chrome/installer/setup/install.cc
@@ -6,8 +6,10 @@
 
 #include <shlobj.h>
 #include <time.h>
+#include <vector>
 
 #include "base/command_line.h"
+#include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/path_service.h"
@@ -383,30 +385,53 @@ bool CreateOrUpdateChromeShortcuts(const FilePath& setup_path,
   return ret;
 }
 
-bool RegisterComDlls(const Package& install,
-                     const Version* current_version,
+// Local helper to call RegisterComDllList for all DLLs in a set of products
+// managed by a given package.
+bool RegisterComDlls(const Package& package,
+                     const Version* old_version,
                      const Version& new_version) {
-  // TODO(tommi): setup.exe should always have at least one DLL to register.
-  // Currently we rely on scan_server_dlls.py to populate the array for us,
-  // but we might as well use an explicit static array of required components.
-  if (kNumDllsToRegister <= 0) {
-    NOTREACHED() << "no dlls to register";
-    return false;
+  // First collect the list of DLLs to be registered from each product.
+  const Products& products = package.products();
+  Products::const_iterator product_iter(products.begin());
+  std::vector<FilePath> com_dll_list;
+  for (; product_iter != products.end(); ++product_iter) {
+    BrowserDistribution* dist = product_iter->get()->distribution();
+    std::vector<FilePath> dist_dll_list(dist->GetComDllList());
+    std::copy(dist_dll_list.begin(), dist_dll_list.end(),
+              std::back_inserter(com_dll_list));
   }
 
-  // Unregister DLLs that were left from the old version that is being upgraded.
-  if (current_version) {
-    FilePath old_dll_path(install.path().Append(
-        UTF8ToWide(current_version->GetString())));
-    // Ignore failures to unregister old DLLs.
-    installer::RegisterComDllList(old_dll_path, install.system_level(), false,
-                                  false);
+  // Then, if we got some, attempt to unregister the DLLs from the old
+  // version directory and then re-register them in the new one.
+  // Note that if we are migrating the install directory then we will not
+  // successfully unregister the old DLLs.
+  // TODO(robertshield): See whether we need to fix the migration case.
+  // TODO(robertshield): If we ever remove a DLL from a product, this will
+  // not unregister it on update. We should build the unregistration list from
+  // saved state instead of assuming it is the same as the registration list.
+  bool success = true;
+  if (!com_dll_list.empty()) {
+    if (old_version) {
+      FilePath old_dll_path(
+          package.path().Append(UTF8ToWide(old_version->GetString())));
+
+      installer::RegisterComDllList(old_dll_path,
+                                    com_dll_list,
+                                    package.system_level(),
+                                    false,   // Unregister
+                                    false);  // Ignore failures
+    }
+
+    FilePath dll_path(
+        package.path().Append(UTF8ToWide(new_version.GetString())));
+    success = installer::RegisterComDllList(dll_path,
+                                            com_dll_list,
+                                            package.system_level(),
+                                            true,   // Register
+                                            true);  // Rollback on failure.
   }
 
-  FilePath dll_path(install.path().Append(
-      UTF8ToWide(new_version.GetString())));
-  return installer::RegisterComDllList(dll_path, install.system_level(), true,
-                                       true);
+  return success;
 }
 
 // After a successful copying of all the files, this function is called to
@@ -507,11 +532,9 @@ bool DoPostInstallTasks(bool multi_install,
     }
   }
 
-  if (FindProduct(products, BrowserDistribution::CHROME_FRAME)) {
-    if (!RegisterComDlls(package, current_version, new_version)) {
-      LOG(ERROR) << "RegisterComDlls failed.  Aborting.";
-      return false;
-    }
+  if (!RegisterComDlls(package, current_version, new_version)) {
+    LOG(ERROR) << "RegisterComDlls failed.  Aborting.";
+    return false;
   }
 
   // If we're told that we're an MSI install, make sure to set the marker
@@ -829,9 +852,16 @@ installer::InstallStatus InstallOrUpdateChrome(
   return result;
 }
 
-bool RegisterComDllList(const FilePath& dll_folder, bool system_level,
-                        bool do_register, bool rollback_on_failure) {
-  bool success = false;
+bool RegisterComDllList(const FilePath& dll_folder,
+                        const std::vector<FilePath>& dll_list,
+                        bool system_level,
+                        bool do_register,
+                        bool rollback_on_failure) {
+  if (dll_list.empty()) {
+    VLOG(1) << "No COM DLLs to register";
+    return true;
+  }
+
   scoped_ptr<WorkItemList> work_item_list;
   if (rollback_on_failure) {
     work_item_list.reset(WorkItem::CreateWorkItemList());
@@ -839,21 +869,20 @@ bool RegisterComDllList(const FilePath& dll_folder, bool system_level,
     work_item_list.reset(WorkItem::CreateNoRollbackWorkItemList());
   }
 
-  // TODO(robertshield): What if the list of old dlls and new ones isn't
-  // the same?  I (elmo) think we should start storing the list of DLLs
-  // somewhere.
-  if (InstallUtil::BuildDLLRegistrationList(dll_folder.value(), kDllsToRegister,
-                                            kNumDllsToRegister, do_register,
-                                            !system_level,
-                                            work_item_list.get())) {
-    // Ignore failures to unregister old DLLs.
-    success = work_item_list->Do();
-    if (!success && rollback_on_failure) {
-      work_item_list->Rollback();
-    }
+  std::vector<FilePath>::const_iterator dll_iter(dll_list.begin());
+  for (; dll_iter != dll_list.end(); ++dll_iter) {
+    FilePath dll_path = dll_folder.Append(*dll_iter);
+    work_item_list->AddSelfRegWorkItem(dll_path.value(),
+                                       do_register,
+                                       !system_level);
   }
 
+  bool success = work_item_list->Do();
+  if (!success && rollback_on_failure) {
+    work_item_list->Rollback();
+  }
   return success;
 }
 
+
 }  // namespace installer
diff --git a/chrome/installer/setup/install.h b/chrome/installer/setup/install.h
index 7310e4d..5b5a4d9 100644
--- a/chrome/installer/setup/install.h
+++ b/chrome/installer/setup/install.h
@@ -17,6 +17,7 @@
 #include "chrome/installer/util/util_constants.h"
 
 class DictionaryValue;
+class FilePath;
 
 namespace installer {
 
@@ -45,13 +46,19 @@ installer::InstallStatus InstallOrUpdateChrome(
     const installer::MasterPreferences& prefs, const Version& new_version,
     const Package& package);
 
-// Registers or unregisters COM DLLs in a specific folder as declared in
-// kDllsToRegister.
-// TODO(robertshield): What if the list of old dlls and new ones isn't
-// the same?  I think we should start storing the list of DLLs somewhere as
-// part of the installation data.
-bool RegisterComDllList(const FilePath& dll_folder, bool system_level,
-                        bool do_register, bool rollback_on_failure);
+// Registers or unregisters the COM DLLs whose file names are given in
+// |dll_files| and who reside in the path |dll_folder|.
+// |system_level| specifies whether to call the system or user level DLL
+// registration entry points.
+// |do_register| says whether to register or unregister.
+// |rollback_on_failure| says whether we should try to revert the registration
+// or unregistration by calling the opposite entry point on the DLLs if
+// something goes wrong.
+bool RegisterComDllList(const FilePath& dll_folder,
+                        const std::vector<FilePath>& dll_files,
+                        bool system_level,
+                        bool do_register,
+                        bool rollback_on_failure);
 
 }  // namespace installer
 
diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index 72d6f00..b5b1986 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -598,15 +598,14 @@ InstallStatus UninstallChrome(const FilePath& setup_path,
       hklm_key.Close();
     }
 
-    // Unregister any dll servers that we may have registered for Chrome Frame
-    // and CEEE builds only.
-    // TODO(tommi): We should only do this when the folder itself is
-    // being removed and we know that the DLLs were previously registered.
-    // Simplest would be to always register them.
-    if (installed_version != NULL && !is_chrome) {
-      RegisterComDllList(product.package().path().Append(
-          UTF8ToWide(installed_version->GetString())), product.system_level(),
-          false, false);
+    // Unregister any dll servers that we may have registered for this
+    // product.
+    if (installed_version != NULL) {
+      std::vector<FilePath> com_dll_list(browser_dist->GetComDllList());
+      FilePath dll_folder = product.package().path().Append(
+          UTF8ToWide(installed_version->GetString()));
+      RegisterComDllList(dll_folder, com_dll_list, product.system_level(),
+                         false, false);
     }
   }