Switch to using scoped_ptr with UserData

Switch to using scoped_ptr with UserData

BUG=690937
TEST=none

Review-Url: https://codereview.chromium.org/2815913005
Cr-Commit-Position: refs/heads/master@{#466896}
Committed: https://chromium.googlesource.com/chromium/src/+/1ed4a43703873eced52dae51c62682fef5cfd9b7

[Avi (use Gerrit), 2017-04-13 21:34:48]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 21:35:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 21:46:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 21:46:40]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Avi (use Gerrit), 2017-04-13 23:05:39]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 23:06:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-14 02:47:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-14 02:47:49]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Avi (use Gerrit), 2017-04-14 14:28:50]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-14 14:28:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Avi (use Gerrit), 2017-04-14 14:35:43]

avi@chromium.org changed reviewers:
+ davidben@chromium.org, nasko@chromium.org, rogerta@chromium.org,
sdefresne@chromium.org, thakis@chromium.org

[Avi (use Gerrit), 2017-04-14 14:35:44]

Nico: chrome/
Sylvain: components/
Nasko: content/
Roger: google_apis/
David: net/

[Nico, 2017-04-14 15:46:58]

Thanks for all the CLs pointing out how wonky our ownership is everywhere ;-)

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
(right):

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc:519:
browser_context, base::WrapUnique(download_manager_));
That looks really surprising, having a member owned by a thing that gets passed
in. At least add a comment to the member variable explaining this; ideally
reshuffle things to make ownership less weird.

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/browsing...
File
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
(right):

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc:735:
testing_profile, base::WrapUnique(download_manager_));
Same here.

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/downloads_handler_unittest.cc (right):

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/downloads_handler_unittest.cc:28: &profile_,
base::WrapUnique(download_manager_));
and here

[Avi (use Gerrit), 2017-04-14 18:16:30]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-14 18:16:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Avi (use Gerrit), 2017-04-14 18:17:51]

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
(right):

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc:519:
browser_context, base::WrapUnique(download_manager_));
On 2017/04/14 15:46:58, Nico wrote:
> That looks really surprising, having a member owned by a thing that gets
passed
> in. At least add a comment to the member variable explaining this; ideally
> reshuffle things to make ownership less weird.

Done.

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/browsing...
File
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
(right):

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc:735:
testing_profile, base::WrapUnique(download_manager_));
On 2017/04/14 15:46:58, Nico wrote:
> Same here.

Done.

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/downloads_handler_unittest.cc (right):

https://codereview.chromium.org/2815913005/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/downloads_handler_unittest.cc:28: &profile_,
base::WrapUnique(download_manager_));
On 2017/04/14 15:46:58, Nico wrote:
> and here

Done.

[Nico, 2017-04-14 18:53:20]

chrome/ lgtm, thanks

https://codereview.chromium.org/2815913005/diff/40001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
(right):

https://codereview.chromium.org/2815913005/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc:530:
content::MockDownloadManager* download_manager_;  // owned by browser context
nit: style guide says upper case, . at end (in the other files too, but
optional, and iirc many of our "owned by" comments look like this here)

https://codereview.chromium.org/2815913005/diff/40001/chrome/browser/browsing...
File
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
(right):

https://codereview.chromium.org/2815913005/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc:749:
content::MockDownloadManager* download_manager_;  // owned by testing profile
ditto

[Avi (use Gerrit), 2017-04-14 18:59:38]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[Avi (use Gerrit), 2017-04-14 18:59:39]

https://codereview.chromium.org/2815913005/diff/40001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
(right):

https://codereview.chromium.org/2815913005/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc:530:
content::MockDownloadManager* download_manager_;  // owned by browser context
On 2017/04/14 18:53:20, Nico wrote:
> nit: style guide says upper case, . at end (in the other files too, but
> optional, and iirc many of our "owned by" comments look like this here)

Done.

https://codereview.chromium.org/2815913005/diff/40001/chrome/browser/browsing...
File
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
(right):

