New issue
Advanced search Search tips

Issue 623114 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 612334



Sign in to add a comment

Consider making CreateAccessTokenStore() return std::unique_ptr<AccessTokenStore>

Project Member Reported by mcasas@chromium.org, Jun 24 2016

Issue description

AccessTokenStores are created for Geolocation via ContentBrowserClient's
method
 virtual AccessTokenStore* CreateAccessTokenStore();

its implementations in {Chrome,Shell}ContentBrowserClient [2,3]
return a new object. Consider making the whole chain based on
std::unique_ptr<AccessTokenStore> to clarify ownership.

[1] https://cs.chromium.org/chromium/src/content/public/browser/content_browser_client.h?type=cs&q=%22virtual+AccessTokenStore*+CreateAccessTokenStore%22&sq=package:chromium&l=535

[2] https://cs.chromium.org/chromium/src/content/public/browser/content_browser_client.h?type=cs&q=%22virtual+AccessTokenStore*+CreateAccessTokenStore%22&sq=package:chromium&l=535

[3] https://cs.chromium.org/chromium/src/chrome/browser/chrome_content_browser_client.cc?type=cs&sq=package:chromium&rcl=1466767076&l=2249
 

Comment 1 by mcasas@chromium.org, Jun 24 2016

Cc: w...@chromium.org
Owner: mcasas@chromium.org
Status: Started (was: Available)
Blocking: 612334
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 19 2016

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

commit 1161c5753adf296ea253d56f1617d3d926152b55
Author: mcasas <mcasas@chromium.org>
Date: Tue Jul 19 23:24:15 2016

Geolocation cleanup: corrects uses of content::AccessTokenStore* and net::URLRequestContextGetter*

This CL corrects uses of content::AccessTokenStore* to scoped_refptr<>
versions, since that class is ref-counted.

Same applies toc* .

Also this CL does some cleanups in the touched files:
- s/NULL/nullptr/
- makes const and/or preferred initialize over assignment for
  some member variables, where applicable/available,
- method ::GetAccessTokenStore() is made private.
- NewSystemLocationProvider(); is changed to return std::unique_ptr<>

TEST=all relevant unittests and browser_tests working, in
particular
./out/gn/browser_tests --gtest_filter="Geolocation*"
./out/gn/content_unittests --gtest_filter=Geolocation*
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

BUG= 623114 

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

[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/android_webview/browser/aw_content_browser_client.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/chrome/browser/geolocation/access_token_store_browsertest.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/chromecast/browser/cast_content_browser_client.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/browser/geolocation/fake_access_token_store.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/browser/geolocation/location_arbitrator_impl.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/browser/geolocation/location_arbitrator_impl.h
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/browser/geolocation/location_arbitrator_impl_unittest.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/browser/geolocation/location_provider_android.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/browser/geolocation/network_location_provider.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/browser/geolocation/network_location_provider.h
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/browser/geolocation/network_location_provider_unittest.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/browser/geolocation/network_location_request.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/browser/geolocation/network_location_request.h
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/public/browser/access_token_store.h
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/public/browser/geolocation_delegate.cc
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/public/browser/geolocation_delegate.h
[modify] https://crrev.com/1161c5753adf296ea253d56f1617d3d926152b55/content/shell/browser/shell_content_browser_client.cc

Comment 5 by mcasas@chromium.org, Jul 20 2016

Status: Fixed (was: Started)
Components: Blink>Geolocation
Components: -Blink>Location

Sign in to add a comment