[Extensions] Convert APIs to use movable types [8]

chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc

Index: chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc
diff --git a/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc b/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc
index dfd7abbb0ce09b8cd0c2fc0fa3175170da75517d..3de1e72e9ea72d028e1e7f97e2d063cc47986708 100644
--- a/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc
+++ b/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc
@@ -7,6 +7,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/strings/utf_string_conversions.h"
@@ -22,7 +23,8 @@ PasswordsPrivateEventRouter::PasswordsPrivateEventRouter(
     content::BrowserContext* context)
     : context_(context),
       event_router_(nullptr),
-      listening_(false) {
+      listening_(false),
+      ignore_updates_(false) {
   // Register with the event router so we know when renderers are listening to
   // our events. We first check and see if there *is* an event router, because
   // some unit tests try to create all context services, but don't initialize
@@ -59,7 +61,12 @@ void PasswordsPrivateEventRouter::OnListenerAdded(
     const EventListenerInfo& details) {
   // Start listening to change events and propagate the original lists to
   // listeners.
-  StartOrStopListeningForChanges();
+  {
+    // Some delegates will immediately update observers. Since we dispatch the
+    // events again below, we want to ignore this here.
+    base::AutoReset<bool> ignore_updates(&ignore_updates_, true);
+    StartOrStopListeningForChanges();

[Devlin, 2016/03/25 13:39:48]

Flow:
- Starts listening
- Adds self as observer
- Delegate immediately sends "OnFooChanged" notifications, which used to cause
this to then SendFooListToListeners
- Continues to line 70/71, where everything is re-sent.
This results in two updates being sent immediately for extensions registering
for listeners, which made the API test fail.  The reason this didn't fail before
is because the ObserverListThreadSafe made these updates asynchronous.

Note that even with this change, we still have the side effect of notifying
*all* observers every time a new observer is added, even if nothing's changed. 
The fix would be to either dispatch only to the new listener or to provide a
GetFoo() api method - but that's a separate issue, and since this is a private
API, not really a big deal.

[stevenjb, 2016/03/29 00:52:37]

I'd actually rather fix this separately. I agree that the current behavior is a
side effect of inappropriately using ObserverListThreadSafe is strange and bad,
but I'd rather fix this more correctly separately.

Could you file a bug for this and assign it to me? I'll fix or reassign
appropriately. Thanks!

[Devlin, 2016/03/29 14:19:26]

Unfortunately, the API test fails without this fix (since the listener gets
notified too many times).  I could proceed with this band-aid for now or change
the API test and add a TODO, or I can wait for the proper fix before landing
this one (I have a slight preference for one of the former so that this isn't
blocked).  Preferences?

+  }
   SendSavedPasswordListToListeners();
   SendPasswordExceptionListToListeners();
 }
@@ -71,15 +78,14 @@ void PasswordsPrivateEventRouter::OnListenerRemoved(
 }
 
 void PasswordsPrivateEventRouter::OnSavedPasswordsListChanged(
-    const std::vector<linked_ptr<
-        api::passwords_private::PasswordUiEntry>>& entries) {
+    const std::vector<api::passwords_private::PasswordUiEntry>& entries) {
   cached_saved_password_parameters_ =
       api::passwords_private::OnSavedPasswordsListChanged::Create(entries);
   SendSavedPasswordListToListeners();
 }
 
 void PasswordsPrivateEventRouter::SendSavedPasswordListToListeners() {
-  if (!cached_saved_password_parameters_.get())
+  if (!cached_saved_password_parameters_.get() || ignore_updates_)
     // If there is nothing to send, return early.
     return;
 
@@ -99,7 +105,7 @@ void PasswordsPrivateEventRouter::OnPasswordExceptionsListChanged(
 }
 
 void PasswordsPrivateEventRouter::SendPasswordExceptionListToListeners() {
-  if (!cached_password_exception_parameters_.get())
+  if (!cached_password_exception_parameters_.get() || ignore_updates_)
     // If there is nothing to send, return early.
     return;