Remove Geolocation AccessTokenStore |
||||||||
Issue descriptionIt turns out that access tokens are not required (and are ignored) by the geolocate[1] Maps API called by NetworkLocationProvider. (Our support for access tokens was migrated from an earlier Gears geolocation API which is long since deprecated.) Therefore we can remove the logic for persisting access tokens between calls to the network geolocation API, i.e. remove AccessTokenStore class. Detailed plan to follow. [1]: https://developers.google.com/maps/documentation/geolocation/intro
,
Sep 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7697a35a5c86bf1a26d22c62d638ad7fa387eb2a commit 7697a35a5c86bf1a26d22c62d638ad7fa387eb2a Author: Andrew Moylan <amoylan@chromium.org> Date: Thu Sep 07 01:44:42 2017 Don't send or expect access tokens with network geolocation requests This CL stops NetworkLocationRequest from sending or expecting to receive access tokens from the Maps Geolocation API. After this change, NetworkLocationProvider and NetworkLocationRequest are unaware of access tokens. This is part of the overall removal of access tokens, which are not part of the Geolocation API any more. See design: https://docs.google.com/document/d/1A-ZYMadJkwdQad_pC3MbpuCVNOOnRP_UZjGnXl-FOqE/edit?pli=1# This CL is step 1 of the implementation plan outlined there. TEST=Ran these: device_unittests browser_tests --gtest_filter="Geolocation*" contentunit_tests --gtest_filter="Geolocation*" Bug: 748921 Change-Id: I009dd9ff0816a5932dbd19cdb2aabf1d13ed18f2 Reviewed-on: https://chromium-review.googlesource.com/633259 Commit-Queue: Andrew Moylan <amoylan@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#500172} [modify] https://crrev.com/7697a35a5c86bf1a26d22c62d638ad7fa387eb2a/device/geolocation/location_arbitrator.cc [modify] https://crrev.com/7697a35a5c86bf1a26d22c62d638ad7fa387eb2a/device/geolocation/network_location_provider.cc [modify] https://crrev.com/7697a35a5c86bf1a26d22c62d638ad7fa387eb2a/device/geolocation/network_location_provider.h [modify] https://crrev.com/7697a35a5c86bf1a26d22c62d638ad7fa387eb2a/device/geolocation/network_location_provider_unittest.cc [modify] https://crrev.com/7697a35a5c86bf1a26d22c62d638ad7fa387eb2a/device/geolocation/network_location_request.cc [modify] https://crrev.com/7697a35a5c86bf1a26d22c62d638ad7fa387eb2a/device/geolocation/network_location_request.h
,
Sep 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cca5b6582227c7d9d643f87bb5b51533306a336d commit cca5b6582227c7d9d643f87bb5b51533306a336d Author: Andrew Moylan <amoylan@chromium.org> Date: Thu Sep 07 03:22:59 2017 Explicitly disable network location for shell The content shell's implementation of AccessTokenStore returns a dummy (empty-string) endpoint URL for network geolocation. The effect of this is to cause NetworkLocationProvider::StartProvider() to return (false) early and not start fetching location updates. This also triggers a LOG(WARNING) of !url.is_valid(). This CL instead explicitly disables network geolocation (via the existing UseNetworkLocationProviders() in GeolocationDelegate) for the content shell. The LOG(WARNING) is elevated to a DCHECK in NetworkLocationRequest. This removes an edge case in the kinds of URLs passed to NetworkLocationProvider and is a step towards making NetworkLocationProvider always use the Google Maps Geolocation API endpoint URL (realistically the only URL with which it is compatible). This is in turn part of the removal of AccessTokenStore and is described in more detail in the design: https://docs.google.com/document/d/1A-ZYMadJkwdQad_pC3MbpuCVNOOnRP_UZjGnXl-FOqE/edit?pli=1# Bug: 748921 Change-Id: Iaab7072c86fcd989155e5b9521ca4651ef59a3f7 Reviewed-on: https://chromium-review.googlesource.com/641175 Commit-Queue: Andrew Moylan <amoylan@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#500205} [modify] https://crrev.com/cca5b6582227c7d9d643f87bb5b51533306a336d/content/shell/browser/shell_access_token_store.cc [modify] https://crrev.com/cca5b6582227c7d9d643f87bb5b51533306a336d/content/shell/browser/shell_browser_main_parts.cc [modify] https://crrev.com/cca5b6582227c7d9d643f87bb5b51533306a336d/device/geolocation/network_location_provider.cc [modify] https://crrev.com/cca5b6582227c7d9d643f87bb5b51533306a336d/device/geolocation/network_location_request.cc
,
Sep 8 2017
,
Sep 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ca1a7c380122e8dcfa270d1aaf71462a4372203a commit ca1a7c380122e8dcfa270d1aaf71462a4372203a Author: Andrew Moylan <amoylan@chromium.org> Date: Sun Sep 10 23:27:28 2017 Expand comments in NetworkLocationProvider tests Here I am just sprucing up the unittests for NetworkLocationProvider with some comments that I would have found handy when studying these, as part of updating them for some upcoming changes (separate future CL). Also removed a relic unused argument of one helper function. Bug: 748921 Change-Id: Ifdb47837a6be66a1d1a728b88f219f4291ca7606 Reviewed-on: https://chromium-review.googlesource.com/656879 Commit-Queue: Andrew Moylan <amoylan@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#500817} [modify] https://crrev.com/ca1a7c380122e8dcfa270d1aaf71462a4372203a/device/geolocation/network_location_provider_unittest.cc
,
Sep 13 2017
,
Sep 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ce4c522e456697bc8a31b4bdac30ed3bbc4be386 commit ce4c522e456697bc8a31b4bdac30ed3bbc4be386 Author: Andrew Moylan <amoylan@chromium.org> Date: Wed Sep 13 01:10:55 2017 Remove shell & webview AccessTokenStores Content shell and Android webview don't make network geolocation requests. Previously these embedders had dummy (no-op) implementations of AccessTokenStore. This CL removes the dummy implementations and instead uses the existing UseNetworkLocationProviders flag in GeolocationDelegate as a gate. This is part of the removal of all AccessTokenStores. Overall design: https://docs.google.com/document/d/1A-ZYMadJkwdQad_pC3MbpuCVNOOnRP_UZjGnXl-FOqE Details of reasoning why this is safe: Content shell: AccessTokenStore was being created but LoadAccessTokens was not being called on it (after crrev.com/c/641175). Android webview: AccessTokenStore was being created and LoadAccessTokens was being called on it, but to no consequence due to an "#if defined(OS_ANDROID)" block at location_arbitrator.cc:174. Bug: 748921 Change-Id: I10ed3668ee015f855acf604e445643617bbe7713 Reviewed-on: https://chromium-review.googlesource.com/658010 Commit-Queue: Andrew Moylan <amoylan@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Selim Gurun <sgurun@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#501494} [modify] https://crrev.com/ce4c522e456697bc8a31b4bdac30ed3bbc4be386/android_webview/browser/aw_browser_main_parts.cc [modify] https://crrev.com/ce4c522e456697bc8a31b4bdac30ed3bbc4be386/content/shell/BUILD.gn [delete] https://crrev.com/fa312b778f668d1c9576da18969efcb3576d6f77/content/shell/browser/shell_access_token_store.cc [delete] https://crrev.com/fa312b778f668d1c9576da18969efcb3576d6f77/content/shell/browser/shell_access_token_store.h [modify] https://crrev.com/ce4c522e456697bc8a31b4bdac30ed3bbc4be386/content/shell/browser/shell_browser_main_parts.cc [modify] https://crrev.com/ce4c522e456697bc8a31b4bdac30ed3bbc4be386/device/geolocation/geolocation_delegate.h [modify] https://crrev.com/ce4c522e456697bc8a31b4bdac30ed3bbc4be386/device/geolocation/location_arbitrator.cc
,
Sep 14 2017
,
Sep 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/babe76124f3e49dfe5c895ded11060b38ec30c35 commit babe76124f3e49dfe5c895ded11060b38ec30c35 Author: Andrew Moylan <amoylan@chromium.org> Date: Fri Sep 22 00:10:31 2017 Remove UseNetworkLocationProviders This CL removes GeolocationDelegate::UseNetworkLocationProviders. Its responsibility (of optionally disabling network geolocation) is explicitly moved to GeolocationDelegate::CreateAccessTokenStore. Today GeolocationDelegate has UseNetworkLocationProviders (default true) and CreateAccessTokenStore (default nullptr), and these are effectively &&'d together by LocationArbitrator to decide whether to try to use network geolocation. Since AccessTokenStore has other uses, UseNetworkLocationProviders is essentially redundant. I surveyed embedders internal and external to Chromium repo and found no cases of contradictory use of these two signals (i.e. no cases of setting UseNetworkLocationProviders to false but still overriding CreateAccessTokenStore to return a non-nullptr). I also spruced up the LocationArbitrator unit tests with some comments and made them a bit more "injecty" to simplify understanding them. Bug: 748921 Change-Id: I528f6f0a81263df868795be81201141c260aa836 Reviewed-on: https://chromium-review.googlesource.com/670964 Reviewed-by: Selim Gurun <sgurun@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Miguel Casas <mcasas@chromium.org> Commit-Queue: Andrew Moylan <amoylan@chromium.org> Cr-Commit-Position: refs/heads/master@{#503619} [modify] https://crrev.com/babe76124f3e49dfe5c895ded11060b38ec30c35/android_webview/browser/aw_browser_main_parts.cc [modify] https://crrev.com/babe76124f3e49dfe5c895ded11060b38ec30c35/content/shell/browser/shell_browser_main_parts.cc [modify] https://crrev.com/babe76124f3e49dfe5c895ded11060b38ec30c35/device/geolocation/geolocation_delegate.cc [modify] https://crrev.com/babe76124f3e49dfe5c895ded11060b38ec30c35/device/geolocation/geolocation_delegate.h [modify] https://crrev.com/babe76124f3e49dfe5c895ded11060b38ec30c35/device/geolocation/location_arbitrator.cc [modify] https://crrev.com/babe76124f3e49dfe5c895ded11060b38ec30c35/device/geolocation/location_arbitrator_unittest.cc
,
Sep 22 2017
,
Sep 22 2017
,
Sep 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/17329fccf83134a098084e70a1213564d71cfa6f commit 17329fccf83134a098084e70a1213564d71cfa6f Author: Andrew Moylan <amoylan@chromium.org> Date: Mon Sep 25 07:43:30 2017 Use fixed endpoint URL for network geolocation This change makes NetworkLocationRequest no longer accept a user-provided endpoint URL for geolocation. Instead, it always uses the Google Maps API geolocate endpoint URL (which is the existing default). In practice, this is the only URL which can be used anyway, as NetworkLocationRequest's implementation is tied closely to the details of that API. With this change, LocationArbitrator always creates exactly one NetworkLocationProvider, instead of creating one NetworkLocationProvider per entry in the access token store. The theoretical possibility of using multiple NetworkLocationProviders with different URLs was not useful in practice because NetworkLocationProvider/NetworkLocationRequest are closely tied specifically to the Google Maps API interface. For more details / motivation / results of looking at internal/external embedders, please see the following design document: https://docs.google.com/document/d/1Bkd06tmAFb9OY0qkq_IeWc4kr3ZDxzhrYkg3mGUMGRE The only cases where a non-default URL appear to be in use (internal or external to Chromium repo) are for embedders manually appending their Googgle API key to the URL. Therefore, this change adds support for setting the geolocation API key if desired. Embedders do this by overriding ContentBrowserClient::GetGeolocationApiKey(). Embedders who have been using the default endpoint URL were automatically using the key taken from google_apis::GetAPIKey(). These embedders will now need to override GetGeolocationApiKey() to return google_apis::GetAPIKey(). This CL does this for Chrome & Chromecast, which are the only two embedders in Chromium that are using NetworkLocationProviders. [Another option would be to make google_apis::GetAPIKey() the default implementation (Chrome and Chromecast then wouldn't need to provide an override). I didn't go for this as content/ has a -"google_apis" DEPS entry and expresses a preference for embedders using google_apis directly.] This change is part of the overall removal of AccessTokenStore, as detailed in the above doc. Misc: - Added an opportunistic integration test checking that, for Chrome, the expected URL and API key does in fact get used. - Removed an unused layer of abstraction in the form of the |NewNetworkLocationProvider| factory function. Bug: 748921 , 718694 Change-Id: I12bae25908c48cfbe7fb93cc2d25f1a91ce03bd8 Reviewed-on: https://chromium-review.googlesource.com/675023 Commit-Queue: Andrew Moylan <amoylan@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Raymes Khoury <raymes@chromium.org> Reviewed-by: Miguel Casas <mcasas@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#504002} [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/chrome/browser/geolocation/geolocation_browsertest.cc [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/content/browser/service_manager/service_manager_context.cc [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/content/public/browser/content_browser_client.cc [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/content/public/browser/content_browser_client.h [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/BUILD.gn [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/DEPS [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/geolocation_provider.h [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/geolocation_provider_impl.cc [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/location_arbitrator.cc [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/location_arbitrator.h [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/location_arbitrator_unittest.cc [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/network_location_provider.cc [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/network_location_provider.h [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/network_location_provider_unittest.cc [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/network_location_request.cc [modify] https://crrev.com/17329fccf83134a098084e70a1213564d71cfa6f/device/geolocation/network_location_request.h
,
Sep 29 2017
,
Sep 29 2017
,
Oct 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7 commit 8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7 Author: Andrew Moylan <amoylan@chromium.org> Date: Tue Oct 10 04:29:04 2017 Get Geolocation URL context via ContentBrowserClient This CL changes the Geolocation system's LocationArbitrator to obtain a URLRequestContextGetter (used for network geolocation requests) from an embedder-provided method in ContentBrowserClient. Previously, the URLRequestContextGetter was provided as a side effect of the embedder-provided AccessTokenStore::LoadAccessTokens. The motivations for this change are the following: 1. AccessTokenStore is going away as access tokens are no longer needed: https://docs.google.com/document/d/1Bkd06tmAFb9OY0qkq_IeWc4kr3ZDxzhrYkg3mGUMGRE 2. Geolocation is moving to Device service and ContentBrowserClient is an appropriate home for embedder supplied parameters in that world. With this change, AccessTokenStore::LoadAccessTokens is no longer called by LocationArbitrator (or by any non-test code). The only remaining use for AccessTokenStore is its role in determining whether network geolocation should be performed. (Namely: If CreateAccessTokenStore() returns non-nullptr, then do perform network geolocation.) This final use of AccessTokenStore will be removed in a followup CL (a nullptr URLRequestContextGetter will become the flag instead). Following that, AccessTokenStore will be removed wholly. The embedders which currently use network geolocation are Chrome and Chromecast (reasoning: these are the only implementers of CreateAccessTokenStore()). Therefore these two embedders have been provided with implementations of ContentBrowserClient::GetGeolocationRequestContextGetter. The implementations are identical to the existing implementations in ChromeAccessTokenStore and CastAccessTokenStore respectively. I explored the possibility of removing the existing (now unused) URLRequestContextGetter functionality from (Chrome|Cast)AccessTokenStore. I opted not to do so because the associated modifications of existing access token store unit tests end up requiring either (a) quite a bit of refactoring to make them rational, or (b) minimal surgery leaving them in a weird state. Neither option seems as comprehensible as leaving the functionality intact but unused. For reference, the existing Chrome request-context-fetching code can be found at: chrome/browser/geolocation/chrome_access_token_store.cc:86 And the existing cast code can be found at: chromecast/browser/geolocation/cast_access_token_store.cc:25 Bug: 748921 , 718694 Change-Id: I84301bbb3152639ac329ca36ed5284a28aed4747 Reviewed-on: https://chromium-review.googlesource.com/674425 Commit-Queue: Andrew Moylan <amoylan@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Stephen Lanham <slan@chromium.org> Reviewed-by: Miguel Casas <mcasas@chromium.org> Reviewed-by: Colin Blundell (at a convergence, slow until Oct 16) <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#507585} [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/chromecast/browser/cast_content_browser_client.cc [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/chromecast/browser/cast_content_browser_client.h [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/content/browser/service_manager/service_manager_context.cc [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/content/public/browser/content_browser_client.cc [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/content/public/browser/content_browser_client.h [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/device/geolocation/geolocation_provider.h [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/device/geolocation/geolocation_provider_impl.cc [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/device/geolocation/location_arbitrator.cc [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/device/geolocation/location_arbitrator.h [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/device/geolocation/location_arbitrator_unittest.cc [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/device/geolocation/network_location_provider.cc [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/device/geolocation/network_location_provider.h [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/device/geolocation/network_location_request.cc [modify] https://crrev.com/8673dbae36f4ba1a4d7fc8319af8c7d2882fddd7/device/geolocation/network_location_request.h
,
Oct 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/83b35b2be33e82a7bd7163b5d7628eb45141c379 commit 83b35b2be33e82a7bd7163b5d7628eb45141c379 Author: Andrew Moylan <amoylan@chromium.org> Date: Tue Oct 17 22:03:29 2017 Return void from LocationProvider::StartProvider This CL changes the return type of LocationProvider::StartProvider (and related functions) from bool to void. Currently, no-one inspects the value returned from this function, except to pass it through to other places that don't inspect it. Most impls always return true. The situation where an impl doesn't always return true (LocationArbitrator) are responded-to by other means. Specifically, LocationArbitrator provides an Error geoposition in the case that it couldn't create any location providers. I also opportunistically removed the unused MockLocationProvider in this CL. Bug: 748921 Change-Id: Iaa7903b3eecaacc0076b3c1f1cce3be86187285d Reviewed-on: https://chromium-review.googlesource.com/722139 Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Miguel Casas <mcasas@chromium.org> Commit-Queue: Andrew Moylan <amoylan@chromium.org> Cr-Commit-Position: refs/heads/master@{#509561} [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/BUILD.gn [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/android/java/src/org/chromium/device/geolocation/LocationProviderAdapter.java [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/fake_location_provider.cc [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/fake_location_provider.h [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/location_api_adapter_android.cc [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/location_api_adapter_android.h [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/location_arbitrator.cc [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/location_arbitrator.h [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/location_provider.h [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/location_provider_android.cc [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/location_provider_android.h [delete] https://crrev.com/eda21d3879cf6451ab838530a2ca25167fa3b7c5/device/geolocation/mock_location_provider.cc [delete] https://crrev.com/eda21d3879cf6451ab838530a2ca25167fa3b7c5/device/geolocation/mock_location_provider.h [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/network_location_provider.cc [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/network_location_provider.h [modify] https://crrev.com/83b35b2be33e82a7bd7163b5d7628eb45141c379/device/geolocation/network_location_provider_unittest.cc
,
Oct 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b5081e3d2cc6d46805a6ce6362eae1754d5fa1db commit b5081e3d2cc6d46805a6ce6362eae1754d5fa1db Author: Andrew Moylan <amoylan@chromium.org> Date: Wed Oct 18 00:11:28 2017 Skip network geolocation if request context = null This CL changes the way embedders enable/disable network geolocation: Before: Disabled if GeolocationDelegate::CreateAccessTokenStore() returns null (the default). Now: Disabled if ContentBrowserClient::GetGeolocationRequestContext produces a nullptr URLRequestContextGetter (also the default). This change doesn't alter which embedders enable/disable network geolocation, because the only two embedders that currently return a non-null result from CreateAccessTokenStore() are also the only two embedders that produce a non-null result from GetGeolocationRequestContextGetter: Chrome and Chromecast. Embedders external to the Chromium repo will need to implement GetGeolocationRequestContext: https://groups.google.com/a/chromium.org/forum/#!topic/embedder-dev/0ln9lxrsBw8 With this change, LocationArbitrator no longer knows about AccessTokenStore at all, and AccessTokenStore becomes fully unused. It can be removed in a subsequent CL. This is a step in the removal of AccessTokenStore as detailed here: https://docs.google.com/document/d/1A-ZYMadJkwdQad_pC3MbpuCVNOOnRP_UZjGnXl-FOqE Bug: 748921 Change-Id: Id413a0cdb207fcec8fd25f381068c6c13ad4068c Reviewed-on: https://chromium-review.googlesource.com/676826 Commit-Queue: Andrew Moylan <amoylan@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Miguel Casas <mcasas@chromium.org> Cr-Commit-Position: refs/heads/master@{#509616} [modify] https://crrev.com/b5081e3d2cc6d46805a6ce6362eae1754d5fa1db/device/geolocation/location_arbitrator.cc [modify] https://crrev.com/b5081e3d2cc6d46805a6ce6362eae1754d5fa1db/device/geolocation/location_arbitrator.h [modify] https://crrev.com/b5081e3d2cc6d46805a6ce6362eae1754d5fa1db/device/geolocation/location_arbitrator_unittest.cc
,
Oct 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2 commit 2dbc85c3b174787fecc1aaa96d0eda2ee759ced2 Author: Andrew Moylan <amoylan@chromium.org> Date: Fri Oct 20 06:08:33 2017 Remove AccessTokenStore This CL removes AccessTokenStore and derived classes. After previous CLs as part of crbug.com/748921 , this class is now unused. This CL also removes the registration of the access_token_store pref field which won't be used any more. Embedders which were providing non-trivial overrides of AccessTokenStore should read: https://groups.google.com/a/chromium.org/forum/#!topic/embedder-dev/0ln9lxrsBw8 This is the last step in the removal of AccessTokenStore as detailed in the design: https://docs.google.com/document/d/1A-ZYMadJkwdQad_pC3MbpuCVNOOnRP_UZjGnXl-FOqE Bug: 748921 Change-Id: Iad74af6fca5e261970ada5a5ad9e58e98fe83293 Reviewed-on: https://chromium-review.googlesource.com/708494 Reviewed-by: Stephen Lanham <slan@chromium.org> Reviewed-by: Bo <boliu@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Miguel Casas <mcasas@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Commit-Queue: Andrew Moylan <amoylan@chromium.org> Cr-Commit-Position: refs/heads/master@{#510354} [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/android_webview/browser/aw_browser_main_parts.cc [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/android_webview/browser/aw_content_browser_client.cc [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/chrome/browser/BUILD.gn [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/chrome/browser/chrome_browser_main.cc [delete] https://crrev.com/f743d39ebb54688d338611d43225b70cd3076350/chrome/browser/geolocation/access_token_store_browsertest.cc [delete] https://crrev.com/f743d39ebb54688d338611d43225b70cd3076350/chrome/browser/geolocation/chrome_access_token_store.cc [delete] https://crrev.com/f743d39ebb54688d338611d43225b70cd3076350/chrome/browser/geolocation/chrome_access_token_store.h [delete] https://crrev.com/f743d39ebb54688d338611d43225b70cd3076350/chrome/browser/geolocation/geolocation_prefs.cc [delete] https://crrev.com/f743d39ebb54688d338611d43225b70cd3076350/chrome/browser/geolocation/geolocation_prefs.h [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/chrome/browser/prefs/browser_prefs.cc [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/chrome/test/BUILD.gn [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/chromecast/browser/BUILD.gn [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/chromecast/browser/cast_browser_main_parts.cc [delete] https://crrev.com/f743d39ebb54688d338611d43225b70cd3076350/chromecast/browser/geolocation/cast_access_token_store.cc [delete] https://crrev.com/f743d39ebb54688d338611d43225b70cd3076350/chromecast/browser/geolocation/cast_access_token_store.h [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/content/shell/browser/shell_browser_main_parts.cc [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/device/geolocation/BUILD.gn [delete] https://crrev.com/f743d39ebb54688d338611d43225b70cd3076350/device/geolocation/access_token_store.h [delete] https://crrev.com/f743d39ebb54688d338611d43225b70cd3076350/device/geolocation/fake_access_token_store.cc [delete] https://crrev.com/f743d39ebb54688d338611d43225b70cd3076350/device/geolocation/fake_access_token_store.h [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/device/geolocation/geolocation_delegate.cc [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/device/geolocation/geolocation_delegate.h [modify] https://crrev.com/2dbc85c3b174787fecc1aaa96d0eda2ee759ced2/device/geolocation/geolocation_provider_impl_unittest.cc
,
Oct 20 2017
\o/
,
Oct 20 2017
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by amoylan@chromium.org
, Aug 4 2017