Move PlatformSpeechSynthesisVoice off the heap.

Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity:
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account.

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can reside just as well. By doing so,
WebSpeechSynthesisVoice allocations won't allocate on the Oilpan
heap, avoid said GC unsafety.

R=dmazzoni,jochen
BUG=539511
Committed: https://crrev.com/3ebb67d8abcd3d39dc4d67947dfc6a586583fc7e
Cr-Commit-Position: refs/heads/master@{#370960}

[sof, 2016-01-22 12:07:25]

Description was changed from

==========
Move PlatformSpeechSynthesisVoice off the heap.

R=
BUG=539511
==========

to

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details. This is normally a detail
the embedder doesn't need to worry about, but as TtsDispatcher keeps
an (unsavory) weak reference to its speech synthesizer client is a
direct factor in making this unsafe -- this is an uncommon arrangement
in general and something we generally avoid (or try to.)

But to address this weakness, PlatformSpeechSynthesisVoice is
moved off the heap.

R=
BUG=539511
==========

[sof, 2016-01-22 12:13:58]

Description was changed from

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details. This is normally a detail
the embedder doesn't need to worry about, but as TtsDispatcher keeps
an (unsavory) weak reference to its speech synthesizer client is a
direct factor in making this unsafe -- this is an uncommon arrangement
in general and something we generally avoid (or try to.)

But to address this weakness, PlatformSpeechSynthesisVoice is
moved off the heap.

R=
BUG=539511
==========

to

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account. 

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can live just as well.

R=
BUG=539511
==========

[sof, 2016-01-22 12:14:08]

Description was changed from

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account. 

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can live just as well.

R=
BUG=539511
==========

to

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account. 

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can reside just as well.

R=
BUG=539511
==========

[sof, 2016-01-22 12:17:07]

sigbjornf@opera.com changed reviewers:
+ dmazzoni@chromium.org, haraken@chromium.org, tkent@chromium.org

[sof, 2016-01-22 12:17:07]

please take a look.

TtsDispatcher::OnSetVoiceList() is the only problematic method;
WebSpeechSynthesisUtterance usage in its other methods will not bring about heap
allocations.

[dmazzoni, 2016-01-22 14:04:18]

lgtm

Thanks, this makes sense. It sounds like a better solution would be for
TtsDispatcher to have a true weak reference to WebSpeechSynthesizerClient.

Would it work if WebSpeechSynthesizerClient is a wrapper class where the
WebSpeechSynthesizerClientImpl is a weak member?

[sof, 2016-01-22 14:10:43]

On 2016/01/22 14:04:18, dmazzoni wrote:
> lgtm
> 
> Thanks, this makes sense. It sounds like a better solution would be for
> TtsDispatcher to have a true weak reference to WebSpeechSynthesizerClient.
> 
> Would it work if WebSpeechSynthesizerClient is a wrapper class where the
> WebSpeechSynthesizerClientImpl is a weak member?

thanks; not having thought this through, but a WebPrivatePtr<> which allowed a
WeakPersistent<> to be kept/wrapped, might be a way to do it. That
WebSpeechSynthesizerClient layer would then have to check & delegate.

[sof, 2016-01-22 14:14:22]

sigbjornf@opera.com changed reviewers:
+ jochen@chromium.org

[sof, 2016-01-22 14:14:22]

Need Source/platform/ approval to land this - jochen@ wouldn't happen to be
around..? :)

[sof, 2016-01-22 14:17:56]

Description was changed from

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account. 

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can reside just as well.

R=
BUG=539511
==========

to

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account. 

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can reside just as well. By doing so,
WebSpeechSynthesisVoice allocations won't allocate on the Oilpan
heap, avoid said GC unsafety.

R=
BUG=539511
==========

[jochen (gone - plz use gerrit), 2016-01-22 14:18:37]

lgtm

[sof, 2016-01-22 14:19:24]

Description was changed from

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account. 

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can reside just as well. By doing so,
WebSpeechSynthesisVoice allocations won't allocate on the Oilpan
heap, avoid said GC unsafety.

R=
BUG=539511
==========

to

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account. 

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can reside just as well. By doing so,
WebSpeechSynthesisVoice allocations won't allocate on the Oilpan
heap, avoid said GC unsafety.

R=dmazzoni,jochen
BUG=539511
==========

[sof, 2016-01-22 14:19:45]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2016-01-22 14:20:06]

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

[commit-bot: I haz the power, 2016-01-22 14:24:32]

Description was changed from

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account. 

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can reside just as well. By doing so,
WebSpeechSynthesisVoice allocations won't allocate on the Oilpan
heap, avoid said GC unsafety.

R=dmazzoni,jochen
BUG=539511
==========

to

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account. 

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can reside just as well. By doing so,
WebSpeechSynthesisVoice allocations won't allocate on the Oilpan
heap, avoid said GC unsafety.

R=dmazzoni,jochen
BUG=539511
==========

[commit-bot: I haz the power, 2016-01-22 14:24:33]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-22 14:25:32]

Description was changed from

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity: 
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account. 

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can reside just as well. By doing so,
WebSpeechSynthesisVoice allocations won't allocate on the Oilpan
heap, avoid said GC unsafety.

R=dmazzoni,jochen
BUG=539511
==========

to

==========
Keep PlatformSpeechSynthesisVoice off the Oilpan heap.

Keeping this object on the heap makes some sense in terms of regularity:
all the other Blink objects that refer to it are on the heap. However,
it is problematic to do so for this value object considering how
the embedder might use its WebSpeechSynthesisVoice wrapper object.

That is, creating or allocating a WebSpeechSynthesisVoice on the stack
by the embedder will bring about a heap allocation, which in turn can
trigger a GC when the embedder isn't prepared for that -- see
associated bug for stack trace and details.

This is normally a detail the embedder doesn't need to worry about,
but as TtsDispatcher keeps an (unsavory) weak reference to its speech
synthesizer client we're forced to consider GC safety and take
that into account.

Embedder code that keep these bare, but intended weak, references
to Oilpan heap objects should be reworked into something safer,
but to address this local problem, PlatformSpeechSynthesisVoice is
moved off the heap where it can reside just as well. By doing so,
WebSpeechSynthesisVoice allocations won't allocate on the Oilpan
heap, avoid said GC unsafety.

R=dmazzoni,jochen
BUG=539511

Committed: https://crrev.com/3ebb67d8abcd3d39dc4d67947dfc6a586583fc7e
Cr-Commit-Position: refs/heads/master@{#370960}
==========

[commit-bot: I haz the power, 2016-01-22 14:25:32]

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

[haraken, 2016-01-22 15:24:52]

LGTM

[sof, 2016-01-25 22:04:47]

On 2016/01/22 14:10:43, sof wrote:
> On 2016/01/22 14:04:18, dmazzoni wrote:
> > lgtm
> > 
> > Thanks, this makes sense. It sounds like a better solution would be for
> > TtsDispatcher to have a true weak reference to WebSpeechSynthesizerClient.
> > 
> > Would it work if WebSpeechSynthesizerClient is a wrapper class where the
> > WebSpeechSynthesizerClientImpl is a weak member?
> 
> thanks; not having thought this through, but a WebPrivatePtr<> which allowed a
> WeakPersistent<> to be kept/wrapped, might be a way to do it. That
> WebSpeechSynthesizerClient layer would then have to check & delegate.

https://codereview.chromium.org/1618043003/ extends WebPrivatePtr<> to support
weak persistent references.