Add support for component builds in alternate_version_generator.

chrome/installer/test/alternate_version_generator.cc

Index: chrome/installer/test/alternate_version_generator.cc
diff --git a/chrome/installer/test/alternate_version_generator.cc b/chrome/installer/test/alternate_version_generator.cc
index bf492c4cb7b291c7277ebedae8e348a96decc0db..a76a06cffc6b2d353aa19851861a749c2ca31687 100644
--- a/chrome/installer/test/alternate_version_generator.cc
+++ b/chrome/installer/test/alternate_version_generator.cc
@@ -60,7 +60,8 @@ const wchar_t k7zaPathRelative[] = L"..\\..\\third_party\\lzma_sdk\\Executable";
 const wchar_t kB7[] = L"B7";
 const wchar_t kBl[] = L"BL";
 const wchar_t kChromeBin[] = L"Chrome-bin";
-const wchar_t kChromePacked7z[] = L"chrome.packed.7z";
+const wchar_t kChromePacked7z[] = L"CHROME.PACKED.7Z";
+const wchar_t kChrome7z[] = L"CHROME.7Z";
 const wchar_t kExe[] = L"exe";
 const wchar_t kExpandExe[] = L"expand.exe";
 const wchar_t kExtDll[] = L".dll";
@@ -130,6 +131,8 @@ class ChromeVersion {
   ULONGLONG value() const { return version_; }
   void set_value(ULONGLONG value) { version_ = value; }
   std::wstring ToString() const;
+  std::string ToASCII() const;
+
  private:
   ULONGLONG version_;
 };  // class ChromeVersion
@@ -144,6 +147,14 @@ std::wstring ChromeVersion::ToString() const {
   return std::wstring(&buffer[0], string_len);
 }
 
+std::string ChromeVersion::ToASCII() const {
+  char buffer[24];
+  int string_len = sprintf_s(&buffer[0], arraysize(buffer), "%hu.%hu.%hu.%hu",
+                             major(), minor(), build(), patch());
+  DCHECK_NE(-1, string_len);
+  DCHECK_GT(static_cast<int>(arraysize(buffer)), string_len);
+  return std::string(&buffer[0], string_len);
+}
 
 // A read/write mapping of a file.
 // Note: base::MemoryMappedFile is not used because it doesn't support
