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

remoting/host/constants.h

Index: remoting/host/constants.h
diff --git a/remoting/host/constants.h b/remoting/host/constants.h
index 9654afffc708b6d9a073f46b82736dd101487ea3..9358583f1812d3aec665c3bc8d590d9a108b5251 100644
--- a/remoting/host/constants.h
+++ b/remoting/host/constants.h
@@ -5,6 +5,8 @@
 #ifndef REMOTING_HOST_CONSTANTS_H_
 #define REMOTING_HOST_CONSTANTS_H_
 
+#include "base/string16.h"

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

Nit: #ifdef this as well.  I suggest moving the block below up to here so you
can use a single #if for both.

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

I got rid of string16 entirely. 

BTW I don't think I could move this include below. The guide says that platform
specific includes should go right after regular includes:

"For platform-specific #includes, put them below the "regular" includes,
separated with a blank line."

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

Yes, I was suggesting the opposite (to move some code below up to here rather
than to move this #include downwards) as you're correct that we don't want
#includes in the middle of the file.  Sorry if I was unclear.  (And it's moot
now anyway.)

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

Well, actually your comment was quite clear. I just misread it.

+
 namespace remoting {
 
 // Known host exit codes.
@@ -24,6 +26,13 @@ enum HostExitCodes {
   kMaxPermanentErrorExitCode = kInvalidOauthCredentialsExitCode
 };
 
+#if defined(OS_WIN)
+

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

Nit: Extra blank line here and below.  There aren't definitive rules on this,
but the style guide examples generally place blank lines inside most namespace
definitions, and not inside most preprocessor directive blocks.

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

Done.

+// The Omaha Appid of the host.
+extern const char16 kHostOmahaAppid[];

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

This should be a wchar_t rather than a string16, given how it will be used in
breakpad_win.cc.

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

Done.

+
+#endif  // defined(OS_WIN)
+
 }  // namespace remoting
 
 #endif  // REMOTING_HOST_CONSTANTS_H_