Remove all uses of skia::RefPtr and stale includes

Remove all uses of skia::RefPtr and stale includes

This CL is the content/ and extensions/-specific sections
of the change.

Finish deprecating skia::RefPtr in favor of sk_sp<>.
This should significantly reduce the amount of manual reference
management Chromium and Blink authors need to do for Skia objects.

We still have some APIs vending raw pointers to ref-counted Skia
objects which will need to be wrapped in sk_sp<>, but that should
be easier and more predictable. This CL does *not* try to remove
all cases of raw pointers to Skia objects in Chromium.

R=fmalita@chromium.org,piman@chromium.org,rockot@chromium.org,mkwst@chromium.org
BUG=skia:5077, 604716
Committed: https://crrev.com/55241b6041af2b175b1183b60f162af2160ee3e7
Cr-Commit-Position: refs/heads/master@{#396458}

[tomhudson, 2016-05-04 15:09:43]

The CQ bit was checked by tomhudson@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-04 15:09:55]

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

[commit-bot: I haz the power, 2016-05-04 16:20:32]

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

[commit-bot: I haz the power, 2016-05-04 16:20:33]

Dry run: This issue passed the CQ dry run.

[tomhudson, 2016-05-04 16:46:23]

fmalita@: Skia reviewer.
piman@: please review content/.
rockot@: please review extensions/.
mkwst@: please security-review content/common/.

[f(malita), 2016-05-04 17:22:14]

https://codereview.chromium.org/1951013003/diff/1/content/child/font_warmup_w...
File content/child/font_warmup_win.cc (right):

https://codereview.chromium.org/1951013003/diff/1/content/child/font_warmup_w...
content/child/font_warmup_win.cc:228: ret->set_typeface(typeface);
std::move(typeface)

https://codereview.chromium.org/1951013003/diff/1/content/child/font_warmup_w...
content/child/font_warmup_win.cc:318: new_font_obj->set_typeface(old_typeface);
std::move(old_typeface)

https://codereview.chromium.org/1951013003/diff/1/content/common/font_config_...
File content/common/font_config_ipc_linux.cc (right):

https://codereview.chromium.org/1951013003/diff/1/content/common/font_config_...
content/common/font_config_ipc_linux.cc:132: return new SkMemoryStream(data);
std::move(data)

https://codereview.chromium.org/1951013003/diff/1/content/common/font_config_...
content/common/font_config_ipc_linux.cc:175: return
mapped_typefaces_insert_it->second.release();
I'm not familiar with HashingMRUCache, but this looks a bit iffy (is the
iterator pointing to the just-inserted value and we're stealing it?).

return sk_sp<>(mapped_typefaces_insert_it->second) to preserve existing
semantics and stay on the safe side?

[tomhudson, 2016-05-04 17:47:10]

https://codereview.chromium.org/1951013003/diff/1/content/child/font_warmup_w...
File content/child/font_warmup_win.cc (right):

https://codereview.chromium.org/1951013003/diff/1/content/child/font_warmup_w...
content/child/font_warmup_win.cc:228: ret->set_typeface(typeface);
On 2016/05/04 17:22:13, f(malita) wrote:
> std::move(typeface)

Done.

https://codereview.chromium.org/1951013003/diff/1/content/child/font_warmup_w...
content/child/font_warmup_win.cc:318: new_font_obj->set_typeface(old_typeface);
On 2016/05/04 17:22:13, f(malita) wrote:
> std::move(old_typeface)

Done.

https://codereview.chromium.org/1951013003/diff/1/content/common/font_config_...
File content/common/font_config_ipc_linux.cc (right):

https://codereview.chromium.org/1951013003/diff/1/content/common/font_config_...
content/common/font_config_ipc_linux.cc:132: return new SkMemoryStream(data);
On 2016/05/04 17:22:13, f(malita) wrote:
> std::move(data)

Done.

https://codereview.chromium.org/1951013003/diff/1/content/common/font_config_...
content/common/font_config_ipc_linux.cc:175: return
mapped_typefaces_insert_it->second.release();
On 2016/05/04 17:22:13, f(malita) wrote:
> I'm not familiar with HashingMRUCache, but this looks a bit iffy (is the
> iterator pointing to the just-inserted value and we're stealing it?).
> 
> return sk_sp<>(mapped_typefaces_insert_it->second) to preserve existing
> semantics and stay on the safe side?

I'm not happy with this either, but it's inheriting from a base class that
hasn't been updated to return sk_sp<> yet. If we want that, we can put this CL
on hold until that one is finished.

[f(malita), 2016-05-04 18:12:22]

https://codereview.chromium.org/1951013003/diff/1/content/common/font_config_...
File content/common/font_config_ipc_linux.cc (right):

