Don't allow dirtying layout during paint of a plugin placeholder.

Don't allow dirtying layout during paint of a plugin placeholder.

This should not happen anyway, but adding defensive checks may uncover code
which does it anyway.

BUG=545039
Committed: https://crrev.com/66d3b34a3c9a2d6834cb914e0f841de4bd5aed58
Cr-Commit-Position: refs/heads/master@{#374246}

[chrishtr, 2016-02-08 22:14:22]

Description was changed from

==========
none

BUG=
==========

to

==========
Don't allow dirtying layout during paint of a plugin placeholder.

This should not happen anyway, but adding defensive checks may uncover code
which does it anyway.

BUG=545039
==========

[chrishtr, 2016-02-08 22:16:55]

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

[tommycli, 2016-02-08 22:20:43]

https://codereview.chromium.org/1677063004/diff/1/components/plugins/renderer...
File components/plugins/renderer/webview_plugin.cc (right):

https://codereview.chromium.org/1677063004/diff/1/components/plugins/renderer...
components/plugins/renderer/webview_plugin.cc:176: is_painting_ = true;
Seems like this should use base::AutoReset

https://codereview.chromium.org/1677063004/diff/1/components/plugins/renderer...
components/plugins/renderer/webview_plugin.cc:295: if (container_ &&
!is_painting_)
If this should not happen anyways, shouldn't this be a CHECK?

[chrishtr, 2016-02-08 22:43:39]

https://codereview.chromium.org/1677063004/diff/1/components/plugins/renderer...
File components/plugins/renderer/webview_plugin.cc (right):

https://codereview.chromium.org/1677063004/diff/1/components/plugins/renderer...
components/plugins/renderer/webview_plugin.cc:176: is_painting_ = true;
On 2016/02/08 at 22:20:43, tommycli wrote:
> Seems like this should use base::AutoReset

Done.

https://codereview.chromium.org/1677063004/diff/1/components/plugins/renderer...
components/plugins/renderer/webview_plugin.cc:295: if (container_ &&
!is_painting_)
On 2016/02/08 at 22:20:43, tommycli wrote:
> If this should not happen anyways, shouldn't this be a CHECK?

Added DCHECK plus defensive logic.

[tommycli, 2016-02-08 23:09:04]

lgtm! thanks for investigating this chris

https://codereview.chromium.org/1677063004/diff/40001/components/plugins/rend...
File components/plugins/renderer/webview_plugin.cc (right):

https://codereview.chromium.org/1677063004/diff/40001/components/plugins/rend...
components/plugins/renderer/webview_plugin.cc:297: DCHECK(!is_painting_);
Don't only CHECKs show up in crashes? Not sure, just asking.

[chrishtr, 2016-02-08 23:13:56]

https://codereview.chromium.org/1677063004/diff/40001/components/plugins/rend...
File components/plugins/renderer/webview_plugin.cc (right):

https://codereview.chromium.org/1677063004/diff/40001/components/plugins/rend...
components/plugins/renderer/webview_plugin.cc:297: DCHECK(!is_painting_);
On 2016/02/08 at 23:09:04, tommycli wrote:
> Don't only CHECKs show up in crashes? Not sure, just asking.

CHECK will crash in production, DCHECK in debug builds. Another option is to
upgrade
to a CHECK...hmm. Updated to be CHECK instead to see if we can get it to crash
on canary.

[chrishtr, 2016-02-08 23:14:01]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-02-08 23:14:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org
Link to the patchset: https://codereview.chromium.org/1677063004/#ps60001
(title: " ")

[commit-bot: I haz the power, 2016-02-08 23:16:11]

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

[commit-bot: I haz the power, 2016-02-09 00:57:29]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-02-09 00:58:38]

Description was changed from

==========
Don't allow dirtying layout during paint of a plugin placeholder.

This should not happen anyway, but adding defensive checks may uncover code
which does it anyway.

BUG=545039
==========

to

==========
Don't allow dirtying layout during paint of a plugin placeholder.

This should not happen anyway, but adding defensive checks may uncover code
which does it anyway.

BUG=545039

Committed: https://crrev.com/66d3b34a3c9a2d6834cb914e0f841de4bd5aed58
Cr-Commit-Position: refs/heads/master@{#374246}
==========

[commit-bot: I haz the power, 2016-02-09 00:58:39]

Patchset 4 (id:??) landed as
https://crrev.com/66d3b34a3c9a2d6834cb914e0f841de4bd5aed58
Cr-Commit-Position: refs/heads/master@{#374246}