/C++ readability/ - Make Chromoting Host report crashes to Breakpad (Windows only). The user must e…

remoting/host/plugin/daemon_installer_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 "remoting/host/plugin/daemon_installer_win.h"
 
 #include <windows.h>
 
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/process_util.h"
 #include "base/string16.h"
 #include "base/stringize_macros.h"
 #include "base/stringprintf.h"
 #include "base/time.h"
 #include "base/timer.h"
 #include "base/win/object_watcher.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_bstr.h"
 #include "base/win/scoped_comptr.h"
 #include "base/win/scoped_handle.h"
 
 namespace omaha {
 #include "google_update/google_update_idl.h"
 }  // namespace omaha

[Peter Kasting, 2012/06/14 20:16:27]

No other files in the project which #include this file put it in a namespace. 
Is this truly necessary?

I ask this because if we can eliminate this, the correct ordering of the
#includes (including the one you're adding) becomes clearer.

[alexeypa (please no reviews), 2012/06/15 18:45:49]

Unfortunately, the state constants declared in
"google_update/google_update_idl.h" conflict with constants defined in some
other header so I had to put this one into a namespace.

[Peter Kasting, 2012/06/15 19:02:42]

Which other header has the conflict?  Can we change the names declared there? 
(The google_update_idl.h things are autogenerated so changing them might be
trickier.)  Namespacing around a #include is something I really have never seen
other Chrome code do so I'd be willing to go to some length to avoid it.

[alexeypa (please no reviews), 2012/06/15 19:51:24]

It turns out there is no conflict anymore and omaha namespace is not needed at
all. I verified the compiled code has all proper jumps (just in case).

 
+#include "remoting/host/constants.h"
+
 using base::win::ScopedBstr;
 using base::win::ScopedComPtr;
 
 namespace {
 
 // The COM elevation moniker for Omaha.
 const char16 kOmahaElevationMoniker[] =

[Peter Kasting, 2012/06/14 20:16:27]

This file contains many instances of problem's I've suggested correcting in your
new code elsewhere (use of char16 and string16 where wchar_t/wstring are more
correct, use of stringize macros instead of L).  Fixing all these may be beyond
the scope of this change.  Consider filing a bug to clean these up.

[alexeypa (please no reviews), 2012/06/15 18:45:49]

I filed http://crbug.com/133003 for this.

     TO_L_STRING("Elevation:Administrator!new:GoogleUpdate.Update3WebMachine");
 
 // The registry key where the configuration of Omaha is stored.
 const char16 kOmahaUpdateKeyName[] = TO_L_STRING("Software\\Google\\Update");
 
 // The name of the value where the full path to GoogleUpdate.exe is stored.
 const char16 kOmahaPathValueName[] = TO_L_STRING("path");
 
 // The command line format string for GoogleUpdate.exe
 const char16 kGoogleUpdateCommandLineFormat[] =
     TO_L_STRING("\"%ls\" /install \"bundlename=Chromoting%%20Host&appguid=%ls&")
     TO_L_STRING("appname=Chromoting%%20Host&needsadmin=True&lang=%ls\"");
 
-// The Omaha Appid of the host.
-const char16 kOmahaAppid[] =
-    TO_L_STRING("{b210701e-ffc4-49e3-932b-370728c72662}");
-
 // TODO(alexeypa): Get the desired laungage from the web app.
 const char16 kOmahaLanguage[] = TO_L_STRING("en");
 
 // An empty string for optional parameters.
 const char16 kOmahaEmpty[] = TO_L_STRING("");
 
 // The installation status polling interval.
 const int kOmahaPollIntervalMs = 500;
 
 }  // namespace
@@ skipping 75 matching lines @@
     return;
   }
 
   hr = bundle_->initialize();
   if (FAILED(hr)) {
     Done(hr);
     return;
   }
 
   // Add Chromoting Host to the bundle.
-  ScopedBstr appid(kOmahaAppid);
+  ScopedBstr appid(kHostOmahaAppid);
   ScopedBstr empty(kOmahaEmpty);
   ScopedBstr language(kOmahaLanguage);
   hr = bundle_->createApp(appid, empty, language, empty);
   if (FAILED(hr)) {
     Done(hr);
     return;
   }
 
   hr = bundle_->checkForUpdate();
   if (FAILED(hr)) {
@@ skipping 128 matching lines @@
                                 &google_update);
   if (result != ERROR_SUCCESS) {
     Done(HRESULT_FROM_WIN32(result));
     return;
   }
 
   // Launch the updater process and wait for its termination.
   string16 command_line =
       StringPrintf(kGoogleUpdateCommandLineFormat,
                    google_update.c_str(),
-                   kOmahaAppid,
+                   kHostOmahaAppid,
                    kOmahaLanguage);
 
   base::LaunchOptions options;
   if (!base::LaunchProcess(command_line, options, process_.Receive())) {
     result = GetLastError();
     Done(HRESULT_FROM_WIN32(result));
     return;
   }
 
   if (!process_watcher_.StartWatching(process_.Get(), this)) {
@@ skipping 75 matching lines @@
     HWND parent = GetAncestor(window, GA_PARENT);
     if (parent == NULL) {
       return window;
     }
 
     window = parent;
   }
 }
 
 }  // namespace remoting