Remove singleton/observer element pattern from MD Settings |
||
Issue descriptionAfter data binding with IronMeta was lost in 0.8, we needed a way to bind the same data in multiple places, especially data relating to a complicated model: lots of pages might want to show or change this data, but only one element should be responsible for complex logic for responding to model changes. Biggest example was Preferences. Two options were: 1. Have one element at the top of the tree handling the model and providing the data. Normal data binding propagates changes up and down the tree -- the downside is, you have to declare properties in every node in between the elements you want to share data. 2. Replace IronMeta with a Singleton/Observer pattern. Create the singleton element at startup; it owns the data. Any element that wants to bind to that data adds an observer element to its local DOM and binds to that -- no dependencies on ancestors or descendants. I implemented option 1 for Preferences and Languages, both very complicated models that can't just be a Behavior shared by a bunch of elements. In hindsight, the implementation is too obscure, fragile, and (possibly) less efficient than plain old data binding. There's extra code for managing the Observer relationships, and there are singleton elements created at startup that could be created later. Furthermore, it would be a burden for anyone other than myself to get up to speed on and maintain in the future. I want to remove settings-languages-singleton and settings-prefs-singleton. Languages data binding is easy because, unsurprisingly, everything that uses the languages model is a descendant of the Languages page. Prefs is easy because we mostly already do it the normal way -- the Languages singleton is the only other user of the Prefs singleton, which is a pretty good sign that the pattern wasn't useful. Original design doc for context: https://docs.google.com/document/d/1yTA34r7KyFXMlXxkSsR2vSNzh6V1S8MuxOrkh5smKaI The nail in the coffin is that we never came up with other use cases: other complex models live in the browser process, and are handled from JS with simple FooBrowserProxy classes.
,
Aug 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/190a0525f1b960e8f241819421cc94c0f0e0f38a commit 190a0525f1b960e8f241819421cc94c0f0e0f38a Author: michaelpg <michaelpg@chromium.org> Date: Fri Aug 19 06:03:58 2016 MD Settings: reduce complexity and overhead of prefs singleton Remove <settings-prefs-singleton> and put all the logic in <settings-prefs>. No more custom listeners or fragile test structures. This depends on removing <settings-languages-singleton>. BUG= 638802 R=dbeam@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2254113002 Cr-Commit-Position: refs/heads/master@{#413062} [modify] https://crrev.com/190a0525f1b960e8f241819421cc94c0f0e0f38a/chrome/browser/resources/settings/prefs/prefs.js [modify] https://crrev.com/190a0525f1b960e8f241819421cc94c0f0e0f38a/chrome/test/data/webui/settings/fake_settings_private.js [modify] https://crrev.com/190a0525f1b960e8f241819421cc94c0f0e0f38a/chrome/test/data/webui/settings/languages_tests.js [modify] https://crrev.com/190a0525f1b960e8f241819421cc94c0f0e0f38a/chrome/test/data/webui/settings/passwords_and_forms_browsertest.js [modify] https://crrev.com/190a0525f1b960e8f241819421cc94c0f0e0f38a/chrome/test/data/webui/settings/prefs_tests.js
,
Aug 19 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Aug 19 2016