This common class should also handle the case when an OAuth token request fails (due to network_changed or any other problem). It should automatically send a new request if the failure is not permanent.

Yes... just keep in mind existing retry mechanisms (there are some on the network level, and also when getting tokens, at least on Android).

Most of the magic actually happens while obtaining the OAuth access token. At least as a first step, I'll build an "AccessTokenFetcher" that handles all of the special stuff. Then we can see about building an AuthenticatedURLFetcher on top of that.

There's a partial implementation of this in SuggestionsService::AccessTokenFetcher, the plan is to extend and generalize that, and find a home for it somewhere with the other OAuth/signin code, maybe components/signin/core/browser/.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c8c6cfe37c424387e26278371125cccbd16d5f3

commit 4c8c6cfe37c424387e26278371125cccbd16d5f3
Author: treib <treib@chromium.org>
Date: Fri Jan 27 16:12:43 2017

Signin/OAuth: Create an AccessTokenFetcher helper class

Reliably obtaining an OAuth2 access token from OAuth2TokenService is
surprisingly complicated, since there are many special cases to handle
(e.g. auth currently in progress, or auth completed but refresh token
not loaded yet, ...). Various classes implement some subset of these
edge cases.

This CL introduces a new helper class that encapsulates all this, and
adds a first usage as an example (in SuggestionsService).

Partial list of additional classes that can be made simpler and more
robust by using the new helper:
- NTPSnippetsFetcher
- FamilyInfoFetcher
- SafeSearchURLReporter
- PermissionRequestCreatorApiary

BUG=609084

Review-Url: https://codereview.chromium.org/2582573002
Cr-Commit-Position: refs/heads/master@{#446678}

[modify] https://crrev.com/4c8c6cfe37c424387e26278371125cccbd16d5f3/components/signin/core/browser/BUILD.gn
[add] https://crrev.com/4c8c6cfe37c424387e26278371125cccbd16d5f3/components/signin/core/browser/access_token_fetcher.cc
[add] https://crrev.com/4c8c6cfe37c424387e26278371125cccbd16d5f3/components/signin/core/browser/access_token_fetcher.h
[add] https://crrev.com/4c8c6cfe37c424387e26278371125cccbd16d5f3/components/signin/core/browser/access_token_fetcher_unittest.cc
[modify] https://crrev.com/4c8c6cfe37c424387e26278371125cccbd16d5f3/components/signin/core/browser/fake_signin_manager.h
[modify] https://crrev.com/4c8c6cfe37c424387e26278371125cccbd16d5f3/components/suggestions/suggestions_service_impl.cc
[modify] https://crrev.com/4c8c6cfe37c424387e26278371125cccbd16d5f3/components/suggestions/suggestions_service_impl.h
[modify] https://crrev.com/4c8c6cfe37c424387e26278371125cccbd16d5f3/components/suggestions/suggestions_service_impl_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c31ec95e72b921cc625e8a48e7064721fc85de09

commit c31ec95e72b921cc625e8a48e7064721fc85de09
Author: treib <treib@chromium.org>
Date: Mon Mar 27 09:34:04 2017

RemoteSuggestionsFetcher: Use common AccessTokenFetcher helper class

This simplifies the RemoteSuggestionsFetcher quite a bit.

BUG=609084

Review-Url: https://codereview.chromium.org/2771713003
Cr-Commit-Position: refs/heads/master@{#459731}

[modify] https://crrev.com/c31ec95e72b921cc625e8a48e7064721fc85de09/components/ntp_snippets/remote/remote_suggestions_fetcher.cc
[modify] https://crrev.com/c31ec95e72b921cc625e8a48e7064721fc85de09/components/ntp_snippets/remote/remote_suggestions_fetcher.h
[modify] https://crrev.com/c31ec95e72b921cc625e8a48e7064721fc85de09/components/ntp_snippets/remote/remote_suggestions_fetcher_unittest.cc

We should probably introduce an "AccessTokenFetcherCreator" that can be passed to users. That way, it'll be easy to inject fake AccessTokenFetchers in tests, so tests don't have to set up the whole set of dependencies (SigninManager, SigninClient, OAuth2TokenService, PrefService, ...), and are also just cleaner.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885

commit f1e4b577c1e9a6b81f2439fa44c5313e79ae1885
Author: Daniel Kenji Toyama <kenjitoyama@google.com>
Date: Thu Aug 03 16:31:11 2017

Add ContextualSuggestionsService to Omnibox.

This CL was cloned from https://codereview.chromium.org/2965173002/.

Create a new class `ContextualSuggestionsService` to fetch experimental
contextual suggestions for the omnibox.

Bug: 692471,609084,  750985 
Change-Id: Ib54be91b1c0e2cb1fcdaf9a7a43b71984e5c358c
Reviewed-on: https://chromium-review.googlesource.com/576284
Commit-Queue: Owen Min <zmin@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491762}
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/chrome/browser/BUILD.gn
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/chrome/browser/autocomplete/chrome_autocomplete_provider_client.h
[add] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/chrome/browser/autocomplete/contextual_suggestions_service_factory.cc
[add] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/chrome/browser/autocomplete/contextual_suggestions_service_factory.h
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/BUILD.gn
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/DEPS
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/autocomplete_provider_client.h
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/base_search_provider_unittest.cc
[add] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/contextual_suggestions_service.cc
[add] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/contextual_suggestions_service.h
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/keyword_provider_unittest.cc
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/mock_autocomplete_provider_client.cc
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/mock_autocomplete_provider_client.h
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/omnibox_field_trial.cc
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/omnibox_field_trial.h
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/shortcuts_provider_unittest.cc
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/zero_suggest_provider.cc
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/components/omnibox/browser/zero_suggest_provider.h
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc
[modify] https://crrev.com/f1e4b577c1e9a6b81f2439fa44c5313e79ae1885/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.h

I think we can consider this done: The new Identity Service at /services/identity/ implements the functionality from AccessTokenFetcher, so let's not invest more into that.
We *could* still build an AuthenticatedURLFetcher on top of the Identity Service, but I'm not convinced that's worth it.

Urrr… at the very least we should migrate all the existing services that we've built on top of OAuth2TokenService to IdentityManager. I also don't think the IdentityManager implements all the edge cases that we've handled (and duplicated!) in all of those services, like retrying once if we hit the race condition where the access token expires right before being sent.

Given that we now have more code doing this exact flow(Feed, EoC, Zine, maybe Memex soon?), I think finishing this is more of a priority.

Agreed, thanks Patrick.