commit 9f4466132898e523e308589b6f55a407fbe25372
author Colin Blundell <blundell@chromium.org> Wed Mar 06 18:18:31 2019
committer Commit Bot <commit-bot@chromium.org> Wed Mar 06 18:18:31 2019
tree fcdd47e5d070f944c6f82b604315fbbb2724ab31
parent 3f6bee805d5de15212cbea1360ebfda0bd3d984e

[Identity] Rework how to enable OnExtendedAccountInfoRemoved() for tests

This CL takes a step toward the elimination of
FakeAccountFetcherService by eliminating the need for it to override
AccountFetcherService network fetches with a do-nothing implementation.

In a testing context, AccountFetcherService does not do network fetches
by default. One somewhat-surprising consequence of this policy is that
refresh token revocation does not result in AccountTrackerService firing
the corresponding expected notification that the account was removed.
Some tests *require* that flow to work as expected in order to be able
to test their clients' production flow. In order to facilitate such
tests, AccountFetcherService has a testing method that enables network
fetches. This method is typically used by tests in conjunction with a
FakeAccountFetcherService instance, which subclasses
AccountFetcherService and overrides the methods that do the actual
network fetches to do nothing.

As detailed in crbug.com/935998, we are looking to eliminate
FakeAccountFetcherService. This CL changes the way that the above flow
works: instead of these tests enabling network fetches and then using
FakeAccountFetcherService to short-circuit out of the actual network
fetches, AccountFetcherService now provides the facility to simply
enable processing of account removal for tests. That is in fact all
that these tests need. We can then eliminate the overrides of the
above-mentioned methods in FakeAccountFetcherService, moving one
step closer to FakeAccountFetcherService not being necessary.

The one necessary usage of
AccountFetcherService::EnableNetworkFetchesForTests() is in the
AccountTrackerService unittest. Here this usage is deliberate as the
test in fact tests the actual interaction with the network, supplying
a TestURLLoaderFactory to the production AccountFetcherService instance
that it uses. This CL adds comments to the two testing methods to
clarify when to use one vs. the other.

TBR=sdefresne@chromium.org

Bug: 935998
Change-Id: I5106a85cf20874b48aa38bc3935df9a08018ea72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1489241
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638195}