Move dispatching and sending of the last extension specific messages out of TabContents and Rende...

Move dispatching and sending of the last extension specific messages out of TabContents and RenderView.I added a TabContents::Registrar helper class for allowing classing to temporarily observe a TabContents.  This allows them to easily and safetly filter IPC messages.  I used this for ExecuteCodeInTabFunction.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=80468

[jam, 2011-04-04 23:16:16]

In the next (and last extensions) change, I will make ProcessWebUIMessage not
take a ExtensionHostMsg_DomMessage_Params.

[Aaron Boodman, 2011-04-05 01:56:24]

I am OK with this. The interaction btwn ExtensionMessageDispatcher and
ExtensionFunctionDispatcher is a bit odd, but I think I see how to fix it. I
will do that separately. Basically, I think I can finally untangle things so
that EFD doesn't have to have such an intricate lifetime. I will have it just be
owned by EMD and have the same lifetime as the owning RVHD.

Please rename ExtensionMessageDispatcher to something like
ExtensionMessageHandler to avoid confusion with ExtensionFunctionDispatcher.

Matt Perry should also take a look.

http://codereview.chromium.org/6794035/diff/2050/chrome/browser/extensions/ex...
File chrome/browser/extensions/execute_code_in_tab_function.cc (right):

http://codereview.chromium.org/6794035/diff/2050/chrome/browser/extensions/ex...
chrome/browser/extensions/execute_code_in_tab_function.cc:172:
ExtensionMsg_ExecuteCode_Params params;
Hey, I recognize this code.

http://codereview.chromium.org/6794035/diff/8001/content/browser/renderer_hos...
File content/browser/renderer_host/render_view_host.h (right):

http://codereview.chromium.org/6794035/diff/8001/content/browser/renderer_hos...
content/browser/renderer_host/render_view_host.h:79: //  conduit with a
renderer. This is so we can build HTMLe views not only as
whoops

http://codereview.chromium.org/6794035/diff/8001/content/common/notification_...
File content/common/notification_type.h (left):

http://codereview.chromium.org/6794035/diff/8001/content/common/notification_...
content/common/notification_type.h:289: TAB_CODE_EXECUTED,
hooray

[jam, 2011-04-05 17:50:40]

On Mon, Apr 4, 2011 at 6:56 PM, <aa@chromium.org> wrote:

> I am OK with this. The interaction btwn ExtensionMessageDispatcher and
> ExtensionFunctionDispatcher is a bit odd, but I think I see how to fix it.
> I
> will do that separately. Basically, I think I can finally untangle things
> so
> that EFD doesn't have to have such an intricate lifetime. I will have it
> just be
> owned by EMD and have the same lifetime as the owning RVHD.
>

sg.  When you do this, can you take care of making ProcessWebUIMessage not
take a ExtensionHostMsg_DomMessage_Params?  I just realized I can't do this
now, since that struct will need to be passed by WebUI to ExtensionUI to
ExtensionFunctionDispatcher.


>
> Please rename ExtensionMessageDispatcher to something like
> ExtensionMessageHandler to avoid confusion with
> ExtensionFunctionDispatcher.
>

Done


>
> Matt Perry should also take a look.
>
>
>
>
http://codereview.chromium.org/6794035/diff/2050/chrome/browser/extensions/ex...
> File chrome/browser/extensions/execute_code_in_tab_function.cc (right):
>
>
>
http://codereview.chromium.org/6794035/diff/2050/chrome/browser/extensions/ex...
> chrome/browser/extensions/execute_code_in_tab_function.cc:172:
> ExtensionMsg_ExecuteCode_Params params;
> Hey, I recognize this code.
>
>
>
http://codereview.chromium.org/6794035/diff/8001/content/browser/renderer_hos...
> File content/browser/renderer_host/render_view_host.h (right):
>
>
>
http://codereview.chromium.org/6794035/diff/8001/content/browser/renderer_hos...
> content/browser/renderer_host/render_view_host.h:79: //  conduit with a
> renderer. This is so we can build HTMLe views not only as
> whoops
>
>
>
http://codereview.chromium.org/6794035/diff/8001/content/common/notification_...
> File content/common/notification_type.h (left):
>
>
>
http://codereview.chromium.org/6794035/diff/8001/content/common/notification_...
> content/common/notification_type.h:289: TAB_CODE_EXECUTED,
> hooray
>
>
> http://codereview.chromium.org/6794035/
>

[jam, 2011-04-05 19:09:29]

On Tue, Apr 5, 2011 at 10:50 AM, John Abd-El-Malek <jam@chromium.org> wrote:

