Extension ports devtools remote service....

Extension ports devtools remote service.

Wires message ports to extensions through the devtools remote socket.

BUG=none
TEST=none

[sdoyon, 2009-08-21 17:34:28]

I initially put this together for my own use and then thought perhaps
someone else might find it useful.

It allows an external program to exchange messages with an extension
through the devtools remote socket.

Some possible applications:
1) Debugging an extension, remote control, testing.
2) Exploring and debugging web pages through a cooperating content script.
3) Remote read-eval-print JS console, with the help of an extension
   that eval()'s the messages it receives.

My motivation is that the developer tools embedded in Chrome are not
currently accessible to me because I am blind, and I wanted a way to
peek at web page internals and monitor an extension.

I've cobbled together a rough text-mode console client that does #3)
in a few hundred lines. I could post it if there's interest.

Thanks

[Matt Perry, 2009-08-21 18:51:13]

I'm not familiar with the devtools side of things, so I only really looked at
extension_message_service.*.

http://codereview.chromium.org/174226/diff/1/2
File chrome/browser/extensions/extension_message_service.h (right):

http://codereview.chromium.org/174226/diff/1/2#newcode107
Line 107: int OpenDevtoolsChannelToExtension(
I'd prefer to separate the two cases into different methods:
- OpenDevtoolsChannelToTab
- OpenDevtoolsChannelToExtension

The latter is similar to OpenAutomationChannelToExtension. Maybe we can combine
the automation and devtools uses.

[pfeldman, 2009-08-21 18:54:30]

[+apavlov] who should be reviewing the devtools protocol-related code.

[sdoyon, 2009-08-22 01:35:10]

On Fri, 21 Aug 2009, mpcomplete@chromium.org wrote:

> File chrome/browser/extensions/extension_message_service.h (right):
>
> http://codereview.chromium.org/174226/diff/1/2#newcode107
> Line 107: int OpenDevtoolsChannelToExtension(
> I'd prefer to separate the two cases into different methods:
> - OpenDevtoolsChannelToTab
> - OpenDevtoolsChannelToExtension

Sure. Done.

> The latter is similar to OpenAutomationChannelToExtension. Maybe we can
> combine the automation and devtools uses.

I've combined the common code. It's better although it still seems
like a lot of functions.

Thanks!

[sdoyon, 2009-08-22 01:57:47]

Siggi ==>> I don't fully understand your TODO in
extension_message_service.cc, and I'm not clear on the concept of
automation having a tab as context. But IIRC you mentioned to me in
conversation that the connection origin was being ignored anyway. I
THINK we can take out the source_process_id and routing_id parameters
and automation won't care.

Would this patch accomplish the cleanup you wanted, or do you need to
specify an originating tab ID somehow?

Would you be so kind as to test that I won't be breaking your stuff
with this change?

Thanks

[apavlov, 2009-08-22 17:56:03]

While we are at it, can you put together a concise description of your tool
protocol, akin to those for "V8Debugger" and "DevToolsService", so that I can
paste it into the CDTP spec?

http://codereview.chromium.org/174226/diff/22/25
File chrome/browser/debugger/extension_ports_remote_service.cc (right):

http://codereview.chromium.org/174226/diff/22/25#newcode33
Line 33: //   {"command": "connect"
If you are not striving for the correct JSON, I'm fine with the occasionally
missing commas :)

http://codereview.chromium.org/174226/diff/22/25#newcode57
Line 57: //   {"command":"connect", "result":0}
This response describes the same command ("connect") as that in the line 44
above but the "data" below is not filled in. Should the response contain a
different command or am I missing something?

http://codereview.chromium.org/174226/diff/22/25#newcode59
Line 59: // Note this is a confirmation from the devtools protocol layer, not
Does this note pertain to the "connect" response? Should it follow the one
above, at line ~50?

http://codereview.chromium.org/174226/diff/22/25#newcode315
Line 315: << ">, channel_name <" << channel_name << ">"
I guess all leftmost << operators should be aligned

[sdoyon, 2009-08-23 16:30:14]

On Sat, 22 Aug 2009, apavlov@chromium.org wrote:

> While we are at it, can you put together a concise description of your
> tool protocol, akin to those for "V8Debugger" and "DevToolsService", so
> that I can paste it into the CDTP spec?

I'll be glad to!
Will do this tomorrow.

> http://codereview.chromium.org/174226/diff/22/25
> File chrome/browser/debugger/extension_ports_remote_service.cc (right):
>
> http://codereview.chromium.org/174226/diff/22/25#newcode33
> Line 33: //   {"command": "connect"
> If you are not striving for the correct JSON, I'm fine with the
> occasionally missing commas :)

:-) a bit too much hand editing I guess. Fixed.

> http://codereview.chromium.org/174226/diff/22/25#newcode57
> Line 57: //   {"command":"connect", "result":0}
> This response describes the same command ("connect") as that in the line
> 44 above but the "data" below is not filled in. Should the response
> contain a different command or am I missing something?

Oh sorry for the mess.
Response to "postMessage" should be a matching "postMessage" of course,
with no data.

> http://codereview.chromium.org/174226/diff/22/25#newcode59
> Line 59: // Note this is a confirmation from the devtools protocol
> layer, not
> Does this note pertain to the "connect" response? Should it follow the
> one above, at line ~50?

No it's for the "postMessage" reply. Initially I found it nice to have
devtools confirm my postMessage command and keep things in
lockstep. The actual response from the extension, if the extension
chooses to make one, comes back asynchronously, like for V8
commands. So the "postMessage" reply just confirms the message was
sent, which may be a bit superfluous. But I thought I should probably
keep it since errors will be returned if the port gets disconnected.

> http://codereview.chromium.org/174226/diff/22/25#newcode315
> Line 315: << ">, channel_name <" << channel_name << ">"
> I guess all leftmost << operators should be aligned

Done.

Many thanks!!

[apavlov, 2009-08-23 17:53:07]

LGTM (forgot to send it in the original review, sorry if it made you hold the
submission.)

On 2009/08/23 16:30:14, sdoyon wrote:
> > http://codereview.chromium.org/174226/diff/22/25#newcode59
> > Line 59: // Note this is a confirmation from the devtools protocol
> > layer, not
> > Does this note pertain to the "connect" response? Should it follow the
> > one above, at line ~50?
> 
> No it's for the "postMessage" reply. Initially I found it nice to have
> devtools confirm my postMessage command and keep things in
> lockstep. The actual response from the extension, if the extension
> chooses to make one, comes back asynchronously, like for V8
> commands. So the "postMessage" reply just confirms the message was
> sent, which may be a bit superfluous. But I thought I should probably
> keep it since errors will be returned if the port gets disconnected.

Aha, that makes it a lot more clear. Thanks for fixing all the stuff!

[Matt Perry, 2009-08-24 19:13:22]

Cool, lgtm.

[sigurdur.asgeirsson_gmail.com, 2009-08-25 17:16:58]

lgtm