https://codereview.chromium.org/2815913005/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc:749:
content::MockDownloadManager* download_manager_;  // owned by testing profile
On 2017/04/14 18:53:20, Nico wrote:
> ditto

Done.

[davidben, 2017-04-14 19:38:24]

net lgtm

https://codereview.chromium.org/2815913005/diff/60001/net/url_request/url_fet...
File net/url_request/url_fetcher_core.cc (right):

https://codereview.chromium.org/2815913005/diff/60001/net/url_request/url_fet...
net/url_request/url_fetcher_core.cc:12: #include "base/memory/ptr_util.h"
Stray change?

[davidben, 2017-04-14 19:38:24]

net lgtm

https://codereview.chromium.org/2815913005/diff/60001/net/url_request/url_fet...
File net/url_request/url_fetcher_core.cc (right):

https://codereview.chromium.org/2815913005/diff/60001/net/url_request/url_fet...
net/url_request/url_fetcher_core.cc:12: #include "base/memory/ptr_util.h"
Stray change?

[Avi (use Gerrit), 2017-04-14 19:49:20]

https://codereview.chromium.org/2815913005/diff/60001/net/url_request/url_fet...
File net/url_request/url_fetcher_core.cc (right):

https://codereview.chromium.org/2815913005/diff/60001/net/url_request/url_fet...
net/url_request/url_fetcher_core.cc:12: #include "base/memory/ptr_util.h"
On 2017/04/14 19:38:24, davidben wrote:
> Stray change?

Perhaps. Removing to see if things still work.

[Avi (use Gerrit), 2017-04-14 19:49:49]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-14 19:50:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-14 21:56:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-14 21:56:40]

Dry run: This issue passed the CQ dry run.

[sdefresne, 2017-04-18 10:39:41]

lgtm

[Roger Tawa OOO till Jul 10th, 2017-04-18 13:44:19]

google_apis lgtm

[nasko, 2017-04-19 17:56:55]

content/ LGTM

https://codereview.chromium.org/2815913005/diff/80001/content/browser/loader/...
File content/browser/loader/resource_request_info_impl.cc (right):

https://codereview.chromium.org/2815913005/diff/80001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.cc:310:
request->SetUserData(NULL, base::WrapUnique(this));
nit: nullptr while at it?

[Avi (use Gerrit), 2017-04-25 03:50:03]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2017-04-25 03:50:03]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org,
davidben@chromium.org, rogerta@chromium.org, sdefresne@chromium.org,
nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2815913005/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-25 03:50:23]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Avi (use Gerrit), 2017-04-25 03:50:28]

https://codereview.chromium.org/2815913005/diff/80001/content/browser/loader/...
File content/browser/loader/resource_request_info_impl.cc (right):

https://codereview.chromium.org/2815913005/diff/80001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.cc:310:
request->SetUserData(NULL, base::WrapUnique(this));
On 2017/04/19 17:56:55, nasko wrote:
> nit: nullptr while at it?

Done.

[commit-bot: I haz the power, 2017-04-25 05:39:44]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493092203235290,
"parent_rev": "e02af5b94d73c62ad0708593d4514aa15be4ee6d", "commit_rev":
"1ed4a43703873eced52dae51c62682fef5cfd9b7"}

[commit-bot: I haz the power, 2017-04-25 05:39:50]

Description was changed from

==========
Switch to using scoped_ptr with UserData

BUG=690937
TEST=none
==========

to

==========
Switch to using scoped_ptr with UserData

BUG=690937
TEST=none

Review-Url: https://codereview.chromium.org/2815913005
Cr-Commit-Position: refs/heads/master@{#466896}
Committed:
https://chromium.googlesource.com/chromium/src/+/1ed4a43703873eced52dae51c626...
==========

[commit-bot: I haz the power, 2017-04-25 05:39:52]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/1ed4a43703873eced52dae51c626...