https://codereview.chromium.org/1951013003/diff/1/content/common/font_config_...
content/common/font_config_ipc_linux.cc:175: return
mapped_typefaces_insert_it->second.release();
On 2016/05/04 17:47:10, tomhudson wrote:
> On 2016/05/04 17:22:13, f(malita) wrote:
> > I'm not familiar with HashingMRUCache, but this looks a bit iffy (is the
> > iterator pointing to the just-inserted value and we're stealing it?).
> > 
> > return sk_sp<>(mapped_typefaces_insert_it->second) to preserve existing
> > semantics and stay on the safe side?
> 
> I'm not happy with this either, but it's inheriting from a base class that
> hasn't been updated to return sk_sp<> yet. If we want that, we can put this CL
> on hold until that one is finished.

Would certainly be nice to return a smart ref from this method, but you're right
- looks like that'll take some Skia work.

I think it's fine to land this CL as long as we're sticking to the existing
semantics: don't steal the insert iter ref but return a new one (same as
before).

  return SkSafeRef(mapped_typefaces_insert_it->second.get());

[piman, 2016-05-04 21:29:59]

lgtm

[Mike West, 2016-05-09 18:49:11]

LGTM.

[tomhudson, 2016-05-26 20:33:16]

rockot@: ping?
fmalita@: sorry, missed that you wanted one more change. Is this good?

https://codereview.chromium.org/1951013003/diff/1/content/common/font_config_...
File content/common/font_config_ipc_linux.cc (right):

https://codereview.chromium.org/1951013003/diff/1/content/common/font_config_...
content/common/font_config_ipc_linux.cc:175: return
mapped_typefaces_insert_it->second.release();
On 2016/05/04 18:12:22, f(malita) wrote:
> On 2016/05/04 17:47:10, tomhudson wrote:
> > On 2016/05/04 17:22:13, f(malita) wrote:
> > > I'm not familiar with HashingMRUCache, but this looks a bit iffy (is the
> > > iterator pointing to the just-inserted value and we're stealing it?).
> > > 
> > > return sk_sp<>(mapped_typefaces_insert_it->second) to preserve existing
> > > semantics and stay on the safe side?
> > 
> > I'm not happy with this either, but it's inheriting from a base class that
> > hasn't been updated to return sk_sp<> yet. If we want that, we can put this
CL
> > on hold until that one is finished.
> 
> Would certainly be nice to return a smart ref from this method, but you're
right
> - looks like that'll take some Skia work.
> 
> I think it's fine to land this CL as long as we're sticking to the existing
> semantics: don't steal the insert iter ref but return a new one (same as
> before).
> 
>   return SkSafeRef(mapped_typefaces_insert_it->second.get());

Done.

[Ken Rockot(use gerrit already), 2016-05-26 20:35:21]

sorry I missed this earlier. lgtm

[f(malita), 2016-05-27 01:09:37]

On 2016/05/26 20:33:16, tomhudson wrote:
> fmalita@: sorry, missed that you wanted one more change. Is this good?

Thanks, LGTM.

[tomhudson, 2016-05-27 12:37:38]

The CQ bit was checked by tomhudson@google.com

[tomhudson, 2016-05-27 12:37:39]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org,
mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1951013003/#ps40001
(title: "Florin's smart pointer fix")

[commit-bot: I haz the power, 2016-05-27 12:37:54]

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

[commit-bot: I haz the power, 2016-05-27 14:03:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-27 14:05:48]

Description was changed from

==========
Remove all uses of skia::RefPtr and stale includes

This CL is the content/ and extensions/-specific sections
of the change.

Finish deprecating skia::RefPtr in favor of sk_sp<>.
This should significantly reduce the amount of manual reference
management Chromium and Blink authors need to do for Skia objects.

We still have some APIs vending raw pointers to ref-counted Skia
objects which will need to be wrapped in sk_sp<>, but that should
be easier and more predictable. This CL does *not* try to remove
all cases of raw pointers to Skia objects in Chromium.

R=fmalita@chromium.org,piman@chromium.org,rockot@chromium.org,mkwst@chromium.org
BUG=skia:5077,604716
==========

to

==========
Remove all uses of skia::RefPtr and stale includes

This CL is the content/ and extensions/-specific sections
of the change.

Finish deprecating skia::RefPtr in favor of sk_sp<>.
This should significantly reduce the amount of manual reference
management Chromium and Blink authors need to do for Skia objects.

We still have some APIs vending raw pointers to ref-counted Skia
objects which will need to be wrapped in sk_sp<>, but that should
be easier and more predictable. This CL does *not* try to remove
all cases of raw pointers to Skia objects in Chromium.

R=fmalita@chromium.org,piman@chromium.org,rockot@chromium.org,mkwst@chromium.org
BUG=skia:5077,604716

Committed: https://crrev.com/55241b6041af2b175b1183b60f162af2160ee3e7
Cr-Commit-Position: refs/heads/master@{#396458}
==========

[commit-bot: I haz the power, 2016-05-27 14:05:51]

Patchset 3 (id:??) landed as
https://crrev.com/55241b6041af2b175b1183b60f162af2160ee3e7
Cr-Commit-Position: refs/heads/master@{#396458}