New issue
Advanced search Search tips

Issue 674140 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Update prefs::mojom::Preference to be created with two-way-binding

Project Member Reported by jonr...@chromium.org, Dec 14 2016

Issue description

Since prefs::mojom::PreferencesManager and prefs::mojom::PreferencesObserver form a 1:1 pairing, with bi-directional communication, we wish to enforce that binding has occurred before other api calls are made.

Currently this is not possible with creating a PreferencesFactory which would accept PreferencesObserver and return a created PreferencesManager.

However requiring a third mojom interface for all bi-directional communication is too much of a requirement for each client. Especially as each future process accessing preferences would need to have this overhead.

It is desired that service_manager::Connector provide an api to enforce the bi-directional binding at connection time. This will be brought up on chromium-mojo.

Once that is available we need to update the preferences code to use this new method. At that time we can stop forcibly checking the binding state in each method.

More details in code review: https://codereview.chromium.org/2474653003/
 
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/016032b4d15cd6583a5249c10d59c4de854a2b03

commit 016032b4d15cd6583a5249c10d59c4de854a2b03
Author: jonross <jonross@chromium.org>
Date: Thu Jan 19 20:20:03 2017

Switch Preferences to use Factory

Update the Preferences mojom to include a PreferencesFactory. This factory will
be responsible for creating a PreferencesManager, and enforcing the binding of
the PreferencesObserver at creation time. Vs the current method which does not
guarantee this.

Chrome will no longer expose PreferencesManager as a service directly. Ash has
been updated to connect to the factory. PrefObserverStore has been updated to
receive the factory and to bind to the manager this way.

TEST=manual testing, preferences_unittests
BUG= 674140 

Review-Url: https://codereview.chromium.org/2635093002
Cr-Commit-Position: refs/heads/master@{#444832}

[modify] https://crrev.com/016032b4d15cd6583a5249c10d59c4de854a2b03/ash/common/wm_shell.cc
[modify] https://crrev.com/016032b4d15cd6583a5249c10d59c4de854a2b03/chrome/browser/prefs/preferences_connection_manager.cc
[modify] https://crrev.com/016032b4d15cd6583a5249c10d59c4de854a2b03/chrome/browser/prefs/preferences_connection_manager.h
[modify] https://crrev.com/016032b4d15cd6583a5249c10d59c4de854a2b03/chrome/browser/prefs/preferences_manager.cc
[modify] https://crrev.com/016032b4d15cd6583a5249c10d59c4de854a2b03/chrome/browser/prefs/preferences_manager.h
[modify] https://crrev.com/016032b4d15cd6583a5249c10d59c4de854a2b03/chrome/browser/prefs/preferences_manager_unittest.cc
[modify] https://crrev.com/016032b4d15cd6583a5249c10d59c4de854a2b03/chrome/browser/prefs/preferences_manifest.json
[modify] https://crrev.com/016032b4d15cd6583a5249c10d59c4de854a2b03/services/preferences/public/cpp/pref_observer_store.cc
[modify] https://crrev.com/016032b4d15cd6583a5249c10d59c4de854a2b03/services/preferences/public/cpp/pref_observer_store.h
[modify] https://crrev.com/016032b4d15cd6583a5249c10d59c4de854a2b03/services/preferences/public/cpp/tests/pref_observer_store_unittest.cc
[modify] https://crrev.com/016032b4d15cd6583a5249c10d59c4de854a2b03/services/preferences/public/interfaces/preferences.mojom

Status: Fixed (was: Started)
Components: -MUS Internals>Services>WindowService

Sign in to add a comment