Windowed plugins like Flash would cause the hung plugin dialog to show up whe...

Windowed plugins like Flash would cause the hung plugin dialog to show up when the print dialog was displayed.
The proposed fix is to signal the modal dialog event which would ensure that the renderer and plugins both pump
messages when the print dialog is displayed.

This fixes http://code.google.com/p/chromium/issues/detail?id=13804

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

[ananta, 2009-07-07 11:22:08]

Adding jam.

[jam, 2009-07-07 18:10:31]

lgtm

http://codereview.chromium.org/155133/diff/1042/46
File chrome/browser/renderer_host/resource_message_filter.h (right):

http://codereview.chromium.org/155133/diff/1042/46#newcode25
Line 25: #include "chrome/common/render_messages.h"
don't include render_messages in headers, it slows down the build.  just forward
declare ViewHostMsg_ScriptedPrint_Params

http://codereview.chromium.org/155133/diff/1042/56
File chrome/renderer/mock_render_thread.h (right):

http://codereview.chromium.org/155133/diff/1042/56#newcode13
Line 13: #include "chrome/common/render_messages.h"
ditto

[M-A Ruel, 2009-07-07 18:17:42]

http://codereview.chromium.org/155133/diff/1042/48
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/155133/diff/1042/48#newcode757
Line 757: params.routing_id,
It's kind of sad what we can't access it directly from the message. In practice,
this is duplication of data.

John, is there a way to get the currently processed message routing id?

http://codereview.chromium.org/155133/diff/1042/55
File chrome/renderer/mock_render_thread.cc (right):

http://codereview.chromium.org/155133/diff/1042/55#newcode118
Line 118: const ViewHostMsg_ScriptedPrint_Params& params,
align +4

[jam, 2009-07-07 18:24:43]

http://codereview.chromium.org/155133/diff/1042/48
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/155133/diff/1042/48#newcode757
Line 757: params.routing_id,
On 2009/07/07 18:17:42, Marc-Antoine Ruel wrote:
> It's kind of sad what we can't access it directly from the message. In
practice,
> this is duplication of data.
> 
> John, is there a way to get the currently processed message routing id?

Since this message is dispatched in ResourceMessageFilter, it shouldn't even be
routed.  It should be a control message.

To get the routing id of a message, you can't dispatch it with
IPC_MESSAGE_HANDLER.  You'd have to use IPC_MESSAGE_HANDLER_GENERIC, but then
you miss out on the automatic unpacking..

[ananta, 2009-07-07 18:35:28]

maruel, Based on John's comment regarding retrieving the routing id from the ipc
message, I don't think it is worth the effort. If you feel strongly about it let
me know.

Thanks
Ananta

http://codereview.chromium.org/155133/diff/1042/46
File chrome/browser/renderer_host/resource_message_filter.h (right):

http://codereview.chromium.org/155133/diff/1042/46#newcode25
Line 25: #include "chrome/common/render_messages.h"
On 2009/07/07 18:10:31, John Abd-El-Malek wrote:
> don't include render_messages in headers, it slows down the build.  just
forward
> declare ViewHostMsg_ScriptedPrint_Params

Done.

http://codereview.chromium.org/155133/diff/1042/55
File chrome/renderer/mock_render_thread.cc (right):

http://codereview.chromium.org/155133/diff/1042/55#newcode118
Line 118: const ViewHostMsg_ScriptedPrint_Params& params,
On 2009/07/07 18:17:42, Marc-Antoine Ruel wrote:
> align +4

Done.

http://codereview.chromium.org/155133/diff/1042/56
File chrome/renderer/mock_render_thread.h (right):

http://codereview.chromium.org/155133/diff/1042/56#newcode13
Line 13: #include "chrome/common/render_messages.h"
On 2009/07/07 18:10:31, John Abd-El-Malek wrote:
> ditto

Done.

[M-A Ruel, 2009-07-07 18:46:54]

[-chromium-reviews]

Could routing_id be used to remove host_window_id and deduce it
browser-side? Note that it should be done in a separate CL.
M-A

On Tue, Jul 7, 2009 at 2:35 PM, <ananta@chromium.org> wrote:

