Refactor MainPreferences to create Preferences once per activity |
|||||||
Issue descriptionCurrently MainPreferences removes and adds again all preferences each time the activity is resumed, leading to objects being destroyed and re-created unnecessarily. Refactoring it such that Preferences are created only once and then updated as needed is necessary for counting the number of impressions and for recording several metrics regarding the personalized signin promos.
,
Sep 6 2017
,
Sep 7 2017
,
Sep 8 2017
Your change meets the bar and is auto-approved for M62. Please go ahead and merge the CL to branch 3202 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/10c6f24c354593878b2c54f293da3d8807c7b28d commit 10c6f24c354593878b2c54f293da3d8807c7b28d Author: Boris Sazonov <bsazonov@chromium.org> Date: Fri Sep 08 12:02:01 2017 [Android] Change the way preferences are managed in MainPreferenes This CL refactors the way preferences are being updated. Instead of creating new preferences each time the activity is resumed, they are now created once, when the Settings screen is opened by the user, and are removed/added to the PreferenceScreen as needed. The XML file is removed and preferences are now created in the Java code. This change improves flexibility as now it is possible to instantiate one or several preferences without having to reload all of them from the XML file. (cherry picked from commit d587166649f391e57476cbb152fc407336abaa0d) TBR=iuliah@google.com Bug: 761034 Change-Id: Iba2cb91ab5cdff5762ba8c88d0580f8c6c7d8729 Reviewed-on: https://chromium-review.googlesource.com/645887 Reviewed-by: Theresa <twellington@chromium.org> Reviewed-by: Boris Sazonov <bsazonov@chromium.org> Commit-Queue: Iulia Harasim <iuliah@google.com> Cr-Original-Commit-Position: refs/heads/master@{#499968} Reviewed-on: https://chromium-review.googlesource.com/657657 Cr-Commit-Position: refs/branch-heads/3202@{#88} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/10c6f24c354593878b2c54f293da3d8807c7b28d/chrome/android/java/res/xml/main_preferences.xml [modify] https://crrev.com/10c6f24c354593878b2c54f293da3d8807c7b28d/chrome/android/java/src/org/chromium/chrome/browser/preferences/MainPreferences.java
,
Sep 8 2017
Out of curiosity, why did this need to be merged to M62?
,
Sep 8 2017
We count the number of impressions for the "personalized" signin promo in settings and automatically revert back to the smaller, text-based promo after 5 impressions. Conceptually, we want to count an "impression" whenever the user opens the settings screen and sees the promo. However, we were counting an impression in certain situations that caused some odd behavior (e.g. if you clicked on the promo to start the signin flow, then clicked back to go back to the settings screen, you might see the promo change out underneath you to the old version, since we were counting going back as an impression). This fix prevents those weird interactions and makes sure we're not overcounting impressions.
,
Sep 8 2017
That makes sense. Thank you for the explanation!
,
Sep 8 2017
No problem! |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by bugdroid1@chromium.org
, Sep 6 2017