UMA for Windows 8 Secondary Tile pinning/unpinning user actions

base/win/metro.h

Index: base/win/metro.h
diff --git a/base/win/metro.h b/base/win/metro.h
index a43bbc31b23afd439526022816441ac691419ced..ea4fa2ccf3d1e98cb3c57704c4e08011c80a25ef 100644
--- a/base/win/metro.h
+++ b/base/win/metro.h
@@ -9,6 +9,7 @@
 #include <wpcapi.h>
 
 #include "base/base_export.h"
+#include "base/file_path.h"
 #include "base/string16.h"
 
 namespace base {
@@ -36,6 +37,25 @@ enum MetroPreviousExecutionState {
   LASTEXECUTIONSTATE,
 };
 
+// Enum values for UMA histogram reporting of site-specific tile pinning.

[benwells, 2012/11/27 08:56:39]

Does this stuff all have to go in this file? It doesn't feel right going in
base.

[tapted, 2012/11/28 00:33:43]

It needs to go somewhere shared between /win8 and /chrome/browser/ui..

Currently, viable options (i.e. current viable dependencies of both) are:
 * /base, and
 * /chrome/common/common_constants,

/base/win/metro.h currently seems to be the most common way for things to be
shared between the metro driver and the win8 parts of the browser.

There is also:
 * /ui/metro_viewer/[metro_viewer_messages.h]

An option [option "1"] in the same vein as this would be to make:
 * /ui/metro_uma/
 * /ui/metro_uma/metro_uma.gyp
 * /ui/metro_uma/metro_uma_types.h

That feels a bit heavyweight.

The "right" solution to me feels like:
 * /win8/common/
 * /win8/common/win8_common.gyp  (or win8/win8_common.gyp .. or .gypi )

Then the stuff currently in /base/win/metro.h moves to /win8/common/metro.h.
That feels like a separate CL though [option "2"], and there might be a good
reason for having some metro stuff in /base. But then, for this, we would just
add /win8/common/secondary_tile_types.h .

Alternatively, [option "3"] I can get that ball rolling in this CL. That is,
start with /win8/common/{win8_common.gypi,secondary_tile_types.h}. And add
OS=="win" ==> win8_common.gypi dependency to chrome_browser.gypi, right under 
                '../ui/metro_viewer/metro_viewer.gyp:metro_viewer',
then things can start moving out of /base/win in follow-ups.

Or try out a CL for moving everything first [option "2"], and land it before
this (assuming it's feasible).

An [option "4"]:
 * These enums are not necessarily UMA-specific: this comment could rightly drop
the UMA wording and maybe feel less out of place.

An [option "1.1"]:
 * Do "1", but put it in /win8/metro_uma

And there's always ["option 0"]: no-op ;-).

Nothing done yet -- WDYT?

[benwells, 2012/11/29 04:19:46]

I was referring to all the secondary tile stuff you're adding, not just the UMA
stuff. Today I chatted with cpu@ about this and he also wasn't keen to put
secondary related tile stuff into this file.

Why not just put it in win8/metro_driver/secondary_tile.h and refer to that from
metro_pin_tab_helper?

[tapted, 2012/11/29 07:19:27]

** Presubmit ERRORS **
You added one or more #includes that violate checkdeps rules.
  chrome\browser\ui\metro_pin_tab_helper_win.cc
      Illegal include: "win8/metro_driver/secondary_tile_types.h"
    Because of no rule applying.

"One does not simply #include in Chrome..." ;)

I've uploaded a patch with presubmit hooks skippped to make sure this is what
you had in mind. I'll look into policies/procedures around this tomorrow.
Perhaps, for approval, there is a "right" way to do it (maybe along the lines of
option [3]).

[MAD, 2012/11/29 15:26:18]

If these are specific to UMA I would leave them here and put UMA in the name
somewhere...

[tapted, 2012/11/30 00:30:31]

Done.

+enum MetroSecondaryTilePinState {
+  METRO_PIN_STATE_NONE,
+  METRO_PIN_INITIATED,
+  METRO_PIN_LOGO_READY,
+  METRO_PIN_REQUEST_SHOW_ERROR,
+  METRO_PIN_RESULT_CANCEL,
+  METRO_PIN_RESULT_OK,
+  METRO_PIN_RESULT_OTHER,
+  METRO_PIN_RESULT_ERROR,
+  METRO_UNPIN_INITIATED,
+  METRO_UNPIN_REQUEST_SHOW_ERROR,
+  METRO_UNPIN_RESULT_CANCEL,
+  METRO_UNPIN_RESULT_OK,
+  METRO_UNPIN_RESULT_OTHER,
+  METRO_UNPIN_RESULT_ERROR,
+  METRO_PIN_STATE_LIMIT
+};
+
 // Contains information about the currently displayed tab in metro mode.
 struct CurrentTabInfo {
   wchar_t* title;
@@ -97,6 +117,23 @@ typedef void (*MetroNotification)(const char* origin_url,
                                   MetroNotificationClickedHandler handler,
                                   const wchar_t* handler_context);
 
+// Callback invoked by Metro Pin and UnPin functions after user gesture.
+typedef void (*MetroPinResultCallback)(void* callback_data,
+                                       MetroSecondaryTilePinState result);
+
+// Function to pin a site-specific tile (bookmark) to the start screen.
+typedef void (*MetroPinToStartScreen)(const string16& tile_id,
+                                      const string16& title,
+                                      const string16& url,
+                                      const FilePath& logo_path,
+                                      MetroPinResultCallback callback,
+                                      void* callback_data);
+
+// Function to un-pin a site-specific tile (bookmark) from the start screen.
+typedef void (*MetroUnPinFromStartScreen)(const string16& title_id,
+                                          MetroPinResultCallback callback,
+                                          void* callback_data);
+
 }  // namespace win
 }  // namespace base