Only clears the unblock flag on sync IPCs during other sync dispatches

Only clears the unblock flag on sync IPCs during other sync dispatches

BUG=22210
(this is not a partial mitigation, not a fix)

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=32397

[jam, 2009-11-17 21:09:00]

nice find :)

http://codereview.chromium.org/399043/diff/1/3
File chrome/plugin/plugin_channel_base.cc (right):

http://codereview.chromium.org/399043/diff/1/3#newcode134
Line 134: if (message.is_sync())
can't use message here since it's already deleted, so just store a boolean

http://codereview.chromium.org/399043/diff/1/3#newcode219
Line 219: void PluginChannelBase::SendUnblockingOnlyDuringSyncDispatch() {
since you're changing this, mind moving it to the header, where it should have
been?  it should be set_send_unblocking_only_during_sync_dispatch().

[jam, 2009-11-17 21:11:13]

lgtm

http://codereview.chromium.org/399043/diff/1/3
File chrome/plugin/plugin_channel_base.cc (right):

http://codereview.chromium.org/399043/diff/1/3#newcode134
Line 134: if (message.is_sync())
On 2009/11/17 21:09:01, John Abd-El-Malek wrote:
> can't use message here since it's already deleted, so just store a boolean

argh, i didn't mean that.  nvm!!

[jamesr, 2009-11-17 21:37:37]

On 2009/11/17 21:11:13, John Abd-El-Malek wrote:
> lgtm
> 
> http://codereview.chromium.org/399043/diff/1/3
> File chrome/plugin/plugin_channel_base.cc (right):
> 
> http://codereview.chromium.org/399043/diff/1/3#newcode134
> Line 134: if (message.is_sync())
> On 2009/11/17 21:09:01, John Abd-El-Malek wrote:
> > can't use message here since it's already deleted, so just store a boolean
> 
> argh, i didn't mean that.  nvm!!

Running ui_tests locally I see these failures:

[  FAILED  ] NPAPITester.SelfDeletePluginGetUrl
[  FAILED  ] NPAPITester.SelfDeletePluginInvoke
[  FAILED  ] TabRestoreUITest.RestoreIntoSameWindow

They seem real, going to try to figure them out before landing.

[jam, 2009-11-17 21:39:52]

On Tue, Nov 17, 2009 at 1:37 PM, <jamesr@chromium.org> wrote:

> On 2009/11/17 21:11:13, John Abd-El-Malek wrote:
>
>> lgtm
>>
>
>  http://codereview.chromium.org/399043/diff/1/3
>> File chrome/plugin/plugin_channel_base.cc (right):
>>
>
>  http://codereview.chromium.org/399043/diff/1/3#newcode134
>> Line 134: if (message.is_sync())
>> On 2009/11/17 21:09:01, John Abd-El-Malek wrote:
>> > can't use message here since it's already deleted, so just store a
>> boolean
>>
>
>  argh, i didn't mean that.  nvm!!
>>
>
> Running ui_tests locally I see these failures:
>
> [  FAILED  ] NPAPITester.SelfDeletePluginGetUrl
> [  FAILED  ] NPAPITester.SelfDeletePluginInvoke
>

these are probably because you don't the plugins built.  ui_tests should
have a dependency but it's missing (i think it used to exist, but got lost
in the gyp switch).  try rebuilding webkit\tools\*_test_plugin


> [  FAILED  ] TabRestoreUITest.RestoreIntoSameWindow
>
> They seem real, going to try to figure them out before landing.
>
>
> http://codereview.chromium.org/399043
>

[jamesr, 2009-11-17 21:55:16]

On 2009/11/17 21:39:52, John Abd-El-Malek wrote:
> On Tue, Nov 17, 2009 at 1:37 PM, <mailto:jamesr@chromium.org> wrote:
> 
> > On 2009/11/17 21:11:13, John Abd-El-Malek wrote:
> >
> >> lgtm
> >>
> >
> >  http://codereview.chromium.org/399043/diff/1/3
> >> File chrome/plugin/plugin_channel_base.cc (right):
> >>
> >
> >  http://codereview.chromium.org/399043/diff/1/3#newcode134
> >> Line 134: if (message.is_sync())
> >> On 2009/11/17 21:09:01, John Abd-El-Malek wrote:
> >> > can't use message here since it's already deleted, so just store a
> >> boolean
> >>
> >
> >  argh, i didn't mean that.  nvm!!
> >>
> >
> > Running ui_tests locally I see these failures:
> >
> > [  FAILED  ] NPAPITester.SelfDeletePluginGetUrl
> > [  FAILED  ] NPAPITester.SelfDeletePluginInvoke
> >
> 
> these are probably because you don't the plugins built.  ui_tests should
> have a dependency but it's missing (i think it used to exist, but got lost
> in the gyp switch).  try rebuilding webkit\tools\*_test_plugin

Yup, that fixed it.
> 
> 
> > [  FAILED  ] TabRestoreUITest.RestoreIntoSameWindow

This one is a definite failure (it's a crash in the renderer process).  Not 100%
reproducible and possibly not due to this patch but it's a real failure.
> >
> > They seem real, going to try to figure them out before landing.
> >
> >
> > http://codereview.chromium.org/399043
> >
>

[jamesr, 2009-11-17 23:22:27]

Failure identified - it's not this patch.  Will land whenever the tree opens up.