sync: Implement fake server reflection blocking

chrome/browser/sync/test/integration/fake_server_invalidation_service.h

Index: chrome/browser/sync/test/integration/fake_server_invalidation_service.h
diff --git a/chrome/browser/sync/test/integration/fake_server_invalidation_service.h b/chrome/browser/sync/test/integration/fake_server_invalidation_service.h
index d164c14d87f60541f93780d0060c5d6ebc6814d2..dd51877302866c63ffbe027f525d76f06125e365 100644
--- a/chrome/browser/sync/test/integration/fake_server_invalidation_service.h
+++ b/chrome/browser/sync/test/integration/fake_server_invalidation_service.h
@@ -50,11 +50,19 @@ class FakeServerInvalidationService : public invalidation::InvalidationService,
       base::Callback<void(const base::DictionaryValue&)> caller) const OVERRIDE;
   virtual IdentityProvider* GetIdentityProvider() OVERRIDE;
 
+  // When true, this flag enables the sending of notifications to all clients,
+  // including the one responsible for the commit.
+  void SetSelfNotificationsEnabled(bool enabled);

[pval...(no longer on Chromium), 2014/05/21 17:37:03]

How about two methods: EnableSelfNotifications() and DisableSelfNotifications()?
I see that the call point maybe lends itself to one method, but having two
methods seems like a cleaner API.

Alternatively, could we just add self_notify as a constructor arg? It doesn't
look like we'll need to change the value over the lifetime of the service.

[rlarocque, 2014/05/21 17:50:36]

Yes, I was mainly concerned about the call site when I wrote it this way.  I
could change it to two methods.  That might also allow me to avoid any decisions
about whether or not to use the NOTIFY_ALL / NOTIFY_OTHERS enums declared in the
p2p invalidator code vs. a bool.  Probably a good idea.

As for the constructor approach, that's the route I took with the P2P
invalidaton.  I think I was partly forced into it because of the way its members
are initialized.

The BrowserContextKeyedServiceFactory's SetTestingFactoryAndUse() takes a
function pointer, not a Closure.  So we can't bind arguments to it.  If we want
to have constructor arguments, we need to define dummy functions to wrap the
alternatives.

You can see how this turned out for the P2P case in lines 145-166 of
sync_test.cc.

I'm leaning towards separate methods.  Let me know if you'd prefer the
constructor approach.

[pval...(no longer on Chromium), 2014/05/21 19:46:58]

separate methods sgtm

+
   // FakeServer::Observer:
-  virtual void OnCommit(syncer::ModelTypeSet committed_model_types) OVERRIDE;
+  virtual void OnCommit(
+      const std::string& committer_id,
+      syncer::ModelTypeSet committed_model_types) OVERRIDE;
 
  private:
   std::string client_id_;
+  bool self_notify_;
+
   syncer::InvalidatorRegistrar invalidator_registrar_;
   FakeProfileOAuth2TokenService token_service_;
   FakeIdentityProvider identity_provider_;