Remove unused Kinto settings

RESOLVED FIXED in Firefox 58

Status

()

enhancement
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: cnevinchen, Assigned: cnevinchen)

Tracking

unspecified
Firefox 58
Points:
---

Firefox Tracking Flags

(firefox58 fixed)

Details

(Whiteboard: [FNC][SPT58.4][INT])

Attachments

(1 attachment)

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.
Assignee: nobody → cnevinchen
Priority: -- → P2
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
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
Flags: needinfo?(s.kaspari)
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?
Attachment #8907403 - Flags: review?(s.kaspari) → review-
> - 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).
Flags: needinfo?(s.kaspari)
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 :(
(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?
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.
(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 on attachment 8907403 [details]
Bug 1392538 - Remove unused onboarding experiments.

https://reviewboard.mozilla.org/r/179086/#review199502
Attachment #8907403 - Flags: review?(s.kaspari) → review+
(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.
ni myself to do this
Flags: needinfo?(cnevinchen)
already removed in kinto
Flags: needinfo?(cnevinchen)
Pushed by nechen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c7986c821efe
Remove unused onboarding experiments. r=sebastian
https://hg.mozilla.org/mozilla-central/rev/c7986c821efe
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Whiteboard: [FNC][SPT58.4][INT]
You need to log in before you can comment on or make changes to this bug.