988650 - Telemetry: Sharing

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Chenxia Liu [:liuche]]

The goal of this probe is to measure how and how much Share is used.

Things to measure:
* Sharing through a quickshare button
* Scrolling through the full list of Share services

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: uitelemetry-sharing v0.1

This patch adds telemetry Events to the various places where Sharing is executed. I think I found all the places. I added "share.1" as an Event and "list" and "button" as Methods. I tried to make the Methods reusable for other Events. Methods should not carry too much context.

The only place where we have a Session to help get context for now is "home.1" and "homepanel.1:<top-sites>". If we need contextmenu and mainmenu Sessions, we can add them in different bugs.

Notes:
1. In PromptListAdapter, I fire the Event *after* a target is selected from the chooser. This is the only place we do that. Maybe I should move the Event out of the onIntentSelected and just put it in the onClick, for consistency.
2. I don't think BrowserApp.shareCurrentUrl is really used anymore. There is no "Share" in the URLBar context menu and the "Share" on the main menu is handled by the GeckoActionProvider. Maybe we should just remove that code in a new bug.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Mark Finkle (:mfinkle) from comment #1)

> 2. I don't think BrowserApp.shareCurrentUrl is really used anymore. There is
> no "Share" in the URLBar context menu and the "Share" on the main menu is
> handled by the GeckoActionProvider. Maybe we should just remove that code in
> a new bug.

I was wrong. This is used on Gingerbread and lower devices where we still have the "Share" URLBar contextmenu item.

Comment 3

[Wesley Johnston (:wesj)]

Comment on attachment 8413260 [details] [diff] [review]
uitelemetry-sharing v0.1

Review of attachment 8413260 [details] [diff] [review]:
-----------------------------------------------------------------

I think you need to remove one of these, but looks good otherwise.

