Create VisualElementsManifest.xml from template -- install VisualElements in the version directory

chrome/installer/setup/install_worker.cc

Index: chrome/installer/setup/install_worker.cc
diff --git a/chrome/installer/setup/install_worker.cc b/chrome/installer/setup/install_worker.cc
index cd17feea47e150abf09e29bb9bc847b780027d0f..f3322b2c017a4c53f9fa5971c3bd791399171a8e 100644
--- a/chrome/installer/setup/install_worker.cc
+++ b/chrome/installer/setup/install_worker.cc
@@ -10,6 +10,8 @@
 #include <oaidl.h>
 #include <shlobj.h>
 #include <time.h>
+
+#include <string>
 #include <vector>
 
 #include "base/command_line.h"
@@ -17,7 +19,9 @@
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/path_service.h"
+#include "base/string16.h"
 #include "base/string_util.h"
+#include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "base/version.h"
 #include "base/win/registry.h"
@@ -42,6 +46,85 @@
 
 using base::win::RegKey;
 
+namespace {
+
+// Elements that make up install target paths.
+const wchar_t kVisualElementsManifest[] = L"VisualElementsManifest.xml";
+const wchar_t kWowHelperExe[] = L"wow_helper.exe";
+
+// Adds work items to |install_list| to lay down a version specific
+// VisualElementsManifest.xml beside chrome.exe.
+void AddVisualElementsInstallWorkItems(const FilePath& src_path,
+                                       const FilePath& target_path,
+                                       const FilePath& temp_path,
+                                       const Version& new_version,
+                                       WorkItemList* install_list) {
+  DCHECK(install_list);
+
+  // Construct the relative path to the Images directory.

[grt (UTC plus 2), 2012/05/02 15:09:44]

"Images" -> "versioned VisualElements"

[gab, 2012/05/02 20:55:58]

Done. (I Know patch set 7 says "version" --> fixed for "versioned" in patch set
8).

+  string16 images_dir(ASCIIToUTF16(new_version.GetString()));

[grt (UTC plus 2), 2012/05/02 15:09:44]

images_dir -> elements_dir

[gab, 2012/05/02 20:55:58]

Done.

+  images_dir.push_back(FilePath::kSeparators[0]);
+  images_dir.append(L"VisualElements");
+
+  // Some distributions of Chromium may not include visual elements. Only
+  // proceed if this distribution does.
+  if (!file_util::PathExists(src_path.Append(images_dir))) {
+    VLOG(1) << "No visual elements found, skipping related work items.";
+  } else {
+    // A printf_p-style format string for generating the visual elements
+    // manifest. Required arguments, in order, are:
+    //   - Localized display name for the product.
+    //   - Relative path to the Images directory.

[grt (UTC plus 2), 2012/05/02 15:09:44]

Images -> VisualElements

[gab, 2012/05/02 20:55:58]

Done.

+    static const char manifest_template[] =

[grt (UTC plus 2), 2012/05/02 15:09:44]

manifest_template -> kManifestTemplate

[gab, 2012/05/02 20:55:58]

Done.

+        "<Application>\r\n"
+        "  <VisualElements\r\n"
+        "      DisplayName='%1$ls'\r\n"
+        "      Logo='%2$ls\\Logo.png'\r\n"
+        "      SmallLogo='%2$ls\\SmallLogo.png'\r\n"
+        "      ForegroundText='light'\r\n"
+        "      BackgroundColor='white'>\r\n"
+        "    <DefaultTile ShowName='allLogos'/>\r\n"
+        "    <SplashScreen Image='%2$ls\\splash-620x300.png'/>\r\n"
+        "  </VisualElements>\r\n"
+        "</Application>";
+
+    // Fill the manifest with the desired values.
+    const string16 manifest_template16 = ASCIIToUTF16(manifest_template);

[grt (UTC plus 2), 2012/05/02 15:09:44]

manifest_template(ASCIIToUTF16(kManifestTemplate))

[gab, 2012/05/02 20:55:58]

Arg! I always forget about this style rule!

+    // TODO(grt): http://crbug.com/75152 Write a reference to a localized

[grt (UTC plus 2), 2012/05/02 15:13:09]

please move this comment down one line so that it immediately precedes the
definition of display_name.

[gab, 2012/05/02 20:55:58]

Done.

+    // resource for |display_name|.
+    BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+    string16 display_name = dist->GetAppShortCutName();

[grt (UTC plus 2), 2012/05/02 15:09:44]

display_name(dist->GetAppShortCutName())

[gab, 2012/05/02 20:55:58]

Done.

+    // XMLEscape |display_name| to be safe. We do not need to escape double

[grt (UTC plus 2), 2012/05/02 15:09:44]

// Escape the display name as per the XML AttValue production
// (http://www.w3.org/TR/2008/REC-xml-20081126/#NT-AttValue) for a value in
// single quotes.

[gab, 2012/05/02 20:55:58]

I like. Done.

+    // quotes as this will be written within single quotes. We can also avoid
+    // escaping '>' as it is a legal character in an XML attribute value.
+    ReplaceChars(display_name, L"&", string16(L"&amp"), &display_name);

[grt (UTC plus 2), 2012/05/02 15:09:44]

string16(L"&amp") -> L"&"

[gab, 2012/05/02 20:55:58]

I was under the impression that Chromium style didn't like implicit cast, but I
guess that's only when making new classes we want to make sure they have
explicit single-param constructors, but that we are allowed to use currently
existing implicit casts?

Done.

[grt (UTC plus 2), 2012/05/03 17:49:37]

yeah.  iirc, the guidance is not to introduce implicit ctors unless there's a
really compelling reason to do so.  if there is one in a class you're using, i
think you're free to use it.

+    ReplaceChars(display_name, L"'", string16(L"&apos"), &display_name);

[grt (UTC plus 2), 2012/05/02 15:09:44]

string16(L"&apos") -> L"'"

[gab, 2012/05/02 20:55:58]

Done.

+    ReplaceChars(display_name, L"<", string16(L"&lt"), &display_name);

[grt (UTC plus 2), 2012/05/02 15:09:44]

string16(L"&lt") -> L"<"

[gab, 2012/05/02 20:55:58]

Done.

+    string16 manifest16 = base::StringPrintf(manifest_template16.c_str(),

[grt (UTC plus 2), 2012/05/02 15:09:44]

manifest16(base::StringPrintf(...))

[gab, 2012/05/02 20:55:58]

Done.

+                                             display_name.c_str(),
+                                             images_dir.c_str());
+
+    // Write the manifest to the directory where the archive was extracted.
+    const std::string manifest = UTF16ToUTF8(manifest16);

[grt (UTC plus 2), 2012/05/02 15:09:44]

manifest(UTF...

[gab, 2012/05/02 20:55:58]

Done :).

+    if (!file_util::WriteFile(src_path.Append(kVisualElementsManifest),
+                              manifest.c_str(), manifest.size())) {
+      LOG(ERROR) << "Error writing " << kVisualElementsManifest

[grt (UTC plus 2), 2012/05/02 15:09:44]

PLOG(ERROR)

[gab, 2012/05/02 20:55:58]

Right, done.

+                 << "to" << src_path.value();
+      // TODO(gab) RETURN FALSE WHEN THE FUNCTION HAS A RETURN :)!
+      return;
+    }
+
+    // Add a work item to move the new manifest to |target_path|.
+    install_list->AddMoveTreeWorkItem(
+        src_path.Append(kVisualElementsManifest).value(),
+        target_path.Append(kVisualElementsManifest).value(),
+        temp_path.value(),
+        WorkItem::ALWAYS_MOVE);
+  }
+}
+
+}  // namespace
+
 namespace installer {
 
 // Local helper to call AddRegisterComDllWorkItems for all DLLs in a set of
@@ -769,41 +852,15 @@ void AddInstallWorkItems(const InstallationState& original_state,
       base::win::OSInfo::WOW64_DISABLED &&
       base::win::GetVersion() <= base::win::VERSION_VISTA) {
     install_list->AddMoveTreeWorkItem(
-        src_path.Append(installer::kWowHelperExe).value(),
-        target_path.Append(installer::kWowHelperExe).value(),
+        src_path.Append(kWowHelperExe).value(),
+        target_path.Append(kWowHelperExe).value(),
         temp_path.value(),
         WorkItem::ALWAYS_MOVE);
   }
 
   if (base::win::GetVersion() >= base::win::VERSION_WIN8) {
-    // Desktop only (i.e. not supporting Metro) versions of Chromium do not have
-    // to include visual elements.
-    scoped_ptr<WorkItemList> win8_work_items(
-        WorkItem::CreateConditionalWorkItemList(new ConditionRunIfFileExists(
-            src_path.Append(L"visualelementsmanifest.xml"))));
-    // TODO (gab): All of these hard-coded strings are temporary and will be
-    // deleted in the patch following this one.
-    win8_work_items->AddMoveTreeWorkItem(
-        src_path.Append(L"visualelementsmanifest.xml").value(),
-        target_path.Append(L"visualelementsmanifest.xml").value(),
-        temp_path.value(),
-        WorkItem::ALWAYS_MOVE);
-    win8_work_items->AddMoveTreeWorkItem(
-        src_path.Append(L"logo.png").value(),
-        target_path.Append(L"logo.png").value(),
-        temp_path.value(),
-        WorkItem::ALWAYS_MOVE);
-    win8_work_items->AddMoveTreeWorkItem(
-        src_path.Append(L"smalllogo.png").value(),
-        target_path.Append(L"smalllogo.png").value(),
-        temp_path.value(),
-        WorkItem::ALWAYS_MOVE);
-    win8_work_items->AddMoveTreeWorkItem(
-        src_path.Append(L"splash-620x300.png").value(),
-        target_path.Append(L"splash-620x300.png").value(),
-        temp_path.value(),
-        WorkItem::ALWAYS_MOVE);
-    install_list->AddWorkItem(win8_work_items.release());
+    AddVisualElementsInstallWorkItems(src_path, target_path, temp_path,
+                                      new_version, install_list);
   }
 
   // In the past, we copied rather than moved for system level installs so that
@@ -822,14 +879,6 @@ void AddInstallWorkItems(const InstallationState& original_state,
       check_for_duplicates ? WorkItem::CHECK_DUPLICATES :
                              WorkItem::ALWAYS_MOVE);
 
-  // Copy the default Dictionaries only if the folder doesn't exist already.
-  // TODO(grt): Use AddMoveTreeWorkItem in a conditional WorkItemList, which
-  // will be more efficient in space and time.
-  install_list->AddCopyTreeWorkItem(
-      src_path.Append(installer::kDictionaries).value(),
-      target_path.Append(installer::kDictionaries).value(),
-      temp_path.value(), WorkItem::IF_NOT_PRESENT);
-
   // Delete any old_chrome.exe if present (ignore failure if it's in use).
   install_list->AddDeleteTreeWorkItem(
       target_path.Append(installer::kChromeOldExe), temp_path)