Oilpan: tidy up CSSCustomFontData::m_fontFaceSource back ref somewhat.

Oilpan: tidy up CSSCustomFontData::m_fontFaceSource back ref somewhat.

R=haraken
BUG=553613
Committed: https://crrev.com/6967287223ccbe29db1454e6ace0d412524aac25
Cr-Commit-Position: refs/heads/master@{#360062}

[sof, 2015-11-12 10:22:45]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[haraken, 2015-11-12 10:36:14]

Would it be hard to move CustomFontData to the heap?

[sof, 2015-11-12 10:38:19]

On 2015/11/12 10:36:14, haraken wrote:
> Would it be hard to move CustomFontData to the heap?

I'm not doing that here, i.e., move the FontData hierarchy to the heap.

[haraken, 2015-11-12 14:27:26]

On 2015/11/12 10:38:19, sof wrote:
> On 2015/11/12 10:36:14, haraken wrote:
> > Would it be hard to move CustomFontData to the heap?
> 
> I'm not doing that here, i.e., move the FontData hierarchy to the heap.

Then WeakPersistent + TODO?

[sof, 2015-11-17 10:52:45]

On 2015/11/12 14:27:26, haraken wrote:
> On 2015/11/12 10:38:19, sof wrote:
> > On 2015/11/12 10:36:14, haraken wrote:
> > > Would it be hard to move CustomFontData to the heap?
> > 
> > I'm not doing that here, i.e., move the FontData hierarchy to the heap.
> 
> Then WeakPersistent + TODO?

Now done.

Just moving CustomFontData to the heap runs the risk of creating a leak
involving Persistent<>s, which is why I'm hesitant to move those font data
objects only to the heap.

As CSSCustomFontData::m_fontFaceSource is already considered a weak reference by
the implementation, encoding that in its Oilpan type seems alright -- the
non-nullptr lifetime is limited, so we won't keep a bunch of extra (weak)
persistents around for very long.

[haraken, 2015-11-17 11:22:29]

On 2015/11/17 10:52:45, sof wrote:
> On 2015/11/12 14:27:26, haraken wrote:
> > On 2015/11/12 10:38:19, sof wrote:
> > > On 2015/11/12 10:36:14, haraken wrote:
> > > > Would it be hard to move CustomFontData to the heap?
> > > 
> > > I'm not doing that here, i.e., move the FontData hierarchy to the heap.
> > 
> > Then WeakPersistent + TODO?
> 
> Now done.
> 
> Just moving CustomFontData to the heap runs the risk of creating a leak
> involving Persistent<>s, which is why I'm hesitant to move those font data
> objects only to the heap.
> 
> As CSSCustomFontData::m_fontFaceSource is already considered a weak reference
by
> the implementation, encoding that in its Oilpan type seems alright -- the
> non-nullptr lifetime is limited, so we won't keep a bunch of extra (weak)
> persistents around for very long.

Thanks for the investigation. LGTM.

[sof, 2015-11-17 11:47:05]

Description was changed from

==========
Oilpan: tidy up CSSCustomFontData::m_fontFaceSource back ref somewhat.

R=
BUG=553613
==========

to

==========
Oilpan: tidy up CSSCustomFontData::m_fontFaceSource back ref somewhat.

R=haraken
BUG=553613
==========

[sof, 2015-11-17 11:47:08]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-11-17 11:47:27]

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

[commit-bot: I haz the power, 2015-11-17 12:04:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-17 12:04:52]

Failed to apply the patch.

[commit-bot: I haz the power, 2015-11-17 12:05:32]

Patchset 2 (id:??) landed as
https://crrev.com/6967287223ccbe29db1454e6ace0d412524aac25
Cr-Commit-Position: refs/heads/master@{#360062}