::: mobile/android/base/prompts/PromptListAdapter.java
@@ +200,5 @@
>                      public void onIntentSelected(final Intent intent, final int p) {
>                          provider.chooseActivity(p);
> +
> +                        // Context: Sharing via content contextmenu list (no explicit session is active)
> +                        Telemetry.sendUIEvent(TelemetryContract.Event.SHARE, TelemetryContract.Method.LIST);

After talking to you the other day, I don't know that we want this. This will wind up firing through the GeckoActionProvider and get counted again there. i.e. Share will get double counted this way.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Wesley Johnston (:wesj) from comment #3)

> After talking to you the other day, I don't know that we want this. This
> will wind up firing through the GeckoActionProvider and get counted again
> there. i.e. Share will get double counted this way.

Wes and I talked on IRC and we discovered that the patch will not double count shares. It's OK.

Comment 5

[Chenxia Liu [:liuche]]

Comment on attachment 8413260 [details] [diff] [review]
uitelemetry-sharing v0.1

Review of attachment 8413260 [details] [diff] [review]:
-----------------------------------------------------------------

Just a few things I noticed:
- There's an inconsistency between HomeFragment sharing and all other forms of sharing, in that from a HomeFragment, we send a message for opening the list of share providers, whereas for everything else (menu, context menu from webpages, etc) we send the event when you tap on the share provider.
- I searched for ACTION_SEND, and there are a few other places we might want to instrument share (if what we care about is "how do people share"):
  - Sharing from Reader Mode (BrowserApp handling Reader:Share)
  - Sharing from text selection (GeckoApp handling Share:Text)

::: mobile/android/base/BrowserApp.java
@@ +896,5 @@
>          GeckoAppShell.openUriExternal(url, "text/plain", "", "",
>                                        Intent.ACTION_SEND, tab.getDisplayTitle());
> +
> +        // Context: Sharing via chrome list (no explicit session is active)
> +        Telemetry.sendUIEvent(TelemetryContract.Event.SHARE, TelemetryContract.Method.LIST);

This is also called when you click Share from the context menu.

If we want to use CONTEXT_MENU as a method (which will land with bug 977196), we'll want to put the Event calls into each of the callers of shareCurrentUrl().

::: mobile/android/base/home/HomeFragment.java
@@ +135,5 @@
>                  GeckoAppShell.openUriExternal(info.url, SHARE_MIME_TYPE, "", "",
>                                                Intent.ACTION_SEND, info.getDisplayTitle());
> +
> +                // Context: Sharing via chrome homepage contextmenu list (home session should be active)
> +                Telemetry.sendUIEvent(TelemetryContract.Event.SHARE, TelemetryContract.Method.LIST);

Maybe we want to use CONTEXT_MENU from bug 977196 with this?

Comment 6

[Chenxia Liu [:liuche]]

> - I searched for ACTION_SEND, and there are a few other places we might want
> to instrument share (if what we care about is "how do people share"):
>   - Sharing from Reader Mode (BrowserApp handling Reader:Share)
Oops, forgot I reviewed this in bug 986095!

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Sharing is a bit of a mess when it comes to trying to get a context and coming up with Methods.
1. Main menu has both Quick share buttons and the button to display the list
2. Web content context menu has quick share buttons and button/item to display the raw Android list
3. HomeFragment has a context menu item item that displays the raw Android list
4. Actionbar (text selection) has a button to display the raw Android list
5. URLBar (gingerbread) has a context menu item to display the raw Android list
6. Reader viewer has a button to display the raw Android list

Notes:
1. Fennec is out of the loop by the time the raw Android list is shown, so we need to fire an Event before that.
2. Since mainmenu and web contextmenu have both quick share and list-share items, we can't only use the same Method (contextmenu) for both. We need to differentiate them. We could use an Extra for quickshare.
3. We end up using the same code path for mainmenu and web contextmenu quick share. I did not find a place to add a click/tap handler that was different for main and context menus. We will *not* know from the Event that the mainmenu or contextmenu started the quick share. If we care which is which, we'll need to use Sessions some how, but not in this bug.

The goal of this bug is to measure: Quick Share vs Share from list

TODO:
1. Add actionbar (text selection) probe
2. Use Method=button for quick share and Method=list for all else. Notes #2 and #3 make it too complex to use Method=contextmenu and the goal of the bug doesn't really care about that.
3. If we need more context, we can look into adding some Sessions for actionbar, mainmenu and web contextmenu in separate bugs.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: uitelemetry-sharing v0.2

This patch adds a probe for Share:Text

Note to self: Share:Text and Reader:Share are too similar to remain separate. File a mentor bug for kill Share:Text and make Reader:Text more generic.

Comment 9

[Chenxia Liu [:liuche]]

Comment on attachment 8415752 [details] [diff] [review]
uitelemetry-sharing v0.2

Review of attachment 8415752 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the super-detailed explanation and clarifying that we're looking to compare how people use Quickshare vs list-share. I'm good with this iteration.

::: mobile/android/base/GeckoApp.java
@@ +636,5 @@
>                  String text = message.getString("text");
>                  GeckoAppShell.openUriExternal(text, "text/plain", "", "", Intent.ACTION_SEND, "");
> +
> +                // Context: Sharing via chrome list (no explicit session is active)
> +                Telemetry.sendUIEvent(TelemetryContract.Event.SHARE, TelemetryContract.Method.LIST);

Want to add a "text" to the extra, if we're adding this probe anyways?

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Chenxia Liu [:liuche] from comment #9)

> Want to add a "text" to the extra, if we're adding this probe anyways?

Since we don't use Extras for the other places, I skipped it for now. But I do intend to add an Actionbar Session soon. That would give us more context about the text-selection sharing.

remote:   https://hg.mozilla.org/integration/fx-team/rev/a41caa97af9e

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a41caa97af9e

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8415752 [details] [diff] [review]
uitelemetry-sharing v0.2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: We miss out on some "before changes" telemetry that could show affect of Fx32 changes
Testing completed (on m-c, etc.): Working and being analyzed
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/a7e0bd79002e

Comment 14

[Flaviu Cos]

The following probes are logged in logcat:
1. Share text:
SendUIEvent: action = share.1 method = list timestamp = 3348870 extras = null
2. Share via context menu:
SendUIEvent: action = action.1 method = contextmenu timestamp = 3406517 extras = home_share
SendUIEvent: action = share.1 method = list timestamp = 3406523 extras = null
3. Quick share:
SendUIEvent: action = share.1 method = button timestamp = 3489024 extras = null
4. Share via Menu -> Share:
SendUIEvent: action = share.1 method = list timestamp = 3708619 extras = null

Nothing is logged when scrolling the share list;
Nothing is logged when sharing from reader mode;
I this as expected?

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Flaviu Cos, QA [:flaviu] from comment #14)

> Nothing is logged when scrolling the share list;

Scrolling is not logged or tracked. Only an actual share.

> Nothing is logged when sharing from reader mode;

Reader mode is tracked from JavaScript, not Java. Nothing will be output to logcat.

Comment 16

[Flaviu Cos]

Verified as fixed in builds:
- 32.0a1 (2014-06-03);
- 31.0a2 (2014-06-03);
Device: Motorola RAZR (Android 4.0.4).