Skip to content

Commit

Permalink
UMA for Windows 8 Secondary Tile pinning/unpinning user actions
Browse files Browse the repository at this point in the history
triggered from hotdog -> Bookmarks when in Metro mode.

To perform the metrics recording, this CL includes changes
to provide a callback mechanism for the asynchronous tile
creation in metro_driver.dll to report back the result of
the use gesture to the browser process. Note that both
metro_driver.dll and chrome.dll link statically to base, so
they have distinct data segments holding UMA histograms,
hence the callback.

BUG=160840
TEST=In Windows 8 Metro Mode, Hotdog -> Bookmarks -> Pin to start page:
 - pin and cancel (escape/click/touch off popup)
 - pin and confirm
 - (when pinned) unpin and cancel
 - unpin and confirm
Above should all work (and report UMA histogram data).

Review URL: https://chromiumcodereview.appspot.com/11280112

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@170916 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
tapted@chromium.org committed Dec 4, 2012
1 parent 1b6d130 commit c17ca9d
Show file tree
Hide file tree
Showing 6 changed files with 187 additions and 50 deletions.
39 changes: 39 additions & 0 deletions base/win/metro.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <wpcapi.h>

#include "base/base_export.h"
#include "base/callback.h"
#include "base/file_path.h"
#include "base/string16.h"

namespace base {
Expand Down Expand Up @@ -36,6 +38,26 @@ enum MetroPreviousExecutionState {
LASTEXECUTIONSTATE,
};

// Enum values for UMA histogram reporting of site-specific tile pinning.
// TODO(tapted): Move this to win8/util when ready (http://crbug.com/160288).
enum MetroSecondaryTilePinUmaResult {
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;
Expand Down Expand Up @@ -97,6 +119,23 @@ typedef void (*MetroNotification)(const char* origin_url,
MetroNotificationClickedHandler handler,
const wchar_t* handler_context);

// Callback for UMA invoked by Metro Pin and UnPin functions after user gesture.
typedef base::Callback<void(MetroSecondaryTilePinUmaResult)>
MetroPinUmaResultCallback;

// 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,
const MetroPinUmaResultCallback& callback);

// Function to un-pin a site-specific tile (bookmark) from the start screen.
typedef void (*MetroUnPinFromStartScreen)(
const string16& title_id,
const MetroPinUmaResultCallback& callback);

} // namespace win
} // namespace base

Expand Down
42 changes: 33 additions & 9 deletions chrome/browser/ui/metro_pin_tab_helper_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "base/logging.h"
#include "base/memory/ref_counted.h"
#include "base/memory/ref_counted_memory.h"
#include "base/metrics/histogram.h"
#include "base/path_service.h"
#include "base/string_number_conversions.h"
#include "base/utf_string_conversions.h"
Expand Down Expand Up @@ -40,6 +41,9 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(MetroPinTabHelper)

