sync: Implement fake server reflection blocking

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_SYNC_TEST_INTEGRATION_FAKE_SERVER_INVALIDATION_SERVICE_H_
 #define CHROME_BROWSER_SYNC_TEST_INTEGRATION_FAKE_SERVER_INVALIDATION_SERVICE_H_
 
 #include <string>
 #include <utility>
 
@@ skipping 32 matching lines @@
   virtual void UnregisterInvalidationHandler(
       syncer::InvalidationHandler* handler) OVERRIDE;
 
   virtual syncer::InvalidatorState GetInvalidatorState() const OVERRIDE;
   virtual std::string GetInvalidatorClientId() const OVERRIDE;
   virtual invalidation::InvalidationLogger* GetInvalidationLogger() OVERRIDE;
   virtual void RequestDetailedStatus(
       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_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeServerInvalidationService);
 };
 
 }  // namespace fake_server
 
 #endif  // CHROME_BROWSER_SYNC_TEST_INTEGRATION_FAKE_SERVER_INVALIDATION_SERVICE_H_