>
>
> On Mon, Apr 4, 2011 at 6:56 PM, <aa@chromium.org> wrote:
>
>> I am OK with this. The interaction btwn ExtensionMessageDispatcher and
>> ExtensionFunctionDispatcher is a bit odd, but I think I see how to fix it.
>> I
>> will do that separately. Basically, I think I can finally untangle things
>> so
>> that EFD doesn't have to have such an intricate lifetime. I will have it
>> just be
>> owned by EMD and have the same lifetime as the owning RVHD.
>>
>
> sg.  When you do this, can you take care of making ProcessWebUIMessage not
> take a ExtensionHostMsg_DomMessage_Params?  I just realized I can't do
> this now, since that struct will need to be passed by WebUI to ExtensionUI
> to ExtensionFunctionDispatcher.
>

also, when you do this, please add a deps rule to content/browser
disallowing chrome/common/extensions to prevent any more sneaking in (the
struct is the last dependency)


>
>>
>> Please rename ExtensionMessageDispatcher to something like
>> ExtensionMessageHandler to avoid confusion with
>> ExtensionFunctionDispatcher.
>>
>
> Done
>
>
>>
>> Matt Perry should also take a look.
>>
>>
>>
>>
http://codereview.chromium.org/6794035/diff/2050/chrome/browser/extensions/ex...
>> File chrome/browser/extensions/execute_code_in_tab_function.cc (right):
>>
>>
>>
http://codereview.chromium.org/6794035/diff/2050/chrome/browser/extensions/ex...
>> chrome/browser/extensions/execute_code_in_tab_function.cc:172:
>> ExtensionMsg_ExecuteCode_Params params;
>> Hey, I recognize this code.
>>
>>
>>
http://codereview.chromium.org/6794035/diff/8001/content/browser/renderer_hos...
>> File content/browser/renderer_host/render_view_host.h (right):
>>
>>
>>
http://codereview.chromium.org/6794035/diff/8001/content/browser/renderer_hos...
>> content/browser/renderer_host/render_view_host.h:79: //  conduit with a
>> renderer. This is so we can build HTMLe views not only as
>> whoops
>>
>>
>>
http://codereview.chromium.org/6794035/diff/8001/content/common/notification_...
>> File content/common/notification_type.h (left):
>>
>>
>>
http://codereview.chromium.org/6794035/diff/8001/content/common/notification_...
>> content/common/notification_type.h:289: TAB_CODE_EXECUTED,
>> hooray
>>
>>
>> http://codereview.chromium.org/6794035/
>>
>
>

[Matt Perry, 2011-04-05 19:18:32]

Cool. It's good to see this stuff getting untangled.

I just found a few nits. LGTM.

http://codereview.chromium.org/6794035/diff/7103/chrome/browser/extensions/ex...
File chrome/browser/extensions/execute_code_in_tab_function.cc (right):

http://codereview.chromium.org/6794035/diff/7103/chrome/browser/extensions/ex...
chrome/browser/extensions/execute_code_in_tab_function.cc:197: if
(message_request_id != request_id())
why do this manually? can't you do this check in OnExecuteCodeFinished?

http://codereview.chromium.org/6794035/diff/7103/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_message_handler.cc (right):

http://codereview.chromium.org/6794035/diff/7103/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_message_handler.cc:49: routing_id,
params.request_id, false, std::string(),
nit: indent + 2

http://codereview.chromium.org/6794035/diff/7103/content/browser/tab_contents...
File content/browser/tab_contents/tab_contents_observer.h (right):

http://codereview.chromium.org/6794035/diff/7103/content/browser/tab_contents...
content/browser/tab_contents/tab_contents_observer.h:18: // Use this as a member
variable in a class that uses the emptry constructor
nit: empty

[jam, 2011-04-05 19:25:38]

I'll do the nits in the next cl

http://codereview.chromium.org/6794035/diff/7103/chrome/browser/extensions/ex...
File chrome/browser/extensions/execute_code_in_tab_function.cc (right):

http://codereview.chromium.org/6794035/diff/7103/chrome/browser/extensions/ex...
chrome/browser/extensions/execute_code_in_tab_function.cc:197: if
(message_request_id != request_id())
On 2011/04/05 19:18:33, Matt Perry wrote:
> why do this manually? can't you do this check in OnExecuteCodeFinished?

that way I can reply false and let other message ExecuteCodeInTabFunction
objects whose request_id match get the message

http://codereview.chromium.org/6794035/diff/7103/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_message_handler.cc (right):

http://codereview.chromium.org/6794035/diff/7103/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_message_handler.cc:49: routing_id,
params.request_id, false, std::string(),
On 2011/04/05 19:18:33, Matt Perry wrote:
> nit: indent + 2

Done.

http://codereview.chromium.org/6794035/diff/7103/content/browser/tab_contents...
File content/browser/tab_contents/tab_contents_observer.h (right):

http://codereview.chromium.org/6794035/diff/7103/content/browser/tab_contents...
content/browser/tab_contents/tab_contents_observer.h:18: // Use this as a member
variable in a class that uses the emptry constructor
On 2011/04/05 19:18:33, Matt Perry wrote:
> nit: empty

Done.