Add UMA for --app-shim-error.

chrome/browser/web_applications/web_app_mac.mm

Index: chrome/browser/web_applications/web_app_mac.mm
diff --git a/chrome/browser/web_applications/web_app_mac.mm b/chrome/browser/web_applications/web_app_mac.mm
index 02f3f0dc7c14ead4c29c6e2e6b3ff23c5122760f..bf14ded30c10b0bdae95fe40575a96a6185df801 100644
--- a/chrome/browser/web_applications/web_app_mac.mm
+++ b/chrome/browser/web_applications/web_app_mac.mm
@@ -16,6 +16,7 @@
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/mac/scoped_nsobject.h"
+#include "base/metrics/histogram.h"
 #include "base/path_service.h"
 #include "base/process/process_handle.h"
 #include "base/strings/string16.h"
@@ -57,6 +58,16 @@ namespace {
 // Launch Services Key to run as an agent app, which doesn't launch in the dock.
 NSString* const kLSUIElement = @"LSUIElement";
 
+// When a --app-shim-error is triggered, this is used to record the version of
+// the app shim.

[tapted, 2014/07/01 05:53:58]

nit: mention that buckets are chosen to track the 32->64 bit change on Mac

[jackhou1, 2014/07/01 07:37:51]

Done.

+// Order must be maintained as this is used in UMA.
+enum AppShimError {
+  APP_SHIM_ERROR_37_OR_LOWER,
+  APP_SHIM_ERROR_38,
+  APP_SHIM_ERROR_39_OR_HIGHER,
+  APP_SHIM_ERROR_COUNT,
+};
+
 class ScopedCarbonHandle {
  public:
   ScopedCarbonHandle(size_t initial_size) : handle_(NewHandle(initial_size)) {
@@ -496,6 +507,25 @@ web_app::ShortcutInfo BuildShortcutInfoFromBundle(
   return shortcut_info;
 }
 
+web_app::ShortcutInfo RecordAppShimErrorAndBuildShortcutInfo(
+    const base::FilePath& bundle_path) {
+  NSDictionary* plist = ReadPlist(GetPlistPath(bundle_path));
+  int version = [[[[plist valueForKey:app_mode::kCFBundleShortVersionStringKey]
+      componentsSeparatedByString:@"."] objectAtIndex:0] intValue];

[tapted, 2014/07/01 05:53:59]

since we're rebuilding a possibly corrupt shim.. perhaps this should be robust
to that. I think objective C's you-can-call-methods-on-nil will mean this will
be pretty safe (the only think I'm not certain about is whether calling
componentsSeparatedByString on an empty string (as opposed to nil) returns an
empty array or an array with a single empty string [documentation suggests the
latter, but if it's an empty array, objectAtIndex[0] will crash]).

However, then the error_enum should handle `0` -- so maybe add
AP_SHIM_ERROR_UNKNOWN_VERSION;

[jackhou1, 2014/07/01 07:37:51]

Done.

I tried breaking the field in various ways and it seems to handle it fine.

+  int error_enum;

[tapted, 2014/07/01 05:53:58]

int -> AppShimError?

[jackhou1, 2014/07/01 07:37:51]

Done.

+  if (version < 38)
+    error_enum = APP_SHIM_ERROR_37_OR_LOWER;
+  else if (version == 38)
+    error_enum = APP_SHIM_ERROR_38;
+  else
+    error_enum = APP_SHIM_ERROR_39_OR_HIGHER;
+  UMA_HISTOGRAM_ENUMERATION("Apps.AppShimError",
+                            error_enum,
+                            APP_SHIM_ERROR_COUNT);
+
+  return BuildShortcutInfoFromBundle(bundle_path);
+}
+
 void UpdateFileTypes(NSMutableDictionary* plist,
                      const extensions::FileHandlersInfo& file_handlers_info) {
   NSMutableArray* document_types =
@@ -939,7 +969,7 @@ bool MaybeRebuildShortcut(const CommandLine& command_line) {
   base::PostTaskAndReplyWithResult(
       content::BrowserThread::GetBlockingPool(),
       FROM_HERE,
-      base::Bind(&BuildShortcutInfoFromBundle,
+      base::Bind(&RecordAppShimErrorAndBuildShortcutInfo,
                  command_line.GetSwitchValuePath(app_mode::kAppShimError)),
       base::Bind(&RebuildAppAndLaunch));
   return true;