Convert ntp_snippets to use Identity Service client library |
||
Issue descriptionThere is a bunch of usage, but it's all straightforward to convert to the Identity Service client lib: SubscriptionJsonRequest: Include appears dead SubscriptionManagerImpl observes SigninManager in order to perform updates on signin status changing. It makes access token requests via AccessTokenFetcher and checks whether the user is authenticated to determine whether to make access token requests. ContentSuggestionsService observes SigninManager to perform updates on signin status changing. ContextualJsonRequest: Include appears stale JsonRequest: Includes appear stale RemoteSuggestionsFetcherImpl makes access token requests via AccessTokenFetcher and checks whether the user is authenticated to determine whether to make access token requests. RemoteSuggestionsStatusServiceImpl checks whether the user is signed in as part of determining whether signin state has actually changed in response to an OnStateChanged() call.
,
Jan 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c49c973caa5b2b4b467858f3faf194176ee69590 commit c49c973caa5b2b4b467858f3faf194176ee69590 Author: Colin Blundell <blundell@chromium.org> Date: Mon Jan 15 16:14:58 2018 Remove AuthInProgress() check from RemoteSuggestionsFetcherImpl RemoteSuggestionsFetcherImpl currently checks whether the user is signed in or signin is in progress in order to determine whether to request an access token or simply fetch suggestions without an access token. As we are preparing to convert this class to use the Identity Service client library, the usage of SigninManager::AuthInProgress() here is problematic: we have not yet decided how or whether we are going to expose this concept via the Identity Service client library. In this case, it is unproblematic to simply remove the usage altogether: the NTP Snippets feature already handles the case where snippets are fetched for a signed-out user and then the user signs in by initiating a refetch. Removing the AuthInProgress() case just means that the corner case of a fetch being initiated while signin is in progress will fall into the larger case of "initiate refetch when user signs in." Bug: 798413 Change-Id: I93f3e271db35dcc9a72e2d70d12603fbac98e8d3 Reviewed-on: https://chromium-review.googlesource.com/864151 Reviewed-by: Jan Krcal <jkrcal@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#529291} [modify] https://crrev.com/c49c973caa5b2b4b467858f3faf194176ee69590/components/ntp_snippets/remote/remote_suggestions_fetcher_impl.cc
,
Jan 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ce266a696e1c79889e24e078f69262af6af0d99e commit ce266a696e1c79889e24e078f69262af6af0d99e Author: Colin Blundell <blundell@chromium.org> Date: Tue Jan 16 11:07:16 2018 Change how RemoteSuggestionsStatusServiceImpl gets signin status This CL implements a TODO on RemoteSuggestionsStatusServiceImpl to have it track the user's signin status via state changes passed from OnSigninStatusChanged() rather than directly asking SigninManager. The motivation is preparatory cleanup for changing NTP Snippets to use the Identity Service client library. Bug: 798413 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: Iba39bfa510fcf35c1a4b1fc2f0eaccd2c9c83a71 Reviewed-on: https://chromium-review.googlesource.com/862142 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Gauthier Ambard <gambard@chromium.org> Reviewed-by: vitaliii <vitaliii@chromium.org> Reviewed-by: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#529400} [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/components/ntp_snippets/content_suggestions_provider.h [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/components/ntp_snippets/content_suggestions_service.cc [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/components/ntp_snippets/content_suggestions_service.h [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/components/ntp_snippets/remote/remote_suggestions_provider_impl.h [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/components/ntp_snippets/remote/remote_suggestions_scheduler_impl_unittest.cc [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/components/ntp_snippets/remote/remote_suggestions_status_service.h [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/components/ntp_snippets/remote/remote_suggestions_status_service_impl.cc [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/components/ntp_snippets/remote/remote_suggestions_status_service_impl.h [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/components/ntp_snippets/remote/remote_suggestions_status_service_impl_unittest.cc [modify] https://crrev.com/ce266a696e1c79889e24e078f69262af6af0d99e/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory_util.cc
,
Jan 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/683639a46be8c1a9c4ee278fe518bccd1a3ef0c5 commit 683639a46be8c1a9c4ee278fe518bccd1a3ef0c5 Author: Colin Blundell <blundell@chromium.org> Date: Thu Jan 18 14:13:36 2018 [NTP Snippets] Have RemoteSuggestionsFetcherImpl use IdentityManager This CL converts RemoteSuggestionsFetcherImpl from using //components/signin to using the Identity Service client library. The conversion is straightforward: - Get info of primary account via IdentityManager rather than SigninManager. - Create PrimaryAccountAccessTokenFetcher via IdentityManager rather than directly. I tested the relevant behavior manually by going to the NTP for a signed-in and syncing test user on Android and confirming that their content suggestions showed up after this change identically to before this change. A followup change will convert the test to use IdentityTestEnvironment. TBR=gambard@chromium.org Bug: 798413 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: If639f55969f70edbaff758598f95e0d74406b492 Reviewed-on: https://chromium-review.googlesource.com/870313 Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Gauthier Ambard <gambard@chromium.org> Reviewed-by: Jan Krcal <jkrcal@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#530135} [modify] https://crrev.com/683639a46be8c1a9c4ee278fe518bccd1a3ef0c5/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc [modify] https://crrev.com/683639a46be8c1a9c4ee278fe518bccd1a3ef0c5/components/ntp_snippets/BUILD.gn [modify] https://crrev.com/683639a46be8c1a9c4ee278fe518bccd1a3ef0c5/components/ntp_snippets/remote/remote_suggestions_fetcher_impl.cc [modify] https://crrev.com/683639a46be8c1a9c4ee278fe518bccd1a3ef0c5/components/ntp_snippets/remote/remote_suggestions_fetcher_impl.h [modify] https://crrev.com/683639a46be8c1a9c4ee278fe518bccd1a3ef0c5/components/ntp_snippets/remote/remote_suggestions_fetcher_impl_unittest.cc [modify] https://crrev.com/683639a46be8c1a9c4ee278fe518bccd1a3ef0c5/ios/chrome/browser/ntp_snippets/BUILD.gn [modify] https://crrev.com/683639a46be8c1a9c4ee278fe518bccd1a3ef0c5/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc [modify] https://crrev.com/683639a46be8c1a9c4ee278fe518bccd1a3ef0c5/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory_util.cc
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c21c1fe3ccf26a6411b9c0fdfe658297af81c693 commit c21c1fe3ccf26a6411b9c0fdfe658297af81c693 Author: Colin Blundell <blundell@chromium.org> Date: Fri Jan 19 09:32:39 2018 [NTP Snippets] Have ContextualSuggestionsFetcherImpl use IdentityManager This CL converts ContextualSuggestionsFetcherImpl from using //components/signin to using the Identity Service client library. The conversion is straightforward: - Get info of primary account via IdentityManager rather than SigninManager. - Create PrimaryAccountAccessTokenFetcher via IdentityManager rather than directly. I tested the relevant behavior manually by going to the NTP for a signed-in and syncing test user on Android and confirming that their content suggestions showed up after this change identically to before this change. A followup change will convert the test to use IdentityTestEnvironment. Bug: 798413 Change-Id: I8da6bc3a812960078c0e22a1239bb00142f1d25e Reviewed-on: https://chromium-review.googlesource.com/870831 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#530461} [modify] https://crrev.com/c21c1fe3ccf26a6411b9c0fdfe658297af81c693/chrome/browser/ntp_snippets/contextual_content_suggestions_service_factory.cc [modify] https://crrev.com/c21c1fe3ccf26a6411b9c0fdfe658297af81c693/components/ntp_snippets/contextual/contextual_suggestions_fetcher_impl.cc [modify] https://crrev.com/c21c1fe3ccf26a6411b9c0fdfe658297af81c693/components/ntp_snippets/contextual/contextual_suggestions_fetcher_impl.h [modify] https://crrev.com/c21c1fe3ccf26a6411b9c0fdfe658297af81c693/components/ntp_snippets/contextual/contextual_suggestions_fetcher_impl_unittest.cc
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fce90bed4910ce81836dca45bc923bda6220d278 commit fce90bed4910ce81836dca45bc923bda6220d278 Author: Colin Blundell <blundell@chromium.org> Date: Fri Jan 19 10:31:40 2018 [NTP Snippets] RemoteSuggestionsFetcherImpl test uses IdentityTestEnv This change is a followup to https://chromium-review.googlesource.com/c/chromium/src/+/870313. It changes the RemoteSuggestionsFetcherImpl unittest to use IdentityTestEnvironment now that the production code is using IdentityManager. This CL also adds an API to IdentityTestEnvironment that is needed for the conversion. Bug: 798413 Change-Id: Ie2ec495c32d86130ee93bdb56b92de9cf93f9500 Reviewed-on: https://chromium-review.googlesource.com/870594 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Jan Krcal <jkrcal@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Cr-Commit-Position: refs/heads/master@{#530475} [modify] https://crrev.com/fce90bed4910ce81836dca45bc923bda6220d278/components/ntp_snippets/BUILD.gn [modify] https://crrev.com/fce90bed4910ce81836dca45bc923bda6220d278/components/ntp_snippets/remote/remote_suggestions_fetcher_impl_unittest.cc [modify] https://crrev.com/fce90bed4910ce81836dca45bc923bda6220d278/services/identity/public/cpp/identity_test_environment.cc [modify] https://crrev.com/fce90bed4910ce81836dca45bc923bda6220d278/services/identity/public/cpp/identity_test_environment.h
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2f0cc42467d90911d5cb0963d8d4ad063765064b commit 2f0cc42467d90911d5cb0963d8d4ad063765064b Author: Colin Blundell <blundell@chromium.org> Date: Fri Jan 19 13:03:03 2018 [NTP Snippets] ContextualSuggestionsFetcherImpl test uses IdentityTestEnv This change is a followup to https://chromium-review.googlesource.com/c/chromium/src/+/870831. It changes the ContextualSuggestionsFetcherImpl unittest to use IdentityTestEnvironment now that the production code is using IdentityManager. Bug: 798413 Change-Id: I6b9af0df058d4d1644dd245207f81ea7d9413f75 Reviewed-on: https://chromium-review.googlesource.com/870930 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#530495} [modify] https://crrev.com/2f0cc42467d90911d5cb0963d8d4ad063765064b/components/ntp_snippets/contextual/contextual_suggestions_fetcher_impl_unittest.cc
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/83abd4de012fe493c7b04e9e832170e1fbac8ef2 commit 83abd4de012fe493c7b04e9e832170e1fbac8ef2 Author: Colin Blundell <blundell@chromium.org> Date: Fri Jan 19 13:48:16 2018 [NTP Snippets] Clean up SubscriptionManagerImpl signin observer Simple opportunistic code cleanup before refactoring this class to use IdentityManager. Bug: 798413 Change-Id: I0afc5bc00c706a28083249fcd3b78580ce706b98 Reviewed-on: https://chromium-review.googlesource.com/875272 Reviewed-by: vitaliii <vitaliii@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#530501} [modify] https://crrev.com/83abd4de012fe493c7b04e9e832170e1fbac8ef2/components/ntp_snippets/breaking_news/subscription_manager_impl.cc [modify] https://crrev.com/83abd4de012fe493c7b04e9e832170e1fbac8ef2/components/ntp_snippets/breaking_news/subscription_manager_impl.h
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1d8b1158b1d3a12858bfaa9c49064522f4d3df02 commit 1d8b1158b1d3a12858bfaa9c49064522f4d3df02 Author: Colin Blundell <blundell@chromium.org> Date: Fri Jan 19 15:15:50 2018 [NTP Snippets] Clean up SubscriptionManagerImpl creation in test Will facilitate moving this class (and test) to use IdentityManager in forthcoming work. Bug: 798413 Change-Id: Icd9432418a8becae0a034e7325f40bd953040a5a Reviewed-on: https://chromium-review.googlesource.com/874695 Reviewed-by: vitaliii <vitaliii@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#530512} [modify] https://crrev.com/1d8b1158b1d3a12858bfaa9c49064522f4d3df02/components/ntp_snippets/breaking_news/subscription_manager_impl_unittest.cc
,
Jan 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1a63c756df441071386d8bbf50f2aae188ffc25b commit 1a63c756df441071386d8bbf50f2aae188ffc25b Author: Colin Blundell <blundell@chromium.org> Date: Tue Jan 30 15:45:53 2018 [NTP Snippets] Reland RemoteSuggestionsFetcher usage of IdentityManager This CL reverts d8bd981fe9, in effect relanding the changes to have RemoteSuggestionsFetcherImpl and its unittest use IdentityManager. In the interim I have landed a fix to the bug that necessitated reversion of those CLs, i.e. the raciness between IdentityManager getting updated on signin/signout and other SigninManager::Observers getting updated on those same events (fix is here: https://chromium-review.googlesource.com/c/chromium/src/+/883347). To test this change, do the following: - Sign in to Chrome on Android (in a chrome_apk build if building locally) - Observe that there are personalized suggestions on the NTP - Sign out - Observe that there are different, generic suggestions on the NTP You can also more generally that the suggestions get updated on various transitions between a signed-out state and a signed-in state. TBR=gambard@chromium.org Bug: 798413 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I37d182718ef0b53a85be1a083f203aa0acdb3d8d Reviewed-on: https://chromium-review.googlesource.com/888617 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#532888} [modify] https://crrev.com/1a63c756df441071386d8bbf50f2aae188ffc25b/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc [modify] https://crrev.com/1a63c756df441071386d8bbf50f2aae188ffc25b/components/ntp_snippets/remote/remote_suggestions_fetcher_impl.cc [modify] https://crrev.com/1a63c756df441071386d8bbf50f2aae188ffc25b/components/ntp_snippets/remote/remote_suggestions_fetcher_impl.h [modify] https://crrev.com/1a63c756df441071386d8bbf50f2aae188ffc25b/components/ntp_snippets/remote/remote_suggestions_fetcher_impl_unittest.cc [modify] https://crrev.com/1a63c756df441071386d8bbf50f2aae188ffc25b/ios/chrome/browser/ntp_snippets/BUILD.gn [modify] https://crrev.com/1a63c756df441071386d8bbf50f2aae188ffc25b/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc [modify] https://crrev.com/1a63c756df441071386d8bbf50f2aae188ffc25b/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory_util.cc
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/59c1b85b3de77ef2804c2f8755714da792657cd5 commit 59c1b85b3de77ef2804c2f8755714da792657cd5 Author: Colin Blundell <blundell@chromium.org> Date: Tue Feb 06 19:31:38 2018 [NTP Snippets] Have SubscriptionManager use IdentityManager This CL converts SubscriptionManager from using //components/signin to using the Identity Service client library. The conversion is straightforward: - Observer IdentityManager rather than SigninManager. - Get info of primary account via IdentityManager rather than SigninManager. - Create PrimaryAccountAccessTokenFetcher via IdentityManager rather than directly. I tested the relevant behavior manually as follows, based on instructions from mamir@chromium.org: 1. Enabled Breaking News Push by turning on "enable-breaking-newspush on chrome://flags: https://cs.chromium.org/chromium/src/chrome/browser/about_flags.cc?type=cs&q=kEnableBreakingNewsPushName&sq=package:chromium&l=2497 2. Checked the about:histograms for the feature's metrics by doing "find in page" for "subscription". Saw no histogram entries after visiting the NTP when signed out but *did* see histogram entries after visiting the NTP when signed in. Metrics listed here: https://cs.chromium.org/chromium/src/components/ntp_snippets/breaking_news/breaking_news_metrics.cc?type=cs&l=17 A followup change will convert the test to use IdentityTestEnvironment. Bug: 798413 Change-Id: I9193d25c2793cf52fd825186cd5d875b352eb8bb Reviewed-on: https://chromium-review.googlesource.com/888520 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#534750} [modify] https://crrev.com/59c1b85b3de77ef2804c2f8755714da792657cd5/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc [modify] https://crrev.com/59c1b85b3de77ef2804c2f8755714da792657cd5/components/ntp_snippets/breaking_news/subscription_manager_impl.cc [modify] https://crrev.com/59c1b85b3de77ef2804c2f8755714da792657cd5/components/ntp_snippets/breaking_news/subscription_manager_impl.h [modify] https://crrev.com/59c1b85b3de77ef2804c2f8755714da792657cd5/components/ntp_snippets/breaking_news/subscription_manager_impl_unittest.cc
,
Feb 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1022ccafe86ac05153110d69c2e3e214bdbc9703 commit 1022ccafe86ac05153110d69c2e3e214bdbc9703 Author: Colin Blundell <blundell@chromium.org> Date: Wed Feb 07 07:01:00 2018 Convert SubscriptionManager unittests to use IdentityTestEnvironment Followup to https://chromium-review.googlesource.com/c/chromium/src/+/888520, which changes the production code. Bug: 798413 Change-Id: I7e82abffd68845a2145f8560858ddfabc0059b7c Reviewed-on: https://chromium-review.googlesource.com/890743 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#534939} [modify] https://crrev.com/1022ccafe86ac05153110d69c2e3e214bdbc9703/components/ntp_snippets/breaking_news/subscription_manager_impl_unittest.cc
,
Feb 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/076714ff28974787a73a176457d90517dbba93d0 commit 076714ff28974787a73a176457d90517dbba93d0 Author: Colin Blundell <blundell@chromium.org> Date: Mon Feb 12 17:26:14 2018 [NTP Snippets] Convert ContentSuggestionsService to use IdentityManager This CL converts ContentSuggestionsService from using //components/signin to using the Identity Service client library. The conversion is straightforward: - Observe IdentityManager rather than SigninManager. I tested the relevant behavior manually by signing in/out on Android and verifying that: - The suggestions on the NTP changed. - An authenticated (resp. unauthenticated) fetch was shown as having been done in chrome://snippets-internals. TBR=gambard@chromium.org Bug: 798413 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I233e4b656b33fac8b1a02ce889db95cdd2ae93a6 Reviewed-on: https://chromium-review.googlesource.com/891159 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Jan Krcal <jkrcal@chromium.org> Cr-Commit-Position: refs/heads/master@{#536118} [modify] https://crrev.com/076714ff28974787a73a176457d90517dbba93d0/chrome/browser/android/ntp/content_suggestions_notifier_service_unittest.cc [modify] https://crrev.com/076714ff28974787a73a176457d90517dbba93d0/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc [modify] https://crrev.com/076714ff28974787a73a176457d90517dbba93d0/components/ntp_snippets/content_suggestions_service.cc [modify] https://crrev.com/076714ff28974787a73a176457d90517dbba93d0/components/ntp_snippets/content_suggestions_service.h [modify] https://crrev.com/076714ff28974787a73a176457d90517dbba93d0/components/ntp_snippets/content_suggestions_service_unittest.cc [modify] https://crrev.com/076714ff28974787a73a176457d90517dbba93d0/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory_util.cc
,
Feb 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/67b728fe355a16c1ecee007fb7dd91a539532739 commit 67b728fe355a16c1ecee007fb7dd91a539532739 Author: Colin Blundell <blundell@chromium.org> Date: Mon Feb 12 20:05:37 2018 [NTP Snippets] Remove unneeded includes and dependencies //components/ntp_snippets no longer uses //components/signin or //google_apis/gaia after the conversion to use the Identity Service client library. This CL removes remaining dead includes, test utility code, and dependencies. Some other includes/deps have been added as these removals revealed that they were missing. Bug: 798413 Change-Id: Ibf75700a6b1a94a4bf4939e76de9384e746a5e7f Reviewed-on: https://chromium-review.googlesource.com/891320 Reviewed-by: Jan Krcal <jkrcal@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#536167} [modify] https://crrev.com/67b728fe355a16c1ecee007fb7dd91a539532739/components/ntp_snippets/BUILD.gn [modify] https://crrev.com/67b728fe355a16c1ecee007fb7dd91a539532739/components/ntp_snippets/breaking_news/subscription_json_request.h [modify] https://crrev.com/67b728fe355a16c1ecee007fb7dd91a539532739/components/ntp_snippets/contextual/contextual_json_request.h [modify] https://crrev.com/67b728fe355a16c1ecee007fb7dd91a539532739/components/ntp_snippets/contextual/contextual_suggestions_fetcher_impl.cc [modify] https://crrev.com/67b728fe355a16c1ecee007fb7dd91a539532739/components/ntp_snippets/contextual/contextual_suggestions_fetcher_impl.h [modify] https://crrev.com/67b728fe355a16c1ecee007fb7dd91a539532739/components/ntp_snippets/remote/json_request.h [modify] https://crrev.com/67b728fe355a16c1ecee007fb7dd91a539532739/components/ntp_snippets/remote/remote_suggestions_fetcher_impl.cc [modify] https://crrev.com/67b728fe355a16c1ecee007fb7dd91a539532739/components/ntp_snippets/remote/remote_suggestions_fetcher_impl.h [modify] https://crrev.com/67b728fe355a16c1ecee007fb7dd91a539532739/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc [modify] https://crrev.com/67b728fe355a16c1ecee007fb7dd91a539532739/components/ntp_snippets/remote/test_utils.cc [modify] https://crrev.com/67b728fe355a16c1ecee007fb7dd91a539532739/components/ntp_snippets/remote/test_utils.h
,
Feb 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe56cf921b0b202657ef1d5bfbb2b25de4f44019 commit fe56cf921b0b202657ef1d5bfbb2b25de4f44019 Author: Colin Blundell <blundell@chromium.org> Date: Tue Feb 13 14:27:36 2018 [NTP Snippets] Remove usage of internal signin classes in factories This CL eliminates the minimal usage of SigninManager in the ContentSuggestionsService factories on Android and iOS, replacing that usage with equivalent usage of IdentityManager. It also eliminates the dependencies on SigninManager and ProfileOAuth2TokenService entirely. To test the minimal usage change, I verified that the first remote snippets fetch done on Android on cold start of the browser was authenticated if the user was signed in, and unauthenticated if the user was not signed in. TBR=gambard@chromium.org Bug: 798413 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: Ifb79778ef385441634e7624a1c99d366a579b0b2 Reviewed-on: https://chromium-review.googlesource.com/904043 Reviewed-by: Gauthier Ambard <gambard@chromium.org> Reviewed-by: Jan Krcal <jkrcal@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#536352} [modify] https://crrev.com/fe56cf921b0b202657ef1d5bfbb2b25de4f44019/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc [modify] https://crrev.com/fe56cf921b0b202657ef1d5bfbb2b25de4f44019/ios/chrome/browser/ntp_snippets/BUILD.gn [modify] https://crrev.com/fe56cf921b0b202657ef1d5bfbb2b25de4f44019/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc [modify] https://crrev.com/fe56cf921b0b202657ef1d5bfbb2b25de4f44019/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory_util.cc
,
Feb 13 2018
,
Feb 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4fe42077fff1897f316568daa2f7381f331b7402 commit 4fe42077fff1897f316568daa2f7381f331b7402 Author: Colin Blundell <blundell@chromium.org> Date: Tue Feb 27 16:30:17 2018 Fixup of how AutofillClient impls get IdentityManager after r537790 https://chromium-review.googlesource.com/904992 added a new AutofillClient::GetIdentityManager() interface. That CL mistakenly had the ChromeAutofillClient and ChromeAutofillClientIOS implementations return the IdentityManager instance associated with the current Profile. It should actually be the IdentityManager instance associated with the *original Profile* (/ChromeBrowserState) to be parallel with those clients' constructions of the ProfileIdentityProvider instances whose usage IdentityManager is replacing in //components/autofill. Note that I verified that these are the only client implementations that need this fix: - AWAutofillClient returns nullptr for both the IdentityProvider and the IdentityManager. - WebViewAutofillClient (in //ios) doesn't use the original ChromeBrowserState for anything, I assume because it doesn't have incognito. This bug was uncovered by the UBSanVptr bot. Thanks, UBSanVptr bot! Bug: 798413 , 814308 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I3e87888bc622204a29b4f9d3990fa39e8b165eb3 Reviewed-on: https://chromium-review.googlesource.com/928654 Reviewed-by: Mathieu Perreault <mathp@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Jared Saul <jsaul@google.com> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#539451} [modify] https://crrev.com/4fe42077fff1897f316568daa2f7381f331b7402/chrome/browser/ui/autofill/chrome_autofill_client.cc [modify] https://crrev.com/4fe42077fff1897f316568daa2f7381f331b7402/ios/chrome/browser/ui/autofill/chrome_autofill_client_ios.mm |
||
►
Sign in to add a comment |
||
Comment 1 by blundell@chromium.org
, Jan 11 2018Status: Started (was: Available)