The PasswordsPrivateAPI has code to notify a newly-added observer of the current state of what they're observing, but does so in such a way that it causes all existing listeners to also be notified. See discussion on https://codereview.chromium.org/1833773002/diff/40001/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc#newcode68
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b6770b9db207252850b2cac03bec14a327939fa0 commit b6770b9db207252850b2cac03bec14a327939fa0 Author: hcarmona <hcarmona@chromium.org> Date: Thu Apr 14 20:25:51 2016 Update code so 1:1 relation between delegate and router is more clear. Removed the list of delegates and extraneous code around that. Updated tests to match. BUG= 598826 Review URL: https://codereview.chromium.org/1857513002 Cr-Commit-Position: refs/heads/master@{#387406} [modify] https://crrev.com/b6770b9db207252850b2cac03bec14a327939fa0/chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc [modify] https://crrev.com/b6770b9db207252850b2cac03bec14a327939fa0/chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h [modify] https://crrev.com/b6770b9db207252850b2cac03bec14a327939fa0/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc [modify] https://crrev.com/b6770b9db207252850b2cac03bec14a327939fa0/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h [modify] https://crrev.com/b6770b9db207252850b2cac03bec14a327939fa0/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc [modify] https://crrev.com/b6770b9db207252850b2cac03bec14a327939fa0/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/de3ada6f945ac01d767ed68db95d17d1dec0d5f8 commit de3ada6f945ac01d767ed68db95d17d1dec0d5f8 Author: hcarmona <hcarmona@chromium.org> Date: Thu Apr 21 20:43:01 2016 Add functions to the passwords private api that update the lists. Making the update explicit will allow us to decouple listening and updating of both lists. BUG= 598826 Review URL: https://codereview.chromium.org/1892613006 Cr-Commit-Position: refs/heads/master@{#388889} [modify] https://crrev.com/de3ada6f945ac01d767ed68db95d17d1dec0d5f8/chrome/browser/extensions/api/passwords_private/passwords_private_api.cc [modify] https://crrev.com/de3ada6f945ac01d767ed68db95d17d1dec0d5f8/chrome/browser/extensions/api/passwords_private/passwords_private_api.h [modify] https://crrev.com/de3ada6f945ac01d767ed68db95d17d1dec0d5f8/chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc [modify] https://crrev.com/de3ada6f945ac01d767ed68db95d17d1dec0d5f8/chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h [modify] https://crrev.com/de3ada6f945ac01d767ed68db95d17d1dec0d5f8/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc [modify] https://crrev.com/de3ada6f945ac01d767ed68db95d17d1dec0d5f8/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h [modify] https://crrev.com/de3ada6f945ac01d767ed68db95d17d1dec0d5f8/chrome/common/extensions/api/passwords_private.idl [modify] https://crrev.com/de3ada6f945ac01d767ed68db95d17d1dec0d5f8/chrome/test/data/extensions/api_test/passwords_private/test.js [modify] https://crrev.com/de3ada6f945ac01d767ed68db95d17d1dec0d5f8/extensions/browser/extension_function_histogram_value.h [modify] https://crrev.com/de3ada6f945ac01d767ed68db95d17d1dec0d5f8/third_party/closure_compiler/externs/passwords_private.js [modify] https://crrev.com/de3ada6f945ac01d767ed68db95d17d1dec0d5f8/tools/metrics/histograms/histograms.xml
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4b56a037727e266fc1515dc8d9cd4e94e15f8f63 commit 4b56a037727e266fc1515dc8d9cd4e94e15f8f63 Author: hcarmona <hcarmona@chromium.org> Date: Thu Apr 21 22:32:50 2016 Remove implicit update when adding a listener in passwords private API. BUG= 598826 Review URL: https://codereview.chromium.org/1894773002 Cr-Commit-Position: refs/heads/master@{#388926} [modify] https://crrev.com/4b56a037727e266fc1515dc8d9cd4e94e15f8f63/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc [modify] https://crrev.com/4b56a037727e266fc1515dc8d9cd4e94e15f8f63/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h [modify] https://crrev.com/4b56a037727e266fc1515dc8d9cd4e94e15f8f63/chrome/browser/resources/settings/passwords_and_forms_page/passwords_and_forms_page.js [modify] https://crrev.com/4b56a037727e266fc1515dc8d9cd4e94e15f8f63/chrome/common/extensions/api/passwords_private.idl [modify] https://crrev.com/4b56a037727e266fc1515dc8d9cd4e94e15f8f63/chrome/test/data/extensions/api_test/passwords_private/test.js [modify] https://crrev.com/4b56a037727e266fc1515dc8d9cd4e94e15f8f63/third_party/closure_compiler/externs/passwords_private.js
Is there any more work that we need here for the API? API now: No notification when adding a listener Explicit update takes callback and doesn't trigger notifications to listeners Listeners are notified from changes in observed data
I think it is good now, thanks!
Comment 1 by steve...@chromium.org
, Mar 30 2016Components: UI>Settings
Labels: -OS-Linux OS-All
Owner: hcarmona@chromium.org