Remove linked_ptr usage in //base.

Remove linked_ptr usage in //base.

And clean up some code that didn't IWYU.

BUG=556939
Committed: https://crrev.com/2588941279b28a6cf205896b955402e36535af6d
Cr-Commit-Position: refs/heads/master@{#372977}

[dcheng, 2016-01-27 07:56:25]

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

[commit-bot: I haz the power, 2016-01-27 07:56:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1641563002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1641563002/1

[commit-bot: I haz the power, 2016-01-27 08:13:39]

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

[commit-bot: I haz the power, 2016-01-27 08:13:39]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[dcheng, 2016-01-27 08:25:16]

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

[commit-bot: I haz the power, 2016-01-27 08:25:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1641563002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1641563002/20001

[commit-bot: I haz the power, 2016-01-27 09:13:59]

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

[commit-bot: I haz the power, 2016-01-27 09:13:59]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2016-01-27 22:17:40]

Description was changed from

==========
Remove linked_ptr usage in //base.

BUG=556939
==========

to

==========
Remove linked_ptr usage in //base.

And clean up some code that didn't IWYU.

BUG=556939
==========

[dcheng, 2016-01-27 22:17:40]

dcheng@chromium.org changed reviewers:
+ danakj@chromium.org

[dcheng, 2016-01-27 22:18:26]

dcheng@chromium.org changed reviewers:
+ paulmeyer@chromium.org

[dcheng, 2016-01-27 22:18:27]

+paulmeyer for guest view changes.

https://codereview.chromium.org/1641563002/diff/40001/chrome/browser/extensio...
File chrome/browser/extensions/api/web_view/chrome_web_view_internal_api.cc
(left):

https://codereview.chromium.org/1641563002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/web_view/chrome_web_view_internal_api.cc:151:
guest->ShowContextMenu(params->request_id, NULL);
The semantics for this aren't clear. Since it's not actually implemented (and
hasn't been for some time), it seems easier to just remove it...

https://codereview.chromium.org/1641563002/diff/40001/components/guest_view/b...
File components/guest_view/browser/guest_view_base.cc (right):

https://codereview.chromium.org/1641563002/diff/40001/components/guest_view/b...
components/guest_view/browser/guest_view_base.cc:761:
pending_events_.push_back(make_scoped_ptr(event));
Another alternative is:
pending_events_.emplace_back(event);

Though it's technically not on the C++11 allowed library features list.

https://codereview.chromium.org/1641563002/diff/40001/extensions/browser/gues...
File extensions/browser/guest_view/web_view/web_view_find_helper.h (right):

https://codereview.chromium.org/1641563002/diff/40001/extensions/browser/gues...
extensions/browser/guest_view/web_view/web_view_find_helper.h:178:
base::WeakPtr<FindInfo> current_find_session_;
An alternative is to make this a raw pointer, but that makes erasing from the
map more complicated. Since there are three different places in code that erase
from the map, and FindInfo already exposes itself as a WeakPtr, this seemed the
simplest.

[danakj, 2016-01-27 22:20:51]

https://codereview.chromium.org/1641563002/diff/40001/base/supports_user_data.cc
File base/supports_user_data.cc (right):

https://codereview.chromium.org/1641563002/diff/40001/base/supports_user_data...
base/supports_user_data.cc:22: void SupportsUserData::SetUserData(const void*
key, Data* data) {
can you make this function take scoped_ptr<Data>?

https://codereview.chromium.org/1641563002/diff/40001/base/threading/sequence...
File base/threading/sequenced_worker_pool.cc (right):

https://codereview.chromium.org/1641563002/diff/40001/base/threading/sequence...
base/threading/sequenced_worker_pool.cc:791: std::pair<ThreadMap::iterator,
bool> result = threads_.insert(
unrelated, but: auto pair_result? This is the land of easy to write the wrong
type.

[dcheng, 2016-01-27 22:26:01]

https://codereview.chromium.org/1641563002/diff/40001/base/supports_user_data.cc
File base/supports_user_data.cc (right):

https://codereview.chromium.org/1641563002/diff/40001/base/supports_user_data...
base/supports_user_data.cc:22: void SupportsUserData::SetUserData(const void*
key, Data* data) {
On 2016/01/27 at 22:20:51, danakj wrote:
> can you make this function take scoped_ptr<Data>?

There's 100+ usages of this, I was hoping that could happen separately =)

https://codereview.chromium.org/1641563002/diff/40001/base/threading/sequence...
File base/threading/sequenced_worker_pool.cc (right):

https://codereview.chromium.org/1641563002/diff/40001/base/threading/sequence...
base/threading/sequenced_worker_pool.cc:791: std::pair<ThreadMap::iterator,
bool> result = threads_.insert(
On 2016/01/27 at 22:20:51, danakj wrote:
> unrelated, but: auto pair_result? This is the land of easy to write the wrong
type.

Done.

[danakj, 2016-01-27 22:28:31]

https://codereview.chromium.org/1641563002/diff/40001/base/supports_user_data.cc
File base/supports_user_data.cc (right):

https://codereview.chromium.org/1641563002/diff/40001/base/supports_user_data...
base/supports_user_data.cc:22: void SupportsUserData::SetUserData(const void*
key, Data* data) {
On 2016/01/27 22:26:01, dcheng wrote:
> On 2016/01/27 at 22:20:51, danakj wrote:
> > can you make this function take scoped_ptr<Data>?
> 
> There's 100+ usages of this, I was hoping that could happen separately =)

I suppose. base/ LGTM scoped_ptr is better than linked_ptr. Should there be a
bug to change Data*?

[dcheng, 2016-01-27 22:30:30]

https://codereview.chromium.org/1641563002/diff/40001/base/supports_user_data.cc
File base/supports_user_data.cc (right):

https://codereview.chromium.org/1641563002/diff/40001/base/supports_user_data...
base/supports_user_data.cc:22: void SupportsUserData::SetUserData(const void*
key, Data* data) {
On 2016/01/27 at 22:28:31, danakj wrote:
> On 2016/01/27 22:26:01, dcheng wrote:
> > On 2016/01/27 at 22:20:51, danakj wrote:
> > > can you make this function take scoped_ptr<Data>?
> > 
> > There's 100+ usages of this, I was hoping that could happen separately =)
> 
> I suppose. base/ LGTM scoped_ptr is better than linked_ptr. Should there be a
bug to change Data*?

Don't think so, I can create one. There's a couple other base APIs that could
benefit from similar cleanup (base::ListValue::Append, etc)

[paulmeyer, 2016-01-28 14:28:09]

LGTM for GuestView changes.

[vmpstr, 2016-01-28 19:53:34]

vmpstr@chromium.org changed reviewers:
+ vmpstr@chromium.org

[vmpstr, 2016-01-28 19:53:34]

https://codereview.chromium.org/1641563002/diff/60001/components/guest_view/b...
File components/guest_view/browser/guest_view_base.cc (left):

https://codereview.chromium.org/1641563002/diff/60001/components/guest_view/b...
components/guest_view/browser/guest_view_base.cc:774:
event_ptr.release()->Dispatch(this, view_instance_id_);
Just out of curiosity, was this leaking before? My linked_ptr knowledge is poor,
but it seems release would stop managing the memory?

[vmpstr, 2016-01-28 19:53:45]

Description was changed from

==========
Remove linked_ptr usage in //base.

And clean up some code that didn't IWYU.

BUG=556939
==========

to

==========
Remove linked_ptr usage in //base.

And clean up some code that didn't IWYU.

BUG=556939
==========

[vmpstr, 2016-01-28 19:53:45]

vmpstr@chromium.org changed reviewers:
- vmpstr@chromium.org

[dcheng, 2016-01-28 20:32:13]

dcheng@chromium.org changed reviewers:
+ thestig@chromium.org

[dcheng, 2016-01-28 20:32:14]

+thestig for //chrome/browser/printing changes.

https://codereview.chromium.org/1641563002/diff/60001/components/guest_view/b...
File components/guest_view/browser/guest_view_base.cc (left):

https://codereview.chromium.org/1641563002/diff/60001/components/guest_view/b...
components/guest_view/browser/guest_view_base.cc:774:
event_ptr.release()->Dispatch(this, view_instance_id_);
On 2016/01/28 at 19:53:34, vmpstr wrote:
> Just out of curiosity, was this leaking before? My linked_ptr knowledge is
poor, but it seems release would stop managing the memory?

I believe you are correct: the original code leaked.

[Lei Zhang, 2016-01-28 22:23:21]

On 2016/01/28 20:32:14, dcheng wrote:
> +thestig for //chrome/browser/printing changes.

lgtm

[dcheng, 2016-01-28 22:31:09]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2016-01-28 22:33:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1641563002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1641563002/60001

[commit-bot: I haz the power, 2016-01-28 23:26:38]

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

[commit-bot: I haz the power, 2016-01-28 23:26:39]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Fady Samuel, 2016-01-28 23:29:33]

fsamuel@chromium.org changed reviewers:
+ fsamuel@chromium.org

[Fady Samuel, 2016-01-28 23:29:34]

https://codereview.chromium.org/1641563002/diff/60001/components/guest_view/b...
File components/guest_view/browser/guest_view_base.cc (left):

https://codereview.chromium.org/1641563002/diff/60001/components/guest_view/b...
components/guest_view/browser/guest_view_base.cc:774:
event_ptr.release()->Dispatch(this, view_instance_id_);
On 2016/01/28 20:32:14, dcheng wrote:
> On 2016/01/28 at 19:53:34, vmpstr wrote:
> > Just out of curiosity, was this leaking before? My linked_ptr knowledge is
> poor, but it seems release would stop managing the memory?
> 
> I believe you are correct: the original code leaked.

No, it wasn't. the event self-destructs on dispatch. In fact, this might be
broken now.

[Fady Samuel, 2016-01-28 23:30:41]

On 2016/01/28 23:29:34, Fady Samuel wrote:
>
https://codereview.chromium.org/1641563002/diff/60001/components/guest_view/b...
> File components/guest_view/browser/guest_view_base.cc (left):
> 
>
https://codereview.chromium.org/1641563002/diff/60001/components/guest_view/b...
> components/guest_view/browser/guest_view_base.cc:774:
> event_ptr.release()->Dispatch(this, view_instance_id_);
> On 2016/01/28 20:32:14, dcheng wrote:
> > On 2016/01/28 at 19:53:34, vmpstr wrote:
> > > Just out of curiosity, was this leaking before? My linked_ptr knowledge is
> > poor, but it seems release would stop managing the memory?
> > 
> > I believe you are correct: the original code leaked.
> 
> No, it wasn't. the event self-destructs on dispatch. In fact, this might be
> broken now.

GuestViewEvent really ought to have a private destructor. This is my bad.

[dcheng, 2016-01-29 01:10:07]

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

[dcheng, 2016-01-29 01:10:37]

fsamuel: thanks for the tips. I've changed the code to use scoped_ptr in more
places, and removed the self-delete in Dispatch().

PTAL at the guest_view changes (this kind of makes me wish we had MakeUnique...)

[commit-bot: I haz the power, 2016-01-29 01:10:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1641563002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1641563002/80001

[Fady Samuel, 2016-01-29 01:26:14]

guest_view lgtm.

[commit-bot: I haz the power, 2016-01-29 02:01:41]

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

[commit-bot: I haz the power, 2016-01-29 02:01:42]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2016-02-02 00:53:29]

PTAL at the changes in
//extensions/browser/guest_view/web_view/web_view_find_helper.*

I ended up giving up on having unique ownership of FindInfo and just made it
RefCounted (since that's what the current semantics are).

[Fady Samuel, 2016-02-02 00:55:19]

guest_view lgtm

[dcheng, 2016-02-02 07:53:29]

The CQ bit was checked by dcheng@chromium.org

[dcheng, 2016-02-02 07:53:29]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
thestig@chromium.org, paulmeyer@chromium.org, fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/1641563002/#ps140001
(title: "Rebase correctly")

[commit-bot: I haz the power, 2016-02-02 07:54:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1641563002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1641563002/140001

[commit-bot: I haz the power, 2016-02-02 08:52:25]

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

[commit-bot: I haz the power, 2016-02-02 08:52:26]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2016-02-02 17:31:24]

The CQ bit was checked by dcheng@chromium.org

[dcheng, 2016-02-02 17:31:25]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
fsamuel@chromium.org, paulmeyer@chromium.org, thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/1641563002/#ps160001
(title: "Fix rebase, really")

[commit-bot: I haz the power, 2016-02-02 17:31:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1641563002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1641563002/160001

[commit-bot: I haz the power, 2016-02-02 17:42:35]

Description was changed from

==========
Remove linked_ptr usage in //base.

And clean up some code that didn't IWYU.

BUG=556939
==========

to

==========
Remove linked_ptr usage in //base.

And clean up some code that didn't IWYU.

BUG=556939
==========

[commit-bot: I haz the power, 2016-02-02 17:42:36]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-02-02 17:44:29]

Description was changed from

==========
Remove linked_ptr usage in //base.

And clean up some code that didn't IWYU.

BUG=556939
==========

to

==========
Remove linked_ptr usage in //base.

And clean up some code that didn't IWYU.

BUG=556939

Committed: https://crrev.com/2588941279b28a6cf205896b955402e36535af6d
Cr-Commit-Position: refs/heads/master@{#372977}
==========

[commit-bot: I haz the power, 2016-02-02 17:44:31]

Patchset 9 (id:??) landed as
https://crrev.com/2588941279b28a6cf205896b955402e36535af6d
Cr-Commit-Position: refs/heads/master@{#372977}