From 2f9475c46524029e881fa36cee4431d137f9aefb Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Wed, 21 Dec 2016 22:51:27 -0800 Subject: [PATCH] Bug 1319245 - Track rich telemetry data for Activity Stream r=sebastian General concept is to populate the "extras" field with a stringified JSON object which contains bunch of additional data which, in aggregate, might give insight into user's actions. A builder is used in order to make constructing the extras json string easier. This builder has a concept of a "global state" which lets us easily include some information with every A-S telemetry ping. Currently this is used to track whether or not user has a firefox account. An instance of a builder is passed around, augmented as more information becomes known, and is materialized into an "extras" string whenever an action occurs and telemetry event needs to be sent. MozReview-Commit-ID: GDmxkWChnnA --HG-- extra : rebase_source : 025d198e16d3a8af8b6e94bd531e916b80f9841a --- .../gecko/activitystream/ActivityStream.java | 5 + .../ActivityStreamTelemetry.java | 171 ++++++++++++++++++ .../mozilla/gecko/activitystream/Utils.java | 32 ++++ .../home/activitystream/ActivityStream.java | 12 +- .../gecko/home/activitystream/StreamItem.java | 44 +++-- .../activitystream/StreamRecyclerAdapter.java | 18 +- .../menu/ActivityStreamContextMenu.java | 96 ++++++---- .../menu/BottomSheetContextMenu.java | 4 +- .../activitystream/menu/PopupContextMenu.java | 4 + .../activitystream/topsites/TopSitesCard.java | 20 +- mobile/android/base/moz.build | 2 + .../tests/testActivityStreamContextMenu.java | 5 +- 12 files changed, 345 insertions(+), 68 deletions(-) create mode 100644 mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStreamTelemetry.java create mode 100644 mobile/android/base/java/org/mozilla/gecko/activitystream/Utils.java diff --git a/mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStream.java b/mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStream.java index d1c3f591691d..d1f26b666af4 100644 --- a/mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStream.java +++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStream.java @@ -8,10 +8,14 @@ package org.mozilla.gecko.activitystream; import android.content.Context; import android.net.Uri; import android.os.AsyncTask; +import android.support.annotation.NonNull; import android.text.TextUtils; import com.keepsafe.switchboard.SwitchBoard; +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; import org.mozilla.gecko.AppConstants; import org.mozilla.gecko.Experiments; import org.mozilla.gecko.GeckoSharedPrefs; @@ -20,6 +24,7 @@ import org.mozilla.gecko.util.StringUtils; import org.mozilla.gecko.util.publicsuffix.PublicSuffix; import java.util.Arrays; +import java.util.HashMap; import java.util.List; public class ActivityStream { diff --git a/mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStreamTelemetry.java b/mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStreamTelemetry.java new file mode 100644 index 000000000000..eace45b0a249 --- /dev/null +++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStreamTelemetry.java @@ -0,0 +1,171 @@ +/* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; indent-tabs-mode: nil; -*- + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.gecko.activitystream; + +import android.support.annotation.NonNull; + +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; +import org.mozilla.gecko.R; +import org.mozilla.gecko.db.BrowserContract; + +import java.util.HashMap; + +/** + * Telemetry constants and an 'extras' builder specific to Activity Stream. + */ +public class ActivityStreamTelemetry { + public static class Contract { + // Keys + public final static String FX_ACCOUNT_PRESENT = "fx_account_present"; + public final static String ITEM = "item"; + public final static String SOURCE_TYPE = "source_type"; + public final static String SOURCE_SUBTYPE = "source_subtype"; + public final static String ACTION_POSITION = "action_position"; + public final static String COUNT = "count"; + + // Values + public final static String TYPE_TOPSITES = "topsites"; + public final static String TYPE_HIGHLIGHTS = "highlights"; + public final static String SUBTYPE_PINNED = "pinned"; + public final static String SUBTYPE_SUGGESTED = "suggested"; + public final static String SUBTYPE_TOP = "top"; + public final static String SUBTYPE_VISITED = "visited"; + public final static String SUBTYPE_BOOKMARKED = "bookmarked"; + public final static String ITEM_SHARE = "share"; + public final static String ITEM_ADD_BOOKMARK = "add_bookmark"; + public final static String ITEM_REMOVE_BOOKMARK = "remove_bookmark"; + public final static String ITEM_PIN = "pin"; + public final static String ITEM_UNPIN = "unpin"; + public final static String ITEM_COPY = "copy"; + public final static String ITEM_ADD_TO_HOMESCREEN = "homescreen"; + public final static String ITEM_NEW_TAB = "newtab"; + public final static String ITEM_PRIVATE_TAB = "privatetab"; + public final static String ITEM_DISMISS = "dismiss"; + public final static String ITEM_DELETE_HISTORY = "delete"; + } + + /** + * A helper class used for composing an 'extras' field. It encapsulates a holder of "global" + * key/value pairs which will be present in every 'extras' constructed by this class, and a + * static builder which is aware of Activity Stream telemetry needs. + */ + public final static class Extras { + private static final HashMap globals = new HashMap<>(); + + public static void setGlobal(String key, Object value) { + globals.put(key, value); + } + + public static Builder builder() { + return new Builder(); + } + + /** + * Allows composing a JSON extras blob, which is then "built" into a string representation. + */ + public final static class Builder { + private final JSONObject data; + + private Builder() { + data = new JSONObject(globals); + } + + /** + * @param value a {@link JSONObject}, {@link JSONArray}, String, Boolean, + * Integer, Long, Double, {@link JSONObject#NULL}, or {@code null}. May not be + * {@link Double#isNaN() NaNs} or {@link Double#isInfinite() + * infinities}. + * @return this object. + */ + public Builder set(@NonNull String key, Object value) { + try { + data.put(key, value); + } catch (JSONException e) { + throw new IllegalArgumentException("Key can't be null"); + } + return this; + } + + /** + * Sets extras values describing a context menu interaction based on the menu item ID. + * + * @param itemId ID of a menu item, which is transformed into a corresponding item + * key/value pair and passed off to {@link this#set(String, Object)}. + * @return this object. + */ + public Builder fromMenuItemId(int itemId) { + switch (itemId) { + case R.id.share: + this.set(Contract.ITEM, Contract.ITEM_SHARE); + break; + + case R.id.copy_url: + this.set(Contract.ITEM, Contract.ITEM_COPY); + break; + + case R.id.add_homescreen: + this.set(Contract.ITEM, Contract.ITEM_ADD_TO_HOMESCREEN); + break; + + case R.id.open_new_tab: + this.set(Contract.ITEM, Contract.ITEM_NEW_TAB); + break; + + case R.id.open_new_private_tab: + this.set(Contract.ITEM, Contract.ITEM_PRIVATE_TAB); + break; + + case R.id.dismiss: + this.set(Contract.ITEM, Contract.ITEM_DISMISS); + break; + + case R.id.delete: + this.set(Contract.ITEM, Contract.ITEM_DELETE_HISTORY); + break; + } + return this; + } + + public Builder forHighlightSource(Utils.HighlightSource source) { + switch (source) { + case VISITED: + this.set(Contract.SOURCE_SUBTYPE, Contract.SUBTYPE_VISITED); + break; + case BOOKMARKED: + this.set(Contract.SOURCE_SUBTYPE, Contract.SUBTYPE_BOOKMARKED); + break; + default: + throw new IllegalStateException("Unknown highlight source: " + source); + } + return this; + } + + public Builder forTopSiteType(int type) { + switch (type) { + case BrowserContract.TopSites.TYPE_PINNED: + this.set(Contract.SOURCE_SUBTYPE, Contract.SUBTYPE_PINNED); + break; + case BrowserContract.TopSites.TYPE_SUGGESTED: + this.set(Contract.SOURCE_SUBTYPE, Contract.SUBTYPE_SUGGESTED); + break; + case BrowserContract.TopSites.TYPE_TOP: + this.set(Contract.SOURCE_SUBTYPE, Contract.SUBTYPE_TOP); + break; + // While we also have a "blank" type, it is not used by Activity Stream. + default: + throw new IllegalStateException("Unknown top site type: " + type); + } + return this; + } + + public String build() { + return data.toString(); + } + } + } +} diff --git a/mobile/android/base/java/org/mozilla/gecko/activitystream/Utils.java b/mobile/android/base/java/org/mozilla/gecko/activitystream/Utils.java new file mode 100644 index 000000000000..bf57d1a4941c --- /dev/null +++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/Utils.java @@ -0,0 +1,32 @@ +/* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; indent-tabs-mode: nil; -*- + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.gecko.activitystream; + +import android.database.Cursor; + +import org.mozilla.gecko.db.BrowserContract; + +/** + * Various util methods and constants that are shared by different parts of Activity Stream. + */ +public class Utils { + public enum HighlightSource { + VISITED, + BOOKMARKED + } + + public static HighlightSource highlightSource(final Cursor cursor) { + if (-1 != cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Combined.BOOKMARK_ID))) { + return HighlightSource.BOOKMARKED; + } + + if (-1 != cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Combined.HISTORY_ID))) { + return HighlightSource.VISITED; + } + + throw new IllegalArgumentException("Unknown highlight source."); + } +} diff --git a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/ActivityStream.java b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/ActivityStream.java index 2f1cf312048f..d0b9e4628170 100644 --- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/ActivityStream.java +++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/ActivityStream.java @@ -11,20 +11,17 @@ import android.os.Bundle; import android.support.v4.app.LoaderManager; import android.support.v4.content.ContextCompat; import android.support.v4.content.Loader; -import android.support.v4.graphics.ColorUtils; import android.support.v7.widget.LinearLayoutManager; import android.support.v7.widget.RecyclerView; import android.util.AttributeSet; -import android.util.DisplayMetrics; -import android.util.Log; -import android.util.TypedValue; import android.widget.FrameLayout; import org.mozilla.gecko.R; +import org.mozilla.gecko.activitystream.ActivityStreamTelemetry; import org.mozilla.gecko.db.BrowserDB; +import org.mozilla.gecko.fxa.FirefoxAccounts; import org.mozilla.gecko.home.HomePager; import org.mozilla.gecko.home.activitystream.topsites.TopSitesPagerAdapter; -import org.mozilla.gecko.util.ContextUtils; import org.mozilla.gecko.widget.RecyclerViewClickSupport; public class ActivityStream extends FrameLayout { @@ -62,6 +59,11 @@ public class ActivityStream extends FrameLayout { desiredTileWidth = resources.getDimensionPixelSize(R.dimen.activity_stream_desired_tile_width); desiredTilesHeight = resources.getDimensionPixelSize(R.dimen.activity_stream_desired_tile_height); tileMargin = resources.getDimensionPixelSize(R.dimen.activity_stream_base_margin); + + ActivityStreamTelemetry.Extras.setGlobal( + ActivityStreamTelemetry.Contract.FX_ACCOUNT_PRESENT, + FirefoxAccounts.firefoxAccountsExist(context) + ); } void setOnUrlOpenListeners(HomePager.OnUrlOpenListener onUrlOpenListener, HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener) { diff --git a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/StreamItem.java b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/StreamItem.java index b4ce1867fc4c..09b50aa0406f 100644 --- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/StreamItem.java +++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/StreamItem.java @@ -23,7 +23,11 @@ import android.widget.TextView; import org.mozilla.gecko.GeckoSharedPrefs; import org.mozilla.gecko.R; +import org.mozilla.gecko.Telemetry; +import org.mozilla.gecko.TelemetryContract; +import org.mozilla.gecko.activitystream.Utils; import org.mozilla.gecko.activitystream.ActivityStream.LabelCallback; +import org.mozilla.gecko.activitystream.ActivityStreamTelemetry; import org.mozilla.gecko.db.BrowserContract; import org.mozilla.gecko.home.HomePager; import org.mozilla.gecko.home.activitystream.menu.ActivityStreamContextMenu; @@ -135,17 +139,14 @@ public abstract class StreamItem extends RecyclerView.ViewHolder { public static class HighlightItem extends StreamItem implements IconCallback { public static final int LAYOUT_ID = R.layout.activity_stream_card_history_item; - enum HighlightSource { - VISITED, - BOOKMARKED - } - String title; String url; @Nullable Boolean isPinned; @Nullable Boolean isBookmarked; + Utils.HighlightSource source; + final FaviconView vIconView; final TextView vLabel; final TextView vTimeSince; @@ -180,12 +181,23 @@ public abstract class StreamItem extends RecyclerView.ViewHolder { menuButton.setOnClickListener(new View.OnClickListener() { @Override public void onClick(View v) { + ActivityStreamTelemetry.Extras.Builder extras = ActivityStreamTelemetry.Extras.builder() + .set(ActivityStreamTelemetry.Contract.SOURCE_TYPE, ActivityStreamTelemetry.Contract.TYPE_HIGHLIGHTS) + .forHighlightSource(source); + ActivityStreamContextMenu.show(v.getContext(), menuButton, + extras, ActivityStreamContextMenu.MenuMode.HIGHLIGHT, title, url, isBookmarked, isPinned, onUrlOpenListener, onUrlOpenInBackgroundListener, vIconView.getWidth(), vIconView.getHeight()); + + Telemetry.sendUIEvent( + TelemetryContract.Event.SHOW, + TelemetryContract.Method.CONTEXT_MENU, + extras.build() + ); } }); @@ -193,12 +205,12 @@ public abstract class StreamItem extends RecyclerView.ViewHolder { } public void bind(Cursor cursor, int tilesWidth, int tilesHeight) { - final long time = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.DATE)); final String ago = DateUtils.getRelativeTimeSpanString(time, System.currentTimeMillis(), DateUtils.MINUTE_IN_MILLIS, 0).toString(); title = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.History.TITLE)); url = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Combined.URL)); + source = Utils.highlightSource(cursor); vLabel.setText(title); vTimeSince.setText(ago); @@ -208,9 +220,7 @@ public abstract class StreamItem extends RecyclerView.ViewHolder { layoutParams.height = tilesHeight; vIconView.setLayoutParams(layoutParams); - final HighlightSource source = highlightSource(cursor); - - updateStateForSource(source, cursor); + updateStateForSource(source); updateUiForSource(source); updatePage(url); @@ -225,7 +235,7 @@ public abstract class StreamItem extends RecyclerView.ViewHolder { .execute(this); } - private void updateStateForSource(HighlightSource source, Cursor cursor) { + private void updateStateForSource(Utils.HighlightSource source) { // We can only be certain of bookmark state if an item is a bookmark item. // Otherwise, due to the underlying highlights query, we have to look up states when // menus are displayed. @@ -243,7 +253,7 @@ public abstract class StreamItem extends RecyclerView.ViewHolder { } } - private void updateUiForSource(HighlightSource source) { + private void updateUiForSource(Utils.HighlightSource source) { switch (source) { case BOOKMARKED: vSourceView.setText(R.string.activity_stream_highlight_label_bookmarked); @@ -279,16 +289,4 @@ public abstract class StreamItem extends RecyclerView.ViewHolder { vIconView.updateImage(response); } } - - private static HighlightItem.HighlightSource highlightSource(final Cursor cursor) { - if (-1 != cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Combined.BOOKMARK_ID))) { - return HighlightItem.HighlightSource.BOOKMARKED; - } - - if (-1 != cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Combined.HISTORY_ID))) { - return HighlightItem.HighlightSource.VISITED; - } - - throw new IllegalArgumentException("Unknown highlight source."); - } } diff --git a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/StreamRecyclerAdapter.java b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/StreamRecyclerAdapter.java index aaa10b2dfb80..8e21035e9803 100644 --- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/StreamRecyclerAdapter.java +++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/StreamRecyclerAdapter.java @@ -13,6 +13,8 @@ import android.view.ViewGroup; import org.mozilla.gecko.Telemetry; import org.mozilla.gecko.TelemetryContract; +import org.mozilla.gecko.activitystream.ActivityStreamTelemetry; +import org.mozilla.gecko.activitystream.Utils; import org.mozilla.gecko.db.BrowserContract; import org.mozilla.gecko.home.HomePager; import org.mozilla.gecko.home.activitystream.StreamItem.HighlightItem; @@ -113,15 +115,25 @@ public class StreamRecyclerAdapter extends RecyclerView.Adapter impl return; } - highlightsCursor.moveToPosition( - translatePositionToCursor(position)); + int actualPosition = translatePositionToCursor(position); + highlightsCursor.moveToPosition(actualPosition); final String url = highlightsCursor.getString( highlightsCursor.getColumnIndexOrThrow(BrowserContract.Combined.URL)); onUrlOpenListener.onUrlOpen(url, EnumSet.of(HomePager.OnUrlOpenListener.Flags.ALLOW_SWITCH_TO_TAB)); - Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.LIST_ITEM, "as_highlights"); + ActivityStreamTelemetry.Extras.Builder extras = ActivityStreamTelemetry.Extras.builder() + .forHighlightSource(Utils.highlightSource(highlightsCursor)) + .set(ActivityStreamTelemetry.Contract.SOURCE_TYPE, ActivityStreamTelemetry.Contract.TYPE_HIGHLIGHTS) + .set(ActivityStreamTelemetry.Contract.ACTION_POSITION, actualPosition) + .set(ActivityStreamTelemetry.Contract.COUNT, highlightsCursor.getCount()); + + Telemetry.sendUIEvent( + TelemetryContract.Event.LOAD_URL, + TelemetryContract.Method.LIST_ITEM, + extras.build() + ); } @Override diff --git a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/ActivityStreamContextMenu.java b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/ActivityStreamContextMenu.java index d9a8bd0766fe..eaa378ebc66a 100644 --- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/ActivityStreamContextMenu.java +++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/ActivityStreamContextMenu.java @@ -18,6 +18,7 @@ import org.mozilla.gecko.IntentHelper; import org.mozilla.gecko.R; import org.mozilla.gecko.Telemetry; import org.mozilla.gecko.TelemetryContract; +import org.mozilla.gecko.activitystream.ActivityStreamTelemetry; import org.mozilla.gecko.annotation.RobocopTarget; import org.mozilla.gecko.db.BrowserDB; import org.mozilla.gecko.home.HomePager; @@ -43,6 +44,8 @@ public abstract class ActivityStreamContextMenu final String title; final String url; + private final ActivityStreamTelemetry.Extras.Builder telemetryExtraBuilder; + // We might not know bookmarked/pinned states, so we allow for null values. private @Nullable Boolean isBookmarked; private @Nullable Boolean isPinned; @@ -60,12 +63,14 @@ public abstract class ActivityStreamContextMenu final MenuMode mode; /* package-private */ ActivityStreamContextMenu(final Context context, + final ActivityStreamTelemetry.Extras.Builder telemetryExtraBuilder, final MenuMode mode, final String title, @NonNull final String url, @Nullable final Boolean isBookmarked, @Nullable final Boolean isPinned, HomePager.OnUrlOpenListener onUrlOpenListener, HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener) { this.context = context; + this.telemetryExtraBuilder = telemetryExtraBuilder; this.mode = mode; @@ -156,13 +161,12 @@ public abstract class ActivityStreamContextMenu @Override protected Void doInBackground() { final Cursor cursor = BrowserDB.from(context).getHistoryForURL(context.getContentResolver(), url); + if (cursor == null) { + hasHistory = false; + return null; + } try { - if (cursor != null && - cursor.getCount() == 1) { - hasHistory = true; - } else { - hasHistory = false; - } + hasHistory = cursor.getCount() == 1; } finally { cursor.close(); } @@ -181,49 +185,76 @@ public abstract class ActivityStreamContextMenu @Override public boolean onNavigationItemSelected(MenuItem item) { + final int menuItemId = item.getItemId(); + + // Sets extra telemetry which doesn't require additional state information. + // Pin and bookmark items are handled separately below, since they do require state + // information to handle correctly. + telemetryExtraBuilder.fromMenuItemId(menuItemId); + switch (item.getItemId()) { case R.id.share: + // NB: Generic menu item action event will be sent at the end of this function. + // We have a seemingly duplicate telemetry event here because we want to emit + // a concrete event in case it is used by other queries to estimate feature usage. Telemetry.sendUIEvent(TelemetryContract.Event.SHARE, TelemetryContract.Method.LIST, "as_contextmenu"); IntentHelper.openUriExternal(url, "text/plain", "", "", Intent.ACTION_SEND, title, false); break; case R.id.bookmark: + final TelemetryContract.Event telemetryEvent; + final String telemetryExtra; + SavedReaderViewHelper rch = SavedReaderViewHelper.getSavedReaderViewHelper(context); + final boolean isReaderViewPage = rch.isURLCached(url); + + // While isBookmarked is nullable, behaviour of postInit - disabling 'bookmark' item + // until we know value of isBookmarked - guarantees that it will be set when we get here. + if (isBookmarked) { + telemetryEvent = TelemetryContract.Event.UNSAVE; + + if (isReaderViewPage) { + telemetryExtra = "as_bookmark_reader"; + } else { + telemetryExtra = "as_bookmark"; + } + telemetryExtraBuilder.set(ActivityStreamTelemetry.Contract.ITEM, ActivityStreamTelemetry.Contract.ITEM_REMOVE_BOOKMARK); + } else { + telemetryEvent = TelemetryContract.Event.SAVE; + telemetryExtra = "as_bookmark"; + telemetryExtraBuilder.set(ActivityStreamTelemetry.Contract.ITEM, ActivityStreamTelemetry.Contract.ITEM_ADD_BOOKMARK); + } + // NB: Generic menu item action event will be sent at the end of this function. + // We have a seemingly duplicate telemetry event here because we want to emit + // a concrete event in case it is used by other queries to estimate feature usage. + Telemetry.sendUIEvent(telemetryEvent, TelemetryContract.Method.CONTEXT_MENU, telemetryExtra); + ThreadUtils.postToBackgroundThread(new Runnable() { @Override public void run() { final BrowserDB db = BrowserDB.from(context); - final TelemetryContract.Event telemetryEvent; - final String telemetryExtra; if (isBookmarked) { db.removeBookmarksWithURL(context.getContentResolver(), url); - SavedReaderViewHelper rch = SavedReaderViewHelper.getSavedReaderViewHelper(context); - final boolean isReaderViewPage = rch.isURLCached(url); - - telemetryEvent = TelemetryContract.Event.UNSAVE; - - if (isReaderViewPage) { - telemetryExtra = "as_bookmark_reader"; - } else { - telemetryExtra = "as_bookmark"; - } } else { // We only store raw URLs in history (and bookmarks), hence we won't ever show about:reader // URLs in AS topsites or highlights. Therefore we don't need to do any special about:reader handling here. db.addBookmark(context.getContentResolver(), title, url); - - telemetryEvent = TelemetryContract.Event.SAVE; - telemetryExtra = "as_bookmark"; } - - Telemetry.sendUIEvent(telemetryEvent, TelemetryContract.Method.CONTEXT_MENU, telemetryExtra); } }); break; case R.id.pin: + // While isPinned is nullable, behaviour of postInit - disabling 'pin' item + // until we know value of isPinned - guarantees that it will be set when we get here. + if (isPinned) { + telemetryExtraBuilder.set(ActivityStreamTelemetry.Contract.ITEM, ActivityStreamTelemetry.Contract.ITEM_UNPIN); + } else { + telemetryExtraBuilder.set(ActivityStreamTelemetry.Contract.ITEM, ActivityStreamTelemetry.Contract.ITEM_PIN); + } + ThreadUtils.postToBackgroundThread(new Runnable() { @Override public void run() { @@ -236,6 +267,7 @@ public abstract class ActivityStreamContextMenu } } }); + break; case R.id.copy_url: Clipboard.setText(url); @@ -243,20 +275,14 @@ public abstract class ActivityStreamContextMenu case R.id.add_homescreen: GeckoAppShell.createShortcut(title, url); - - Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.CONTEXT_MENU, "as_add_to_launcher"); break; case R.id.open_new_tab: onUrlOpenInBackgroundListener.onUrlOpenInBackground(url, EnumSet.noneOf(HomePager.OnUrlOpenInBackgroundListener.Flags.class)); - - Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.CONTEXT_MENU, "as_new_tab"); break; case R.id.open_new_private_tab: onUrlOpenInBackgroundListener.onUrlOpenInBackground(url, EnumSet.of(HomePager.OnUrlOpenInBackgroundListener.Flags.PRIVATE)); - - Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.CONTEXT_MENU, "as_private_tab"); break; case R.id.dismiss: @@ -285,6 +311,12 @@ public abstract class ActivityStreamContextMenu throw new IllegalArgumentException("Menu item with ID=" + item.getItemId() + " not handled"); } + Telemetry.sendUIEvent( + TelemetryContract.Event.ACTION, + TelemetryContract.Method.CONTEXT_MENU, + telemetryExtraBuilder.build() + ); + dismiss(); return true; } @@ -292,7 +324,7 @@ public abstract class ActivityStreamContextMenu @RobocopTarget public static ActivityStreamContextMenu show(Context context, - View anchor, + View anchor, ActivityStreamTelemetry.Extras.Builder telemetryExtraBuilder, final MenuMode menuMode, final String title, @NonNull final String url, @Nullable final Boolean isBookmarked, @Nullable final Boolean isPinned, @@ -303,14 +335,14 @@ public abstract class ActivityStreamContextMenu if (!HardwareUtils.isTablet()) { menu = new BottomSheetContextMenu(context, - menuMode, + telemetryExtraBuilder, menuMode, title, url, isBookmarked, isPinned, onUrlOpenListener, onUrlOpenInBackgroundListener, tilesWidth, tilesHeight); } else { menu = new PopupContextMenu(context, anchor, - menuMode, + telemetryExtraBuilder, menuMode, title, url, isBookmarked, isPinned, onUrlOpenListener, onUrlOpenInBackgroundListener); } diff --git a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/BottomSheetContextMenu.java b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/BottomSheetContextMenu.java index 6e6a8a84521e..9c397884455d 100644 --- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/BottomSheetContextMenu.java +++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/BottomSheetContextMenu.java @@ -7,7 +7,6 @@ package org.mozilla.gecko.home.activitystream.menu; import android.content.Context; import android.support.annotation.NonNull; import android.support.annotation.Nullable; -import android.support.design.widget.BottomSheetBehavior; import android.support.design.widget.BottomSheetDialog; import android.support.design.widget.NavigationView; import android.view.LayoutInflater; @@ -18,6 +17,7 @@ import android.widget.TextView; import org.mozilla.gecko.R; import org.mozilla.gecko.activitystream.ActivityStream; +import org.mozilla.gecko.activitystream.ActivityStreamTelemetry; import org.mozilla.gecko.home.HomePager; import org.mozilla.gecko.icons.IconCallback; import org.mozilla.gecko.icons.IconResponse; @@ -35,6 +35,7 @@ import static org.mozilla.gecko.activitystream.ActivityStream.extractLabel; private final NavigationView navigationView; public BottomSheetContextMenu(final Context context, + final ActivityStreamTelemetry.Extras.Builder telemetryExtraBuilder, final MenuMode mode, final String title, @NonNull final String url, @Nullable final Boolean isBookmarked, @Nullable final Boolean isPinned, @@ -43,6 +44,7 @@ import static org.mozilla.gecko.activitystream.ActivityStream.extractLabel; final int tilesWidth, final int tilesHeight) { super(context, + telemetryExtraBuilder, mode, title, url, diff --git a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/PopupContextMenu.java b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/PopupContextMenu.java index 911e5f754de4..5c7af98d48c9 100644 --- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/PopupContextMenu.java +++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/PopupContextMenu.java @@ -16,6 +16,8 @@ import android.view.View; import android.widget.PopupWindow; import org.mozilla.gecko.R; +import org.mozilla.gecko.activitystream.ActivityStream; +import org.mozilla.gecko.activitystream.ActivityStreamTelemetry; import org.mozilla.gecko.home.HomePager; /* package-private */ class PopupContextMenu @@ -28,6 +30,7 @@ import org.mozilla.gecko.home.HomePager; public PopupContextMenu(final Context context, View anchor, + final ActivityStreamTelemetry.Extras.Builder telemetryExtraBuilder, final MenuMode mode, final String title, @NonNull final String url, @@ -36,6 +39,7 @@ import org.mozilla.gecko.home.HomePager; HomePager.OnUrlOpenListener onUrlOpenListener, HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener) { super(context, + telemetryExtraBuilder, mode, title, url, diff --git a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/topsites/TopSitesCard.java b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/topsites/TopSitesCard.java index 7972ca29ac26..7d5fe6f9ad2e 100644 --- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/topsites/TopSitesCard.java +++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/topsites/TopSitesCard.java @@ -17,6 +17,7 @@ import org.mozilla.gecko.R; import org.mozilla.gecko.Telemetry; import org.mozilla.gecko.TelemetryContract; import org.mozilla.gecko.activitystream.ActivityStream; +import org.mozilla.gecko.activitystream.ActivityStreamTelemetry; import org.mozilla.gecko.db.BrowserContract; import org.mozilla.gecko.home.HomePager; import org.mozilla.gecko.home.activitystream.menu.ActivityStreamContextMenu; @@ -103,13 +104,22 @@ class TopSitesCard extends RecyclerView.ViewHolder @Override public void onClick(View clickedView) { + ActivityStreamTelemetry.Extras.Builder extras = ActivityStreamTelemetry.Extras.builder() + .set(ActivityStreamTelemetry.Contract.SOURCE_TYPE, ActivityStreamTelemetry.Contract.TYPE_TOPSITES) + .forTopSiteType(type); + if (clickedView == itemView) { onUrlOpenListener.onUrlOpen(url, EnumSet.noneOf(HomePager.OnUrlOpenListener.Flags.class)); - Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.LIST_ITEM, "as_top_sites"); + Telemetry.sendUIEvent( + TelemetryContract.Event.LOAD_URL, + TelemetryContract.Method.LIST_ITEM, + extras.build() + ); } else if (clickedView == menuButton) { ActivityStreamContextMenu.show(clickedView.getContext(), menuButton, + extras, ActivityStreamContextMenu.MenuMode.TOPSITE, title.getText().toString(), url, @@ -118,11 +128,15 @@ class TopSitesCard extends RecyclerView.ViewHolder onUrlOpenListener, onUrlOpenInBackgroundListener, faviconView.getWidth(), faviconView.getHeight()); - Telemetry.sendUIEvent(TelemetryContract.Event.SHOW, TelemetryContract.Method.CONTEXT_MENU, "as_top_sites"); + Telemetry.sendUIEvent( + TelemetryContract.Event.SHOW, + TelemetryContract.Method.CONTEXT_MENU, + extras.build() + ); } } - private static boolean isPinned(int type) { + private boolean isPinned(int type) { return type == BrowserContract.TopSites.TYPE_PINNED; } } diff --git a/mobile/android/base/moz.build b/mobile/android/base/moz.build index 58c04dca4e99..3c801a6df0c4 100644 --- a/mobile/android/base/moz.build +++ b/mobile/android/base/moz.build @@ -334,6 +334,8 @@ gbjar.sources += ['java/org/mozilla/gecko/' + x for x in [ 'ActionModeCompatView.java', 'ActivityHandlerHelper.java', 'activitystream/ActivityStream.java', + 'activitystream/ActivityStreamTelemetry.java', + 'activitystream/Utils.java', 'adjust/AdjustBrowserAppDelegate.java', 'animation/AnimationUtils.java', 'animation/HeightChangeAnimation.java', diff --git a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testActivityStreamContextMenu.java b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testActivityStreamContextMenu.java index f32eadf30f30..5ebef2b427ec 100644 --- a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testActivityStreamContextMenu.java +++ b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testActivityStreamContextMenu.java @@ -10,6 +10,8 @@ import android.view.View; import com.robotium.solo.Condition; import org.mozilla.gecko.R; +import org.mozilla.gecko.activitystream.ActivityStream; +import org.mozilla.gecko.activitystream.ActivityStreamTelemetry; import org.mozilla.gecko.db.BrowserDB; import org.mozilla.gecko.home.activitystream.menu.ActivityStreamContextMenu; @@ -61,7 +63,8 @@ public class testActivityStreamContextMenu extends BaseTest { private void testMenuForUrl(final String url, final Boolean isBookmarkedKnownState, final boolean isBookmarked, final Boolean isPinnedKnownState, final boolean isPinned, final boolean isVisited) { final View anchor = new View(getActivity()); - final ActivityStreamContextMenu menu = ActivityStreamContextMenu.show(getActivity(), anchor, ActivityStreamContextMenu.MenuMode.HIGHLIGHT, "foobar", url, isBookmarkedKnownState, isPinnedKnownState, null, null, 100, 100); + final ActivityStreamContextMenu menu = ActivityStreamContextMenu.show( + getActivity(), anchor, ActivityStreamTelemetry.Extras.builder(), ActivityStreamContextMenu.MenuMode.HIGHLIGHT, "foobar", url, isBookmarkedKnownState, isPinnedKnownState, null, null, 100, 100); final int expectedBookmarkString; if (isBookmarked) {