Remove uses of PropVariantTo*; removing the need to DelayLoad propsys.dll.

chrome/browser/shell_integration_win.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 "chrome/browser/shell_integration.h"
 
 #include <windows.h>
 #include <shobjidl.h>
 #include <propkey.h>
-#include <propvarutil.h>
-#include <tchar.h>
-#include <strsafe.h>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_comptr.h"
+#include "base/win/scoped_propvariant.h"
 #include "base/win/shortcut.h"
 #include "base/win/windows_version.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/installer/setup/setup_util.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/create_reg_key_work_item.h"
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/set_reg_value_work_item.h"
 #include "chrome/installer/util/shell_util.h"
 #include "chrome/installer/util/util_constants.h"
 #include "chrome/installer/util/work_item.h"
 #include "chrome/installer/util/work_item_list.h"
 #include "content/public/browser/browser_thread.h"
 
-// propsys.lib is required for PropvariantTo*().
-#pragma comment(lib, "propsys.lib")
-
 using content::BrowserThread;
 
 namespace {
 
 #if defined(GOOGLE_CHROME_BUILD)
 const wchar_t kAppListAppName[] = L"ChromeAppList";
 #else
 const wchar_t kAppListAppName[] = L"ChromiumAppList";
 #endif
 
@@ skipping 323 matching lines @@
   file_util::FileEnumerator shortcuts_enum(
       path, false,  // not recursive
       file_util::FileEnumerator::FILES, FILE_PATH_LITERAL("*.lnk"));
 
   bool is_per_user_install =
       InstallUtil::IsPerUserInstall(chrome_exe.value().c_str());
 
   int shortcuts_migrated = 0;
   FilePath target_path;
   string16 arguments;
-  string16 existing_app_id;
+  base::win::ScopedPropVariant pv_buffer;

[grt (UTC plus 2), 2013/01/10 16:05:20]

why buffer?

[gab, 2013/01/10 18:11:38]

Idk, random name choice? |propvariant| better?

   for (FilePath shortcut = shortcuts_enum.Next(); !shortcut.empty();
        shortcut = shortcuts_enum.Next()) {
     // TODO(gab): Use ProgramCompare instead of comparing FilePaths below once
     // it is fixed to work with FilePaths with spaces.
     if (!base::win::ResolveShortcut(shortcut, &target_path, &arguments) ||
         chrome_exe != target_path) {
       continue;
     }
     CommandLine command_line(CommandLine::FromString(base::StringPrintf(
         L"\"%ls\" %ls", target_path.value().c_str(), arguments.c_str())));
@@ skipping 14 matching lines @@
       DLOG(WARNING) << "Failed loading shortcut at " << shortcut.value();
       continue;
     }
 
     // Any properties that need to be updated on the shortcut will be stored in
     // |updated_properties|.
     base::win::ShortcutProperties updated_properties;
 
     // Validate the existing app id for the shortcut.
     base::win::ScopedComPtr<IPropertyStore> property_store;
-    PROPVARIANT pv_app_id;
-    PropVariantInit(&pv_app_id);
+    pv_buffer.Reset();
     if (FAILED(property_store.QueryFrom(shell_link)) ||
-        property_store->GetValue(PKEY_AppUserModel_ID, &pv_app_id) != S_OK) {
+        property_store->GetValue(PKEY_AppUserModel_ID,
+                                 pv_buffer.Receive()) != S_OK) {

[gab, 2013/01/10 15:30:42]

I was thinking it could be nice if we replaced Receive() by ResetAndReceive()
which would also guarantee we never leak and would prevent having to remember to
call Reset() early above.

This doesn't really go hand-in-hand with typical Scoped* classes in Chromium
though perhaps...

What do you think?

Something like:
  PROPVARIANT* ResetAndReceive() {
    if (pv_.vt != VT_EMPTY)
      Reset();
    return &pv_;
  }

[grt (UTC plus 2), 2013/01/10 16:05:20]

I think code consistency is more important so that readers don't need to
know/remember that this one is special for some reason.

[gab, 2013/01/10 18:11:38]

Ok that's what I thought too, but just checking. Thanks!

       // When in doubt, prefer not updating the shortcut.
       NOTREACHED();
       continue;
-    } else if (pv_app_id.vt == VT_EMPTY) {
-      // If there is no app_id set, set our app_id if one is expected.
-      if (!expected_app_id.empty())
-        updated_properties.set_app_id(expected_app_id);
     } else {
-      // Validate that the existing app_id is the expected app_id; if not, set
-      // the expected app_id on the shortcut.
-      size_t expected_size = expected_app_id.size() + 1;
-      HRESULT result = PropVariantToString(
-          pv_app_id, WriteInto(&existing_app_id, expected_size), expected_size);
-      PropVariantClear(&pv_app_id);
-      if (result != S_OK && result != STRSAFE_E_INSUFFICIENT_BUFFER) {
-        // Accept the STRSAFE_E_INSUFFICIENT_BUFFER error state as it means the
-        // existing appid is longer than |expected_app_id| and thus we will
-        // simply assume inequality.
-        NOTREACHED();
-        continue;
-      } else if (result == STRSAFE_E_INSUFFICIENT_BUFFER ||
-                 expected_app_id != existing_app_id) {
-        updated_properties.set_app_id(expected_app_id);
+      switch (pv_buffer.get().vt) {
+        case VT_EMPTY:
+          // If there is no app_id set, set our app_id if one is expected.
+          if (!expected_app_id.empty())
+            updated_properties.set_app_id(expected_app_id);
+          break;
+        case VT_LPWSTR:
+          if (expected_app_id != string16(pv_buffer.get().pwszVal))
+            updated_properties.set_app_id(expected_app_id);
+          break;
+        default:
+          NOTREACHED();
+          continue;
       }
     }
 
     if (check_dual_mode) {
-      BOOL existing_dual_mode;
-      PROPVARIANT pv_dual_mode;
-      PropVariantInit(&pv_dual_mode);
+      pv_buffer.Reset();
       if (property_store->GetValue(PKEY_AppUserModel_IsDualMode,
-                                   &pv_dual_mode) != S_OK) {
+                                   pv_buffer.Receive()) != S_OK) {
         // When in doubt, prefer to not update the shortcut.
         NOTREACHED();
         continue;
-      } else if (pv_dual_mode.vt == VT_EMPTY) {
-        // If dual_mode is not set at all, make sure it gets set to true.
-        updated_properties.set_dual_mode(true);
       } else {
-        // If it is set to false, make sure it gets set to true as well.
-        if (PropVariantToBoolean(pv_dual_mode, &existing_dual_mode) != S_OK) {
-          NOTREACHED();
-          continue;
+        switch (pv_buffer.get().vt) {
+          case VT_EMPTY:
+            // If dual_mode is not set at all, make sure it gets set to true.
+            updated_properties.set_dual_mode(true);
+            break;
+          case VT_BOOL:
+            // If it is set to false, make sure it gets set to true as well.
+            if (!pv_buffer.get().boolVal)
+              updated_properties.set_dual_mode(true);
+            break;
+          default:
+            NOTREACHED();
+            continue;
         }
-        PropVariantClear(&pv_dual_mode);
-        if (!existing_dual_mode)
-          updated_properties.set_dual_mode(true);
       }
     }
 
     persist_file.Release();
     shell_link.Release();
 
     // Update the shortcut if some of its properties need to be updated.
     if (updated_properties.options &&
         base::win::CreateOrUpdateShortcutLink(
             shortcut, updated_properties,
@@ skipping 24 matching lines @@
     }
 
     shortcut = shortcut.Append(shortcut_name).Append(shortcut_name +
                                                      installer::kLnkExt);
     if (file_util::PathExists(shortcut))
       return shortcut;
   }
 
   return FilePath();
 }