> maruel, Based on John's comment regarding retrieving the routing id from
> the ipc message, I don't think it is worth the effort. If you feel
> strongly about it let me know.
>
> Thanks
> Ananta
>
>
> http://codereview.chromium.org/155133/diff/1042/46
> File chrome/browser/renderer_host/resource_message_filter.h (right):
>
> http://codereview.chromium.org/155133/diff/1042/46#newcode25
> Line 25: #include "chrome/common/render_messages.h"
> On 2009/07/07 18:10:31, John Abd-El-Malek wrote:
>
>> don't include render_messages in headers, it slows down the build.
>>
> just forward
>
>> declare ViewHostMsg_ScriptedPrint_Params
>>
>
> Done.
>
> http://codereview.chromium.org/155133/diff/1042/55
> File chrome/renderer/mock_render_thread.cc (right):
>
> http://codereview.chromium.org/155133/diff/1042/55#newcode118
> Line 118: const ViewHostMsg_ScriptedPrint_Params& params,
> On 2009/07/07 18:17:42, Marc-Antoine Ruel wrote:
>
>> align +4
>>
>
> Done.
>
> http://codereview.chromium.org/155133/diff/1042/56
> File chrome/renderer/mock_render_thread.h (right):
>
> http://codereview.chromium.org/155133/diff/1042/56#newcode13
> Line 13: #include "chrome/common/render_messages.h"
> On 2009/07/07 18:10:31, John Abd-El-Malek wrote:
>
>> ditto
>>
>
> Done.
>
>
> http://codereview.chromium.org/155133
>

[iyengar, 2009-07-07 19:00:22]

I will look into this in a subsequent CL.
Thanks
Ananta

On Tue, Jul 7, 2009 at 11:46 AM, Marc-Antoine Ruel <maruel@chromium.org>wrote:

> [-chromium-reviews]
>
> Could routing_id be used to remove host_window_id and deduce it
> browser-side? Note that it should be done in a separate CL.
> M-A
>
>
> On Tue, Jul 7, 2009 at 2:35 PM, <ananta@chromium.org> wrote:
>
>> maruel, Based on John's comment regarding retrieving the routing id from
>> the ipc message, I don't think it is worth the effort. If you feel
>> strongly about it let me know.
>>
>> Thanks
>> Ananta
>>
>>
>> http://codereview.chromium.org/155133/diff/1042/46
>> File chrome/browser/renderer_host/resource_message_filter.h (right):
>>
>> http://codereview.chromium.org/155133/diff/1042/46#newcode25
>> Line 25: #include "chrome/common/render_messages.h"
>> On 2009/07/07 18:10:31, John Abd-El-Malek wrote:
>>
>>> don't include render_messages in headers, it slows down the build.
>>>
>> just forward
>>
>>> declare ViewHostMsg_ScriptedPrint_Params
>>>
>>
>> Done.
>>
>> http://codereview.chromium.org/155133/diff/1042/55
>> File chrome/renderer/mock_render_thread.cc (right):
>>
>> http://codereview.chromium.org/155133/diff/1042/55#newcode118
>> Line 118: const ViewHostMsg_ScriptedPrint_Params& params,
>> On 2009/07/07 18:17:42, Marc-Antoine Ruel wrote:
>>
>>> align +4
>>>
>>
>> Done.
>>
>> http://codereview.chromium.org/155133/diff/1042/56
>> File chrome/renderer/mock_render_thread.h (right):
>>
>> http://codereview.chromium.org/155133/diff/1042/56#newcode13
>> Line 13: #include "chrome/common/render_messages.h"
>> On 2009/07/07 18:10:31, John Abd-El-Malek wrote:
>>
>>> ditto
>>>
>>
>> Done.
>>
>>
>> http://codereview.chromium.org/155133
>>
>
>

[jam, 2009-07-07 21:22:47]

http://codereview.chromium.org/155133/diff/1071/79
File chrome/common/render_messages_internal.h (right):

http://codereview.chromium.org/155133/diff/1071/79#newcode1081
Line 1081: IPC_SYNC_MESSAGE_ROUTED1_1(ViewHostMsg_ScriptedPrint,
this should be a control message since it's not actually routed.

[ananta, 2009-07-08 01:54:44]

http://codereview.chromium.org/155133/diff/1071/79
File chrome/common/render_messages_internal.h (right):

http://codereview.chromium.org/155133/diff/1071/79#newcode1081
Line 1081: IPC_SYNC_MESSAGE_ROUTED1_1(ViewHostMsg_ScriptedPrint,
On 2009/07/07 21:22:47, John Abd-El-Malek wrote:
> this should be a control message since it's not actually routed.

Done.

[ananta, 2009-07-08 01:56:16]

The ViewHostMsg_ScriptedPrint has been changed to a control message.