@@ -320,6 +331,7 @@ struct VisitResourceContext {
 // Replaces the old version with the new in a resource.  A first pass is made to
 // replace the string form (e.g., "9.0.584.0").  If any replacements are made, a
 // second pass is made to replace the binary form (e.g., 0x0000024800000009).
+// A final pass is made to replace the ASCII string form.
 void VisitResource(const upgrade_test::EntryPath& path,
                    uint8_t* data,
                    DWORD size,
@@ -337,7 +349,7 @@ void VisitResource(const upgrade_test::EntryPath& path,
           reinterpret_cast<const uint8_t*>(ctx.new_version_str.c_str()),
           &changing_version) &&
       changing_version) {
-    // Replace all occurrences of current_version with new_version
+    // Replace all binary occurrences of current_version with new_version.
     struct VersionPair {
       DWORD high;
       DWORD low;
@@ -352,6 +364,13 @@ void VisitResource(const upgrade_test::EntryPath& path,
                reinterpret_cast<const uint8_t*>(&cur_ver) + sizeof(cur_ver),
                reinterpret_cast<const uint8_t*>(&new_ver), NULL);
   }
+
+  // Replace all ASCII occurrences of current_version with new_version.
+  std::string old_version(ctx.current_version.ToASCII());

[gab, 2016/01/25 21:00:12]

s/old_version/current_version/ for consistency

[grt (UTC plus 2), 2016/01/26 00:13:45]

Done.

+  std::string new_version(ctx.new_version.ToASCII());
+  ReplaceAll(data, data + size, reinterpret_cast<uint8_t*>(&old_version[0]),
+             reinterpret_cast<uint8_t*>(&old_version[old_version.size()]),
+             reinterpret_cast<uint8_t*>(&new_version[0]), NULL);
 }
 
 // Updates the version strings and numbers in all of |image_file|'s resources.
@@ -398,6 +417,26 @@ bool UpdateVersionIfMatch(const base::FilePath& image_file,
   return result;
 }
 
+bool UpdateManifestVersion(const base::FilePath& manifest,
+                           VisitResourceContext* context) {
+  std::string contents;
+  if (!base::ReadFileToString(manifest, &contents))
+    return false;
+  std::string old_version(context->current_version.ToASCII());
+  std::string new_version(context->new_version.ToASCII());
+  bool modified = false;
+  if (!ReplaceAll(reinterpret_cast<uint8_t*>(&contents[0]),
+                  reinterpret_cast<uint8_t*>(&contents[contents.size()]),
+                  reinterpret_cast<uint8_t*>(&old_version[0]),
+                  reinterpret_cast<uint8_t*>(&old_version[old_version.size()]),

[gab, 2016/01/25 21:00:12]

Would reinterpret_cast<uint8_t*>(old_version.end()) be valid? Feels using
begin()/end() would be cleaner if possible..?

[grt (UTC plus 2), 2016/01/26 00:13:45]

I don't think so. While the iterator must act like a pointer in that operator*
would "dereference" it, the iterator itself is likely a lot more than just a
pointer, so the typecast would lead to badness.

+                  reinterpret_cast<uint8_t*>(&new_version[0]), &modified)) {
+    return false;
+  }
+  DCHECK(modified);
+  return base::WriteFile(manifest, &contents[0], contents.size()) ==
+         contents.size();
+}
+
 bool IncrementNewVersion(upgrade_test::Direction direction,
                          VisitResourceContext* ctx) {
   DCHECK(ctx);
@@ -453,8 +492,21 @@ bool ApplyAlternateVersion(const base::FilePath& work_dir,
 
   // Change the versioned directory.
   base::FilePath chrome_bin = work_dir.Append(&kChromeBin[0]);
-  doing_great = base::Move(chrome_bin.Append(ctx.current_version_str),
-                           chrome_bin.Append(ctx.new_version_str));
+  if (doing_great) {

[gab, 2016/01/25 21:00:12]

If we don't do anything and just run to returning false at end when we stop
|doing_great|, why not instead return false on first failure and drop this
variable?

[grt (UTC plus 2), 2016/01/26 00:13:45]

Good point. I probably wrote this when I was allergic to early-exit. :-)

+    doing_great = base::Move(chrome_bin.Append(ctx.current_version_str),
+                             chrome_bin.Append(ctx.new_version_str));
+  }
+
+  // Update the manifest.
+  base::FilePath current_manifest =
+      chrome_bin.Append(ctx.new_version_str)
+          .Append(ctx.current_version_str + L".manifest");

[gab, 2016/01/25 21:00:12]

Please link this CL to http://crbug.com/581133 which I just filed so that we
know there is potential cleanup here when we get rid of the version manifest.

[grt (UTC plus 2), 2016/01/26 00:13:45]

Done.

+  if (doing_great && base::PathExists(current_manifest)) {
+    base::FilePath new_manifest =
+        current_manifest.DirName().Append(ctx.new_version_str + L".manifest");
+    doing_great = base::Move(current_manifest, new_manifest) &&
+                  UpdateManifestVersion(new_manifest, &ctx);
+  }
 
   if (doing_great) {
     // Report the version numbers if requested.
@@ -537,8 +589,10 @@ bool GenerateAlternateVersion(const base::FilePath& original_installer_path,
   }
 
   base::FilePath setup_ex_ = work_dir.directory().Append(&kSetupEx_[0]);
-  base::FilePath chrome_packed_7z =
-      work_dir.directory().Append(&kChromePacked7z[0]);
+  base::FilePath chrome_packed_7z;  // Empty for component builds.
+  base::FilePath chrome_7z;
+  const wchar_t* archive_resource_name = nullptr;
+  base::FilePath* archive_file = nullptr;
   // Load the original file and extract setup.ex_ and chrome.packed.7z
   {
     ResourceLoader resource_loader;
@@ -559,15 +613,23 @@ bool GenerateAlternateVersion(const base::FilePath& original_installer_path,
       return false;
     }
 
-    // Write out chrome.packed.7z
-    if (!resource_loader.Load(&kChromePacked7z[0], &kB7[0], &resource_data))
+    // Write out chrome.packed.7z (static build) or chrome.7z (component build)
+    if (resource_loader.Load(&kChromePacked7z[0], &kB7[0], &resource_data)) {
+      archive_resource_name = &kChromePacked7z[0];
+      chrome_packed_7z = work_dir.directory().Append(archive_resource_name);
+      archive_file = &chrome_packed_7z;
+    } else if (resource_loader.Load(&kChrome7z[0], &kB7[0], &resource_data)) {
+      archive_resource_name = &kChrome7z[0];
+      chrome_7z = work_dir.directory().Append(archive_resource_name);
+      archive_file = &chrome_7z;
+    } else {
       return false;
-    written =
-        base::WriteFile(chrome_packed_7z,
-                        reinterpret_cast<const char*>(resource_data.first),
-                        static_cast<int>(resource_data.second));
+    }

[gab, 2016/01/25 21:00:12]

DCHECK(!archive_resource_name.empty());
DCHECK(!chrome_packed_7z.empty() || !chrome_7z.empty();
DCHECK(archive_file);

to document the expectations at this point.

[grt (UTC plus 2), 2016/01/26 00:13:45]

Done.

+    written = base::WriteFile(
+        *archive_file, reinterpret_cast<const char*>(resource_data.first),
+        static_cast<int>(resource_data.second));
     if (written != static_cast<int>(resource_data.second)) {
-      LOG(DFATAL) << "Failed writing \"" << chrome_packed_7z.value() << "\"";
+      LOG(DFATAL) << "Failed writing \"" << archive_file->value() << "\"";
       return false;
     }
   }
@@ -590,26 +652,29 @@ bool GenerateAlternateVersion(const base::FilePath& original_installer_path,
     return false;
   }
 
-  // Unpack chrome.packed.7z
-  std::wstring chrome_7z_name;
-  if (LzmaUtil::UnPackArchive(chrome_packed_7z.value(),
-                              work_dir.directory().value(),
-                              &chrome_7z_name) != NO_ERROR) {
-    LOG(DFATAL) << "Failed unpacking \"" << chrome_packed_7z.value() << "\"";
-    return false;
+  // Unpack chrome.packed.7z (static build only).
+  if (!chrome_packed_7z.empty()) {
+    std::wstring chrome_7z_name;
+    if (LzmaUtil::UnPackArchive(chrome_packed_7z.value(),
+                                work_dir.directory().value(),
+                                &chrome_7z_name) != NO_ERROR) {
+      LOG(DFATAL) << "Failed unpacking \"" << chrome_packed_7z.value() << "\"";
+      return false;
+    }
+    chrome_7z = base::FilePath(chrome_7z_name);
   }

[gab, 2016/01/25 21:00:12]

DCHECK(!chrome_7z.empty());

to document the expectations at this point

[grt (UTC plus 2), 2016/01/26 00:13:45]

Done.

 
   // Unpack chrome.7z
-  if (LzmaUtil::UnPackArchive(chrome_7z_name, work_dir.directory().value(),
+  if (LzmaUtil::UnPackArchive(chrome_7z.value(), work_dir.directory().value(),
                               NULL) != NO_ERROR) {
-    LOG(DFATAL) << "Failed unpacking \"" << chrome_7z_name << "\"";
+    LOG(DFATAL) << "Failed unpacking \"" << chrome_7z.value() << "\"";
     return false;
   }
 
   // Get rid of intermediate files
-  base::FilePath chrome_7z(chrome_7z_name);
   if (!base::DeleteFile(chrome_7z, false) ||
-      !base::DeleteFile(chrome_packed_7z, false) ||
+      (!chrome_packed_7z.empty() &&
+       !base::DeleteFile(chrome_packed_7z, false)) ||
       !base::DeleteFile(setup_ex_, false)) {
     LOG(DFATAL) << "Failed deleting intermediate files";
     return false;
@@ -623,9 +688,11 @@ bool GenerateAlternateVersion(const base::FilePath& original_installer_path,
   if (!CreateArchive(chrome_7z, work_dir.directory().Append(&kChromeBin[0]), 0))
     return false;
 
-  // Compress chrome.7z into chrome.packed.7z
-  if (!CreateArchive(chrome_packed_7z, chrome_7z, 9))
+  // Compress chrome.7z into chrome.packed.7z for static builds.
+  if (!chrome_packed_7z.empty() &&
+      !CreateArchive(chrome_packed_7z, chrome_7z, 9)) {
     return false;
+  }
 
   // Compress setup.exe into setup.ex_
   command_line.assign(1, L'"')
@@ -641,16 +708,20 @@ bool GenerateAlternateVersion(const base::FilePath& original_installer_path,
     return false;
   }
 
-  // Replace the mini_installer's setup.ex_ and chrome.packed.7z resources.
+  // Replace the mini_installer's setup.ex_ and chrome.packed.7z (or chrome.7z
+  // in component builds) resources.
   ResourceUpdater updater;
   if (!updater.Initialize(mini_installer) ||
       !updater.Update(&kSetupEx_[0], &kBl[0],
                       MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US),
                       setup_ex_) ||
-      !updater.Update(&kChromePacked7z[0], &kB7[0],
+      !updater.Update(archive_resource_name, &kB7[0],
                       MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US),
-                      chrome_packed_7z) ||
+                      *archive_file) ||
       !updater.Commit()) {
+    LOG(ERROR) << "It is common for this step to fail for very large resources,"
+                  " as is the case for Debug component=shared_library builds. "
+                  "Try with a Release or component=static_library build.";
     return false;
   }