Pepper: Fix nullptr crash in PepperWebPluginImpl::updateGeometry.

Pepper: Fix nullptr crash in PepperWebPluginImpl::updateGeometry.

Quoting raymes: "What is happening is that while the
plugin is initializing it sends a sync IPC which results in re-entrancy.
ExecuteScript is being run during re-entrancy which runs a script on the page
which can result in layout changing and so reportGeometry can be called."

BUG=486674
Committed: https://crrev.com/5377f4d5f94a4c861d49091ee56b62e15cf89367
Cr-Commit-Position: refs/heads/master@{#329566}

[tommycli, 2015-05-12 20:23:06]

tommycli@chromium.org changed reviewers:
+ raymes@chromium.org

[tommycli, 2015-05-12 20:23:07]

raymes: PTAL. extra small review

[raymes, 2015-05-13 01:05:08]

lgtm

I had a look at the crash in more detail. What is happening is that while the
plugin is initializing it sends a sync IPC which results in re-entrancy.
ExecuteScript is being run during re-entrancy which runs a script on the page
which can result in layout changing and so reportGeometry can be called. So your
fix looks ok.

I look forward to the day when we can remove synchronous scripting!

[raymes, 2015-05-13 01:06:42]

You may want to add the above into your CL description :)

[tommycli, 2015-05-13 01:09:28]

On 2015/05/13 01:05:08, raymes wrote:
> lgtm
> 
> I had a look at the crash in more detail. What is happening is that while the
> plugin is initializing it sends a sync IPC which results in re-entrancy.
> ExecuteScript is being run during re-entrancy which runs a script on the page
> which can result in layout changing and so reportGeometry can be called. So
your
> fix looks ok.
> 
> I look forward to the day when we can remove synchronous scripting!

That is awesome. Never would have guessed. I updated the CL desc with a direct
quote of this.

[tommycli, 2015-05-13 01:11:07]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2015-05-13 01:11:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1137433004/1

[tommycli, 2015-05-13 01:11:46]

On 2015/05/13 01:09:28, tommycli wrote:
> On 2015/05/13 01:05:08, raymes wrote:
> > lgtm
> > 
> > I had a look at the crash in more detail. What is happening is that while
the
> > plugin is initializing it sends a sync IPC which results in re-entrancy.
> > ExecuteScript is being run during re-entrancy which runs a script on the
page
> > which can result in layout changing and so reportGeometry can be called. So
> your
> > fix looks ok.
> > 
> > I look forward to the day when we can remove synchronous scripting!
> 
> That is awesome. Never would have guessed. I updated the CL desc with a direct
> quote of this.

By the way, does your reasoning apply to
https://codereview.chromium.org/1139983002/ also? I made a similar speculative
fix but it sounds like you are much more qualified to diagnose.

[raymes, 2015-05-13 01:26:14]

No that one looks very different. It looks like it's happening on input.
But I don't really have any other interesting insights to share ;(

On Wed, 13 May 2015 at 11:11 <tommycli@chromium.org> wrote:

> On 2015/05/13 01:09:28, tommycli wrote:
> > On 2015/05/13 01:05:08, raymes wrote:
> > > lgtm
> > >
> > > I had a look at the crash in more detail. What is happening is that
> > while
> the
> > > plugin is initializing it sends a sync IPC which results in
> re-entrancy.
> > > ExecuteScript is being run during re-entrancy which runs a script on
> the
> page
> > > which can result in layout changing and so reportGeometry can be
> > called. So
> > your
> > > fix looks ok.
> > >
> > > I look forward to the day when we can remove synchronous scripting!
>
> > That is awesome. Never would have guessed. I updated the CL desc with a
> > direct
> > quote of this.
>
> By the way, does your reasoning apply to
> https://codereview.chromium.org/1139983002/ also? I made a similar
> speculative
> fix but it sounds like you are much more qualified to diagnose.
>
> https://codereview.chromium.org/1137433004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2015-05-13 01:58:21]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-05-13 01:59:07]

Patchset 1 (id:??) landed as
https://crrev.com/5377f4d5f94a4c861d49091ee56b62e15cf89367
Cr-Commit-Position: refs/heads/master@{#329566}