namespace {

// Histogram name for site-specific tile pinning metrics.
const char kMetroPinMetric[] = "Metro.SecondaryTilePin";

// Generate an ID for the tile based on |url_str|. The ID is simply a hash of
// the URL.
string16 GenerateTileId(const string16& url_str) {
Expand Down Expand Up @@ -158,6 +162,14 @@ bool GetPathToBackupLogo(const FilePath& logo_dir,
return file_util::CopyFile(default_logo_path, *logo_path);
}

// UMA reporting callback for site-specific secondary tile creation.
void PinPageReportUmaCallback(
base::win::MetroSecondaryTilePinUmaResult result) {
UMA_HISTOGRAM_ENUMERATION(kMetroPinMetric,
result,
base::win::METRO_PIN_STATE_LIMIT);
}

// The PinPageTaskRunner class performs the necessary FILE thread actions to
// pin a page, such as generating or copying the tile image file. When it
// has performed these actions it will send the tile creation request to the
Expand Down Expand Up @@ -219,21 +231,27 @@ void PinPageTaskRunner::RunOnFileThread() {
return;
}

UMA_HISTOGRAM_ENUMERATION(kMetroPinMetric,
base::win::METRO_PIN_LOGO_READY,
base::win::METRO_PIN_STATE_LIMIT);

HMODULE metro_module = base::win::GetMetroModule();
if (!metro_module)
return;

typedef void (*MetroPinToStartScreen)(const string16&, const string16&,
const string16&, const FilePath&);
MetroPinToStartScreen metro_pin_to_start_screen =
reinterpret_cast<MetroPinToStartScreen>(
base::win::MetroPinToStartScreen metro_pin_to_start_screen =
reinterpret_cast<base::win::MetroPinToStartScreen>(
::GetProcAddress(metro_module, "MetroPinToStartScreen"));
if (!metro_pin_to_start_screen) {
NOTREACHED();
return;
}

metro_pin_to_start_screen(tile_id, title_, url_, logo_path);
metro_pin_to_start_screen(tile_id,
title_,
url_,
logo_path,
base::Bind(&PinPageReportUmaCallback));
}

} // namespace
Expand Down Expand Up @@ -378,10 +396,16 @@ bool MetroPinTabHelper::IsPinned() const {

void MetroPinTabHelper::TogglePinnedToStartScreen() {
if (IsPinned()) {
UMA_HISTOGRAM_ENUMERATION(kMetroPinMetric,
base::win::METRO_UNPIN_INITIATED,
base::win::METRO_PIN_STATE_LIMIT);
UnPinPageFromStartScreen();
return;
}

UMA_HISTOGRAM_ENUMERATION(kMetroPinMetric,
base::win::METRO_PIN_INITIATED,
base::win::METRO_PIN_STATE_LIMIT);
GURL url = web_contents()->GetURL();
string16 url_str = UTF8ToUTF16(url.spec());
string16 title = web_contents()->GetTitle();
Expand Down Expand Up @@ -445,9 +469,8 @@ void MetroPinTabHelper::UnPinPageFromStartScreen() {
if (!metro_module)
return;

typedef void (*MetroUnPinFromStartScreen)(const string16&);
MetroUnPinFromStartScreen metro_un_pin_from_start_screen =
reinterpret_cast<MetroUnPinFromStartScreen>(
base::win::MetroUnPinFromStartScreen metro_un_pin_from_start_screen =
reinterpret_cast<base::win::MetroUnPinFromStartScreen>(
::GetProcAddress(metro_module, "MetroUnPinFromStartScreen"));
if (!metro_un_pin_from_start_screen) {
NOTREACHED();
Expand All @@ -456,7 +479,8 @@ void MetroPinTabHelper::UnPinPageFromStartScreen() {

GURL url = web_contents()->GetURL();
string16 tile_id = GenerateTileId(UTF8ToUTF16(url.spec()));
metro_un_pin_from_start_screen(tile_id);
metro_un_pin_from_start_screen(tile_id,
base::Bind(&PinPageReportUmaCallback));
}

void MetroPinTabHelper::FaviconDownloaderFinished() {
Expand Down
14 changes: 0 additions & 14 deletions win8/metro_driver/chrome_app_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -305,20 +305,6 @@ void FlipFrameWindowsInternal() {

} // namespace

HRESULT ChromeAppView::TileRequestCreateDone(
winfoundtn::IAsyncOperation<bool>* async,
AsyncStatus status) {
if (status == Completed) {
unsigned char result;
CheckHR(async->GetResults(&result));
DVLOG(1) << __FUNCTION__ << " result " << static_cast<int>(result);
} else {
LOG(ERROR) << __FUNCTION__ << " Unexpected async status " << status;
}

return S_OK;
}

void ChromeAppView::DisplayNotification(
const ToastNotificationHandler::DesktopNotification& notification) {
DVLOG(1) << __FUNCTION__;
Expand Down
3 changes: 0 additions & 3 deletions win8/metro_driver/chrome_app_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ class ChromeAppView
static LRESULT CALLBACK CoreWindowProc(HWND window, UINT message, WPARAM wp,
LPARAM lp);

HRESULT TileRequestCreateDone(winfoundtn::IAsyncOperation<bool>* async,
AsyncStatus status);

bool osk_visible_notification_received() const {
return osk_visible_notification_received_;
}
Expand Down
123 changes: 105 additions & 18 deletions win8/metro_driver/secondary_tile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,80 @@

namespace {

void DeleteTileFromStartScreen(const string16& tile_id) {
using base::win::MetroPinUmaResultCallback;

// Callback for asynchronous pin requests.
class TileRequestCompleter {
public:
enum PinType {
PIN,
UNPIN
};
TileRequestCompleter(PinType type, const MetroPinUmaResultCallback& callback)
: type_(type), callback_(callback) {}

void Complete(mswr::ComPtr<winfoundtn::IAsyncOperation<bool>>& completion);

private:
// Callback that responds to user input on the pin request pop-up. This will
// run |callback_|, then delete |this| before returning.
HRESULT Respond(winfoundtn::IAsyncOperation<bool>* async,
AsyncStatus status);

PinType type_;
MetroPinUmaResultCallback callback_;
};

void TileRequestCompleter::Complete(
mswr::ComPtr<winfoundtn::IAsyncOperation<bool>>& completion) {
typedef winfoundtn::IAsyncOperationCompletedHandler<bool> RequestDoneType;
mswr::ComPtr<RequestDoneType> handler(mswr::Callback<RequestDoneType>(
this, &TileRequestCompleter::Respond));
DCHECK(handler.Get() != NULL);
HRESULT hr = completion->put_Completed(handler.Get());
CheckHR(hr, "Failed to put_Completed");
}

HRESULT TileRequestCompleter::Respond(winfoundtn::IAsyncOperation<bool>* async,
AsyncStatus status) {
base::win::MetroSecondaryTilePinUmaResult pin_state =
base::win::METRO_PIN_STATE_NONE;

if (status == Completed) {
unsigned char result;
CheckHR(async->GetResults(&result));
LOG(INFO) << __FUNCTION__ << " result " << static_cast<int>(result);
switch (result) {
case 0:
pin_state = type_ == PIN ?
base::win::METRO_PIN_RESULT_CANCEL :
base::win::METRO_UNPIN_RESULT_CANCEL;
break;
case 1:
pin_state = type_ == PIN ?
base::win::METRO_PIN_RESULT_OK :
base::win::METRO_UNPIN_RESULT_OK;
break;
default:
pin_state = type_ == PIN ?
base::win::METRO_PIN_RESULT_OTHER :
base::win::METRO_UNPIN_RESULT_OTHER;
break;
}
} else {
LOG(ERROR) << __FUNCTION__ << " Unexpected async status " << status;
pin_state = type_ == PIN ?
base::win::METRO_PIN_RESULT_ERROR :
base::win::METRO_UNPIN_RESULT_ERROR;
}
callback_.Run(pin_state);

delete this;
return S_OK;
}

void DeleteTileFromStartScreen(const string16& tile_id,
const MetroPinUmaResultCallback& callback) {
DVLOG(1) << __FUNCTION__;
mswr::ComPtr<winui::StartScreen::ISecondaryTileFactory> tile_factory;
HRESULT hr = winrt_utils::CreateActivationFactory(
Expand All @@ -35,18 +108,23 @@ void DeleteTileFromStartScreen(const string16& tile_id) {
hr = tile->RequestDeleteAsync(completion.GetAddressOf());
CheckHR(hr, "RequestDeleteAsync failed");

typedef winfoundtn::IAsyncOperationCompletedHandler<bool> RequestDoneType;
mswr::ComPtr<RequestDoneType> handler(mswr::Callback<RequestDoneType>(
globals.view, &ChromeAppView::TileRequestCreateDone));
DCHECK(handler.Get() != NULL);
hr = completion->put_Completed(handler.Get());
CheckHR(hr, "Failed to put_Completed");
if (FAILED(hr)) {
callback.Run(base::win::METRO_UNPIN_REQUEST_SHOW_ERROR);
return;
}

// Deleted in TileRequestCompleter::Respond when the async operation
// completes.
TileRequestCompleter* completer =
new TileRequestCompleter(TileRequestCompleter::UNPIN, callback);
completer->Complete(completion);
}

void CreateTileOnStartScreen(const string16& tile_id,
const string16& title_str,
const string16& url_str,
const FilePath& logo_path) {
const FilePath& logo_path,
const MetroPinUmaResultCallback& callback) {
VLOG(1) << __FUNCTION__;

mswr::ComPtr<winui::StartScreen::ISecondaryTileFactory> tile_factory;
Expand Down Expand Up @@ -99,12 +177,16 @@ void CreateTileOnStartScreen(const string16& tile_id,
hr = tile->RequestCreateAsync(completion.GetAddressOf());
CheckHR(hr, "RequestCreateAsync failed");

typedef winfoundtn::IAsyncOperationCompletedHandler<bool> RequestDoneType;
mswr::ComPtr<RequestDoneType> handler(mswr::Callback<RequestDoneType>(
globals.view, &ChromeAppView::TileRequestCreateDone));
DCHECK(handler.Get() != NULL);
hr = completion->put_Completed(handler.Get());
CheckHR(hr, "Failed to put_Completed");
if (FAILED(hr)) {
callback.Run(base::win::METRO_PIN_REQUEST_SHOW_ERROR);
return;
}

// Deleted in TileRequestCompleter::Respond when the async operation
// completes.
TileRequestCompleter* completer =
new TileRequestCompleter(TileRequestCompleter::PIN, callback);
completer->Complete(completion);
}

} // namespace
Expand All @@ -122,19 +204,24 @@ BOOL MetroIsPinnedToStartScreen(const string16& tile_id) {
return exists;
}

void MetroUnPinFromStartScreen(const string16& tile_id) {
void MetroUnPinFromStartScreen(const string16& tile_id,
const MetroPinUmaResultCallback& callback) {
globals.appview_msg_loop->PostTask(
FROM_HERE, base::Bind(&DeleteTileFromStartScreen, tile_id));
FROM_HERE, base::Bind(&DeleteTileFromStartScreen,
tile_id,
callback));
}

void MetroPinToStartScreen(const string16& tile_id,
const string16& title,
const string16& url,
const FilePath& logo_path) {
const FilePath& logo_path,
const MetroPinUmaResultCallback& callback) {
globals.appview_msg_loop->PostTask(
FROM_HERE, base::Bind(&CreateTileOnStartScreen,
tile_id,
title,
url,
logo_path));
logo_path,
callback));
}
16 changes: 10 additions & 6 deletions win8/metro_driver/secondary_tile.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,22 @@

#include "base/file_path.h"
#include "base/string16.h"
#include "base/win/metro.h"

extern "C" __declspec(dllexport)
BOOL MetroIsPinnedToStartScreen(const string16& tile_id);

extern "C" __declspec(dllexport)
void MetroUnPinFromStartScreen(const string16& tile_id);
void MetroUnPinFromStartScreen(
const string16& tile_id,
const base::win::MetroPinUmaResultCallback& callback);

extern "C" __declspec(dllexport)
void MetroPinToStartScreen(const string16& tile_id,
const string16& title,
const string16& url,
const FilePath& logo_path);
void MetroPinToStartScreen(
const string16& tile_id,
const string16& title,
const string16& url,
const FilePath& logo_path,
const base::win::MetroPinUmaResultCallback& callback);

#endif // CHROME_BROWSER_UI_METRO_DRIVER_SECONDARY_TILE_H_

0 comments on commit c17ca9d

Please sign in to comment.