1329156 - Custom tabs: Collect information on custom tab customization features used by 3rd party apps

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Joe Cheng [:jcheng] (please needinfo)]

As Mozilla, we want to understand which custom tab customization features are being used by 3rd party apps

Comment 1

[Rachelle Yang [:ryang][[e-mail]]]

After main feature of custom tab is landed and completed, this will be implemented to collect telemetry information.

Comment 2

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1329156 - Refactor: Utility class should be final.

Review commit: https://reviewboard.mozilla.org/r/129266/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129266/

Comment 3

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1329156 - Add telemetry for customtab customization usage.

Review commit: https://reviewboard.mozilla.org/r/129268/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129268/

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8857306 [details]
Bug 1329156 - Refactor: Utility class should be final.

https://reviewboard.mozilla.org/r/129266/#review131976

Not sure of the win here without a private constructor though?

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8857307 [details]
Bug 1329156 - Add telemetry for customtab customization usage.

https://reviewboard.mozilla.org/r/129268/#review131978

::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:254
(Diff revision 1)
> +        try {
> +            String extras = getResources().getResourceEntryName(item.getItemId());
> +            Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.MENU, "customtab-" + extras);
> +        } catch (Resources.NotFoundException e) {
> +            Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.MENU, "customtab-custom-menu");
> +        }

I'd prefer us to be more explicit and add telemetry requests to the specific cases. Otherwise we might end up sending telemetry for things we didn't want to - especially in future iterations.

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

I've forwarded the review request to :walkingice. I consider him the owner of the custom tabs code and he will know best how/where to integrate the probes. :)

Comment 7

[Julian Chu [:walkingice]]

Comment on attachment 8857307 [details]
Bug 1329156 - Add telemetry for customtab customization usage.

https://reviewboard.mozilla.org/r/129268/#review132840

::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:98
(Diff revision 1)
>  
>          actionBarPresenter = new ActionBarPresenter(actionBar);
>          actionBarPresenter.displayUrlOnly(startIntent.getDataString());
> +        if (IntentUtil.hasToolbarColor(startIntent)) {
> +            Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.INTENT, "customtab-hasToolbarColor");
> +        }

Maybe we can create a method which contains Telemetry initialization checking. The method can be placed in activity first run[1]. Then "customtab-hasToolbarColor", "customtab-hasActionButton", "isActionButtonTinted"...etc can be aggregated in one place.

[1] https://dxr.mozilla.org/mozilla-central/rev/f40e24f40b4c4556944c762d4764eace261297f5/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java#73

::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:254
(Diff revision 1)
> +        try {
> +            String extras = getResources().getResourceEntryName(item.getItemId());
> +            Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.MENU, "customtab-" + extras);
> +        } catch (Resources.NotFoundException e) {
> +            Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.MENU, "customtab-custom-menu");
> +        }

I agree with Sebastian. The menu item id is generated by code so it is supposed to be meaningless for us. If we care about whether user click to trigger pending-intent, we can do something like

```
final PendingIntent intent = menuItemsIntent.get(item.getItemId());
if (intent != null) {
    onCustomMenuItemClicked(intent);
    return true;
}
.....
private void onCustomMenuItemClicked(PendingIntent intent) {
    // send Telemetry here for custom-menu-item
    performPendingIntent(intent);
}
```

::: mobile/android/base/java/org/mozilla/gecko/customtabs/IntentUtil.java:102
(Diff revision 1)
> +     * @param intent which to launch a Custom-Tabs-Activity
> +     * @return true, if the caller customized the color.
> +     */
> +    static boolean hasToolbarColor(@NonNull Intent intent) {
> +        @ColorInt int toolbarColor = intent.getIntExtra(CustomTabsIntent.EXTRA_TOOLBAR_COLOR,
> +                Integer.MAX_VALUE);

what if we use intent.hasExtra()?

Comment 8

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8857307 [details]
Bug 1329156 - Add telemetry for customtab customization usage.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129268/diff/1-2/

Comment 9

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8857307 [details]
Bug 1329156 - Add telemetry for customtab customization usage.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129268/diff/2-3/

Comment 10

[Julian Chu [:walkingice]]

Comment on attachment 8857307 [details]
Bug 1329156 - Add telemetry for customtab customization usage.

https://reviewboard.mozilla.org/r/129268/#review136788

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4847c0e2a3e1
Add telemetry for customtab customization usage. r=walkingice

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4847c0e2a3e1