Closed Bug 1329156 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: jcheng, Assigned: cnevinchen)

References

Details

Attachments

(1 file, 1 obsolete file)

As Mozilla, we want to understand which custom tab customization features are being used by 3rd party apps
After main feature of custom tab is landed and completed, this will be implemented to collect telemetry information.
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?
Attachment #8857306 - Flags: review?(s.kaspari) → review+
Attachment #8857307 - Flags: review?(s.kaspari) → review?(walkingice0204)
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.
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 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()?
Assignee: nobody → cnevinchen
Attachment #8857306 - Attachment is obsolete: true
Comment on attachment 8857307 [details]
Bug 1329156 - Add telemetry for customtab customization usage.

https://reviewboard.mozilla.org/r/129268/#review136788
Attachment #8857307 - Flags: review?(walkingice0204) → review+
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4847c0e2a3e1
Add telemetry for customtab customization usage. r=walkingice
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4847c0e2a3e1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: