1392538 - Remove unused Kinto settings

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, enhancement, P2)

Description

[Nevin Chen(Not active on Bugzilla)]

We had many unused Kinto settings now. e.g. Onboarding experiments.
We should investigate and see what we really need to reduce the Kinto config download size.

Comment 1

[Nevin Chen(Not active on Bugzilla)]

Candidates to remove with NI (not used or already on by default)
- custom-tabs
- compact-tabs
- urlbar-show-origin-only
- urlbar-show-ev-cert-owner
- content-notifications-12hrs
- whatsnew-notification
- content-notifications-8am
- content-notifications-5pm

Evaluate before remove
- top-addons-menu
- offline-cache
- promote-add-to-homescreen
- triple-readerview-bookmark-prompt
- bookmark-history-menu

Stays
- leanplum-start
- full-bookmark-management
- activity-stream-opt-out
- process-background-telemetry
- activity-stream-setting
- activity-stream
- download-content-catalog-sync
- hls-video-playback

Comment 2

[Nevin Chen(Not active on Bugzilla)]

Looks like belong are already removed code.
We can remove them from Kinto after 57 goes to release
- custom-tabs
- compact-tabs
- urlbar-show-origin-only
- urlbar-show-ev-cert-owner
- content-notifications-12hrs
- whatsnew-notification
- content-notifications-8am
- content-notifications-5pm
- top-addons-menu
- offline-cache
- bookmark-history-menu

Comment 3

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1392538 - Remove unused onboarding experiments.

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

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8907403 [details]
Bug 1392538 - Remove unused onboarding experiments.

https://reviewboard.mozilla.org/r/179086/#review184266

::: mobile/android/base/java/org/mozilla/gecko/firstrun/FirstrunAnimationContainer.java
(Diff revision 1)
>          }
>          animateHide();
> -
> -        // Stop all versions of firstrun A/B sessions.
> -        Telemetry.stopUISession(TelemetryContract.Session.EXPERIMENT, Experiments.ONBOARDING3_B);
> -        Telemetry.stopUISession(TelemetryContract.Session.EXPERIMENT, Experiments.ONBOARDING3_C);

Is there no matching startUISession call?

::: mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryCorePingBuilder.java
(Diff revision 1)
>          payload.put(DEVICE, deviceDescriptor);
>          payload.put(LOCALE, Locales.getLanguageTag(Locale.getDefault()));
>          payload.put(OS_VERSION, Integer.toString(Build.VERSION.SDK_INT)); // A String for cross-platform reasons.
>          payload.put(PING_CREATION_DATE, pingCreationDateFormat.format(nowCalendar.getTime()));
>          payload.put(TIMEZONE_OFFSET, DateUtil.getTimezoneOffsetInMinutesForGivenDate(nowCalendar));
> -        payload.putArray(EXPERIMENTS, Experiments.getActiveExperiments(context));

Why do you remove the list of experiments from the core ping?

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

> - custom-tabs

This one should still be used for shipping custom tabs. Somehow the code was backed out in bug 1329152 for no reason.

> - urlbar-show-origin-only
> - urlbar-show-ev-cert-owner

They are still used in the code.

> - whatsnew-notification

This one is also still used and can "trigger what's new" notifications if needed.

> - top-addons-menu

This experiment hasn't even reached release yet (bug 1366681).

Comment 6

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8907403 [details]
Bug 1392538 - Remove unused onboarding experiments.

https://reviewboard.mozilla.org/r/179086/#review185252

::: mobile/android/base/java/org/mozilla/gecko/firstrun/FirstrunAnimationContainer.java
(Diff revision 1)
>          }
>          animateHide();
> -
> -        // Stop all versions of firstrun A/B sessions.
> -        Telemetry.stopUISession(TelemetryContract.Session.EXPERIMENT, Experiments.ONBOARDING3_B);
> -        Telemetry.stopUISession(TelemetryContract.Session.EXPERIMENT, Experiments.ONBOARDING3_C);

We remove all the oboarding experiments after photon since we are pianning for new onboarding design. So yes, they are not used.

::: mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryCorePingBuilder.java
(Diff revision 1)
>          payload.put(DEVICE, deviceDescriptor);
>          payload.put(LOCALE, Locales.getLanguageTag(Locale.getDefault()));
>          payload.put(OS_VERSION, Integer.toString(Build.VERSION.SDK_INT)); // A String for cross-platform reasons.
>          payload.put(PING_CREATION_DATE, pingCreationDateFormat.format(nowCalendar.getTime()));
>          payload.put(TIMEZONE_OFFSET, DateUtil.getTimezoneOffsetInMinutesForGivenDate(nowCalendar));
> -        payload.putArray(EXPERIMENTS, Experiments.getActiveExperiments(context));

Sorry... I must be out of my mind :(

Comment 7

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8907403 [details]
Bug 1392538 - Remove unused onboarding experiments.

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

Comment 8

[Jan Henning [:JanH]]

(In reply to Sebastian Kaspari (:sebastian) from comment #5)
> > - urlbar-show-origin-only
> > - urlbar-show-ev-cert-owner
> 
> They are still used in the code.

Side-tracking question - with the scrollable URL bar and the initial focus on the end of the base domain now implemented, do we want to keep these on standby?

Comment 9

[:Grisha Kruglov]

offline-cache experiment _is_ used: https://dxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#6521

Granted, it's disabled for release users, but hopefully we can work through bugs in Bug 1232867 to enable it at some point.

Please don't remove this experiment from Kinto. It also seems like you haven't fully audited our experiment uses which are outside of java-land, so please do so before committing to any removals.

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Jan Henning [:JanH] from comment #8)
> (In reply to Sebastian Kaspari (:sebastian) from comment #5)
> > > - urlbar-show-origin-only
> > > - urlbar-show-ev-cert-owner
> > 
> > They are still used in the code.
> 
> Side-tracking question - with the scrollable URL bar and the initial focus
> on the end of the base domain now implemented, do we want to keep these on
> standby?

No, we can remove those now - but not only the experiments. Let's remove the code too in a separate bug. :)

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8907403 [details]
Bug 1392538 - Remove unused onboarding experiments.

https://reviewboard.mozilla.org/r/179086/#review199502

Comment 12

[Jan Henning [:JanH]]

(In reply to Sebastian Kaspari (:sebastian) from comment #10)
> (In reply to Jan Henning [:JanH] from comment #8)
> > (In reply to Sebastian Kaspari (:sebastian) from comment #5)
> > > > - urlbar-show-origin-only
> > > > - urlbar-show-ev-cert-owner
> > > 
> > > They are still used in the code.
> > 
> > Side-tracking question - with the scrollable URL bar and the initial focus
> > on the end of the base domain now implemented, do we want to keep these on
> > standby?
> 
> No, we can remove those now - but not only the experiments. Let's remove the
> code too in a separate bug. :)

Bug 1412818.

Comment 13

[Nevin Chen(Not active on Bugzilla)]

ni myself to do this

Comment 14

[Nevin Chen(Not active on Bugzilla)]

already removed in kinto

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7986c821efe
Remove unused onboarding experiments. r=sebastian

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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