Protect contextElement during insertAdjacentHTML call

Protect contextElement during insertAdjacentHTML call

JS event handlers may cause element to lose its last ref during
parsing.

BUG=315842
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161697

[davve, 2013-11-09 13:14:34]

Better fix to https://code.google.com/p/chromium/issues/detail?id=315842 than
https://codereview.chromium.org/67773002/ I hope. I haven't been able verify it,
since I can't reproduce 315842 locally (yet), but this seems like a reasonable
fix to me.

[davve, 2013-11-09 15:22:10]

On 2013/11/09 13:14:34, David Vest wrote:
> I haven't been able verify it, since I can't reproduce 315842 locally (yet),
but this seems like a reasonable
> fix to me.

I've now verified this fix to be effective (after reading about
--js-flags=--expose_gc).

[inferno, 2013-11-10 09:10:46]

https://codereview.chromium.org/67813002/diff/1/Source/core/html/HTMLElement.cpp
File Source/core/html/HTMLElement.cpp (right):

https://codereview.chromium.org/67813002/diff/1/Source/core/html/HTMLElement....
Source/core/html/HTMLElement.cpp:530: RefPtr<Element> protect = contextElement;
Please define the contextElement as RefPtr itself, no need of this line then.

Were you able to minimize the test ?

[inferno, 2013-11-10 09:11:55]

On 2013/11/10 09:10:46, inferno wrote:
>
https://codereview.chromium.org/67813002/diff/1/Source/core/html/HTMLElement.cpp
> File Source/core/html/HTMLElement.cpp (right):
> 
>
https://codereview.chromium.org/67813002/diff/1/Source/core/html/HTMLElement....
> Source/core/html/HTMLElement.cpp:530: RefPtr<Element> protect =
contextElement;
> Please define the contextElement as RefPtr itself, no need of this line then.
> 
> Were you able to minimize the test ?

There is already minimized test in
https://code.google.com/p/chromium/issues/detail?id=315842#c5. Please include a
layout test. gc() is already exposed in content shell

[davve, 2013-11-10 20:02:05]

On 2013/11/10 09:11:55, inferno wrote:
> On 2013/11/10 09:10:46, inferno wrote:
> >
>
https://codereview.chromium.org/67813002/diff/1/Source/core/html/HTMLElement.cpp
> > File Source/core/html/HTMLElement.cpp (right):
> > 
> >
>
https://codereview.chromium.org/67813002/diff/1/Source/core/html/HTMLElement....
> > Source/core/html/HTMLElement.cpp:530: RefPtr<Element> protect =
> contextElement;
> > Please define the contextElement as RefPtr itself, no need of this line
then.
> > 
> > Were you able to minimize the test ?
> 
> There is already minimized test in
> https://code.google.com/p/chromium/issues/detail?id=315842#c5. Please include
a
> layout test. gc() is already exposed in content shell

Thanks! I've included a crash test now.

[davve, 2013-11-10 20:02:13]

https://codereview.chromium.org/67813002/diff/1/Source/core/html/HTMLElement.cpp
File Source/core/html/HTMLElement.cpp (right):

https://codereview.chromium.org/67813002/diff/1/Source/core/html/HTMLElement....
Source/core/html/HTMLElement.cpp:530: RefPtr<Element> protect = contextElement;
On 2013/11/10 09:10:46, inferno wrote:
> Please define the contextElement as RefPtr itself, no need of this line then.

OK.

[inferno, 2013-11-10 20:16:21]

lgtm

[commit-bot: I haz the power, 2013-11-10 20:16:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davve@opera.com/67813002/70001

[commit-bot: I haz the power, 2013-11-10 21:07:32]

Change committed as 161697