Use StringPiece in base::win::ScopedBstr to reduce strlens.

Also make a few gentle updates to the code.

BUG=none
TBR=isherman@chromium.org,pmonette@chromium.org,csharp@chromium.org,garykac@chromium.org

Change-Id: I328d6859024cbdc88c001920a9fe21cbfbb42cc8
Reviewed-on: https://chromium-review.googlesource.com/1189562
Commit-Queue: Greg Thompson <grt@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586664}

base/win/scoped_bstr.cc

@@ -11,26 +11,25 @@
 namespace base {
 namespace win {
 
-ScopedBstr::ScopedBstr(const char16* non_bstr)
-    : bstr_(SysAllocString(non_bstr)) {
-}
+ScopedBstr::ScopedBstr(StringPiece16 non_bstr)
+    : bstr_(::SysAllocStringLen(non_bstr.data(), non_bstr.length())) {}
 
 ScopedBstr::~ScopedBstr() {
   static_assert(sizeof(ScopedBstr) == sizeof(BSTR), "ScopedBstrSize");
-  SysFreeString(bstr_);
+  ::SysFreeString(bstr_);
 }
 
 void ScopedBstr::Reset(BSTR bstr) {
   if (bstr != bstr_) {
-    // if |bstr_| is NULL, SysFreeString does nothing.
-    SysFreeString(bstr_);
+    // SysFreeString handles null properly.
+    ::SysFreeString(bstr_);
     bstr_ = bstr;
   }
 }
 
 BSTR ScopedBstr::Release() {
   BSTR bstr = bstr_;
-  bstr_ = NULL;
+  bstr_ = nullptr;
   return bstr;
 }
 
@@ -45,28 +44,28 @@ BSTR* ScopedBstr::Receive() {
   return &bstr_;
 }
 
-BSTR ScopedBstr::Allocate(const char16* str) {
-  Reset(SysAllocString(str));
+BSTR ScopedBstr::Allocate(StringPiece16 str) {
+  Reset(::SysAllocStringLen(str.data(), str.length()));
   return bstr_;
 }
 
 BSTR ScopedBstr::AllocateBytes(size_t bytes) {
-  Reset(SysAllocStringByteLen(NULL, static_cast<UINT>(bytes)));
+  Reset(::SysAllocStringByteLen(nullptr, static_cast<UINT>(bytes)));
   return bstr_;
 }
 
 void ScopedBstr::SetByteLen(size_t bytes) {
-  DCHECK(bstr_ != NULL) << "attempting to modify a NULL bstr";
+  DCHECK(bstr_);
   uint32_t* data = reinterpret_cast<uint32_t*>(bstr_);
   data[-1] = static_cast<uint32_t>(bytes);
 }
 
 size_t ScopedBstr::Length() const {
-  return SysStringLen(bstr_);
+  return ::SysStringLen(bstr_);
 }
 
 size_t ScopedBstr::ByteLength() const {
-  return SysStringByteLen(bstr_);
+  return ::SysStringByteLen(bstr_);
 }
 
 }  // namespace win

base/win/scoped_bstr.h

@@ -13,6 +13,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/strings/string16.h"
+#include "base/strings/string_piece.h"
 
 namespace base {
 namespace win {
@@ -21,19 +22,18 @@ namespace win {
 // The class interface is based on unique_ptr.
 class BASE_EXPORT ScopedBstr {
  public:
-  ScopedBstr() : bstr_(NULL) {
-  }
+  ScopedBstr() : bstr_(nullptr) {}
 
   // Constructor to create a new BSTR.
   //
   // NOTE: Do not pass a BSTR to this constructor expecting ownership to
   // be transferred - even though it compiles! ;-)
-  explicit ScopedBstr(const char16* non_bstr);
+  explicit ScopedBstr(StringPiece16 non_bstr);
   ~ScopedBstr();
 
-  // Give ScopedBstr ownership over an already allocated BSTR or NULL.
+  // Give ScopedBstr ownership over an already allocated BSTR or null.
   // If you need to allocate a new BSTR instance, use |allocate| instead.
-  void Reset(BSTR bstr = NULL);
+  void Reset(BSTR bstr = nullptr);
 
   // Releases ownership of the BSTR to the caller.
   BSTR Release();
@@ -43,11 +43,11 @@ class BASE_EXPORT ScopedBstr {
   // If you already have a BSTR and want to transfer ownership to the
   // ScopedBstr instance, call |reset| instead.
   //
-  // Returns a pointer to the new BSTR, or NULL if allocation failed.
-  BSTR Allocate(const char16* str);
+  // Returns a pointer to the new BSTR, or null if allocation failed.
+  BSTR Allocate(StringPiece16 str);
 
   // Allocates a new BSTR with the specified number of bytes.
-  // Returns a pointer to the new BSTR, or NULL if allocation failed.
+  // Returns a pointer to the new BSTR, or null if allocation failed.
   BSTR AllocateBytes(size_t bytes);
 
   // Sets the allocated length field of the already-allocated BSTR to be
@@ -68,7 +68,7 @@ class BASE_EXPORT ScopedBstr {
 
   // Retrieves the pointer address.
   // Used to receive BSTRs as out arguments (and take ownership).
-  // The function DCHECKs on the current value being NULL.
+  // The function DCHECKs on the current value being null.
   // Usage: GetBstr(bstr.Receive());
   BSTR* Receive();
 
@@ -82,14 +82,15 @@ class BASE_EXPORT ScopedBstr {
     return bstr_;
   }
 
+  // Forbid comparison of ScopedBstr types.  You should never have the same
+  // BSTR owned by two different scoped_ptrs.
+  bool operator==(const ScopedBstr& bstr2) const = delete;
+  bool operator!=(const ScopedBstr& bstr2) const = delete;
+
  protected:
   BSTR bstr_;
 
  private:
-  // Forbid comparison of ScopedBstr types.  You should never have the same
-  // BSTR owned by two different scoped_ptrs.
-  bool operator==(const ScopedBstr& bstr2) const;
-  bool operator!=(const ScopedBstr& bstr2) const;
   DISALLOW_COPY_AND_ASSIGN(ScopedBstr);
 };
 

base/win/wmi.cc

@@ -52,8 +52,8 @@ bool CreateWmiClassMethodObject(IWbemServices* wmi_services,
                                 ComPtr<IWbemClassObject>* class_instance) {
   // We attempt to instantiate a COM object that represents a WMI object plus
   // a method rolled into one entity.
-  ScopedBstr b_class_name(class_name.data());
-  ScopedBstr b_method_name(method_name.data());
+  ScopedBstr b_class_name(class_name);
+  ScopedBstr b_method_name(method_name);
   ComPtr<IWbemClassObject> class_object;
   HRESULT hr;
   hr = wmi_services->GetObject(b_class_name, 0, nullptr,

chrome/browser/google/google_update_win.cc

@@ -534,7 +534,7 @@ HRESULT UpdateCheckDriver::BeginUpdateCheckInternal(
       // nice to have, a failure to do so does not affect the likelihood that
       // the update check and/or install will succeed.
       app_bundle->put_displayLanguage(
-          base::win::ScopedBstr(base::UTF8ToUTF16(locale_).c_str()));
+          base::win::ScopedBstr(base::UTF8ToUTF16(locale_)));
     }
 
     hresult = app_bundle->initialize();

chrome/chrome_cleaner/os/task_scheduler.cc

@@ -559,7 +559,7 @@ class TaskSchedulerV2 : public TaskScheduler {
 
       // Start now.
       base::Time now(base::Time::NowFromSystemTime());
-      base::win::ScopedBstr start_boundary(GetTimestampString(now).c_str());
+      base::win::ScopedBstr start_boundary(GetTimestampString(now));
       hr = trigger->put_StartBoundary(start_boundary);
       if (FAILED(hr)) {
         PLOG(ERROR) << "Can't put 'StartBoundary' to " << start_boundary << ". "
@@ -587,7 +587,7 @@ class TaskSchedulerV2 : public TaskScheduler {
     // None of the triggers should go beyond kNumDaysBeforeExpiry.
     base::Time expiry_date(base::Time::NowFromSystemTime() +
                            base::TimeDelta::FromDays(kNumDaysBeforeExpiry));
-    base::win::ScopedBstr end_boundary(GetTimestampString(expiry_date).c_str());
+    base::win::ScopedBstr end_boundary(GetTimestampString(expiry_date));
     hr = trigger->put_EndBoundary(end_boundary);
     if (FAILED(hr)) {
       PLOG(ERROR) << "Can't put 'EndBoundary' to " << end_boundary << ". "
@@ -616,14 +616,14 @@ class TaskSchedulerV2 : public TaskScheduler {
       return false;
     }
 
-    base::win::ScopedBstr path(run_command.GetProgram().value().c_str());
+    base::win::ScopedBstr path(run_command.GetProgram().value());
     hr = exec_action->put_Path(path);
     if (FAILED(hr)) {
       PLOG(ERROR) << "Can't set path of exec action. " << std::hex << hr;
       return false;
     }
 
-    base::win::ScopedBstr args(run_command.GetArgumentsString().c_str());
+    base::win::ScopedBstr args(run_command.GetArgumentsString());
     hr = exec_action->put_Arguments(args);
     if (FAILED(hr)) {
       PLOG(ERROR) << "Can't set arguments of exec action. " << std::hex << hr;

chrome/installer/util/advanced_firewall_manager_win.cc

@@ -108,8 +108,7 @@ void AdvancedFirewallManager::DeleteRule(
   // Rename rule to unique name and delete by unique name. We can't just delete
   // rule by name. Multiple rules with the same name and different app are
   // possible.
-  base::win::ScopedBstr unique_name(
-      base::UTF8ToUTF16(base::GenerateGUID()).c_str());
+  base::win::ScopedBstr unique_name(base::UTF8ToUTF16(base::GenerateGUID()));
   rule->put_Name(unique_name);
   firewall_rules_->Remove(unique_name);
 }
@@ -135,16 +134,15 @@ Microsoft::WRL::ComPtr<INetFwRule> AdvancedFirewallManager::CreateUDPRule(
     return Microsoft::WRL::ComPtr<INetFwRule>();
   }
 
-  udp_rule->put_Name(base::win::ScopedBstr(rule_name.c_str()));
-  udp_rule->put_Description(base::win::ScopedBstr(description.c_str()));
-  udp_rule->put_ApplicationName(
-      base::win::ScopedBstr(app_path_.value().c_str()));
+  udp_rule->put_Name(base::win::ScopedBstr(rule_name));
+  udp_rule->put_Description(base::win::ScopedBstr(description));
+  udp_rule->put_ApplicationName(base::win::ScopedBstr(app_path_.value()));
   udp_rule->put_Protocol(NET_FW_IP_PROTOCOL_UDP);
   udp_rule->put_Direction(NET_FW_RULE_DIR_IN);
   udp_rule->put_Enabled(VARIANT_TRUE);
   udp_rule->put_LocalPorts(
-      base::win::ScopedBstr(base::StringPrintf(L"%u", port).c_str()));
-  udp_rule->put_Grouping(base::win::ScopedBstr(app_name_.c_str()));
+      base::win::ScopedBstr(base::StringPrintf(L"%u", port)));
+  udp_rule->put_Grouping(base::win::ScopedBstr(app_name_));
   udp_rule->put_Profiles(NET_FW_PROFILE2_ALL);
   udp_rule->put_Action(NET_FW_ACTION_ALLOW);
 

chrome/services/util_win/shell_util_win_impl.cc

@@ -108,8 +108,7 @@ bool IsPinnedToTaskbarHelper::ShortcutHasUnpinToTaskbarVerb(
 
   Microsoft::WRL::ComPtr<FolderItem> item;
   hresult = folder->ParseName(
-      base::win::ScopedBstr(shortcut.BaseName().value().c_str()),
-      item.GetAddressOf());
+      base::win::ScopedBstr(shortcut.BaseName().value()), item.GetAddressOf());
   if (FAILED(hresult) || !item) {
     error_occured_ = true;
     return false;

remoting/host/desktop_session_win.cc

@@ -306,7 +306,7 @@ bool RdpSession::Initialize(const ScreenResolution& resolution) {
   Microsoft::WRL::ComPtr<IRdpDesktopSessionEventHandler> event_handler(
       new EventHandler(weak_factory_.GetWeakPtr()));
   terminal_id_ = base::GenerateGUID();
-  base::win::ScopedBstr terminal_id(base::UTF8ToUTF16(terminal_id_).c_str());
+  base::win::ScopedBstr terminal_id(base::UTF8ToUTF16(terminal_id_));
   result = rdp_desktop_session_->Connect(host_size.width(), host_size.height(),
                                          kDefaultRdpDpi, kDefaultRdpDpi,
                                          terminal_id, server_port,

remoting/host/setup/win/auth_code_getter.cc

@@ -41,8 +41,8 @@ void AuthCodeGetter::GetAuthCode(
     on_auth_code_.Run("");
     return;
   }
-  base::win::ScopedBstr url(base::UTF8ToWide(
-      GetOauthStartUrl(GetDefaultOauthRedirectUrl())).c_str());
+  base::win::ScopedBstr url(
+      base::UTF8ToWide(GetOauthStartUrl(GetDefaultOauthRedirectUrl())));
   base::win::ScopedVariant empty_variant;
   hr = browser_->Navigate(url, empty_variant.AsInput(), empty_variant.AsInput(),
                           empty_variant.AsInput(), empty_variant.AsInput());

remoting/host/win/rdp_client_window.cc

@@ -265,8 +265,8 @@ LRESULT RdpClientWindow::OnCreate(CREATESTRUCT* create_struct) {
   Microsoft::WRL::ComPtr<mstsc::IMsTscSecuredSettings> secured_settings;
   Microsoft::WRL::ComPtr<mstsc::IMsRdpClientSecuredSettings> secured_settings2;
   base::win::ScopedBstr server_name(
-      base::UTF8ToUTF16(server_endpoint_.ToStringWithoutPort()).c_str());
-  base::win::ScopedBstr terminal_id(base::UTF8ToUTF16(terminal_id_).c_str());
+      base::UTF8ToUTF16(server_endpoint_.ToStringWithoutPort()));
+  base::win::ScopedBstr terminal_id(base::UTF8ToUTF16(terminal_id_));
 
   // Create the child window that actually hosts the ActiveX control.
   RECT rect = {0, 0, screen_resolution_.dimensions().width(),