Remove dependencies on extensions from content/browser/debugger.

Remove dependencies on extensions from content/browser/debugger.

This is done by moving embedder-specific protocol registration to
ContentBrowserClient, moving ExtensionPortsRemoteService back to
Chrome, and retrieving a list of "extensions" (in the WebKit debugger
sense, not the Chrome extensions sense) to add to the debugger from
the embedder.

BUG=84078
TEST=existing (primarily unit_tests and interactive_ui_tests)

[brettw, 2011-07-06 16:15:39]

I realize you didn't write the test stuff, but can you make a fix to the
timeouts?

http://codereview.chromium.org/7309011/diff/1/chrome/browser/debugger/devtool...
File chrome/browser/debugger/devtools_extension_debug_unittest.cc (right):

http://codereview.chromium.org/7309011/diff/1/chrome/browser/debugger/devtool...
chrome/browser/debugger/devtools_extension_debug_unittest.cc:48: 4*1000);
What's this magic number? Can you have a named const for this with units in the
name? Actually, if this is some kind of timeout, you should be using the
appropriate value from base/test/test_timeouts.h rather than hardcoding it
yourself.

Also, should probably be indented to the ( on the previous line.

http://codereview.chromium.org/7309011/diff/1/chrome/browser/debugger/devtool...
chrome/browser/debugger/devtools_extension_debug_unittest.cc:71: 4*1000);
Ditto

http://codereview.chromium.org/7309011/diff/1/chrome/browser/debugger/extensi...
File chrome/browser/debugger/extension_ports_remote_service.h (right):

http://codereview.chromium.org/7309011/diff/1/chrome/browser/debugger/extensi...
chrome/browser/debugger/extension_ports_remote_service.h:9: #ifndef
CHROME_BROWSER_DEBUGGER_EXTENSION_PORTS_REMOTE_SERVICE_H_
Update guards.

[brettw, 2011-07-06 16:15:47]

LGTM with previous comments addressed.

[Jói, 2011-07-06 16:38:56]

Thanks Brett, I plan to submit once try jobs pass.

http://codereview.chromium.org/7309011/diff/1/chrome/browser/debugger/devtool...
File chrome/browser/debugger/devtools_extension_debug_unittest.cc (right):

http://codereview.chromium.org/7309011/diff/1/chrome/browser/debugger/devtool...
chrome/browser/debugger/devtools_extension_debug_unittest.cc:48: 4*1000);
On 2011/07/06 16:15:39, brettw wrote:
> What's this magic number? Can you have a named const for this with units in
the
> name? Actually, if this is some kind of timeout, you should be using the
> appropriate value from base/test/test_timeouts.h rather than hardcoding it
> yourself.
> 
> Also, should probably be indented to the ( on the previous line.

Changed to:

      MessageLoop::current()->PostDelayedTask(
          FROM_HERE, delayed_quit, 2 * TestTimeouts::action_timeout());

(2 times action timeout so as not to change timeout on normal runs, where action
timeout is 2s)

http://codereview.chromium.org/7309011/diff/1/chrome/browser/debugger/devtool...
chrome/browser/debugger/devtools_extension_debug_unittest.cc:71: 4*1000);
On 2011/07/06 16:15:39, brettw wrote:
> Ditto

Done.

http://codereview.chromium.org/7309011/diff/1/chrome/browser/debugger/extensi...
File chrome/browser/debugger/extension_ports_remote_service.h (right):

http://codereview.chromium.org/7309011/diff/1/chrome/browser/debugger/extensi...
chrome/browser/debugger/extension_ports_remote_service.h:9: #ifndef
CHROME_BROWSER_DEBUGGER_EXTENSION_PORTS_REMOTE_SERVICE_H_
On 2011/07/06 16:15:39, brettw wrote:
> Update guards.

This guard is already correct, but I will look (separately at why it was already
correct - I think I submitted the guard change you reviewed, but maybe something
weird happened.  At a quick look, the guards in content/browser/debugger look
correct but I will verify.

[brettw, 2011-07-06 16:56:32]

http://codereview.chromium.org/7309011/diff/4002/content/browser/debugger/dev...
File content/browser/debugger/devtools_sanity_unittest.h (right):

http://codereview.chromium.org/7309011/diff/4002/content/browser/debugger/dev...
content/browser/debugger/devtools_sanity_unittest.h:18: class CancelableQuitTask
: public Task {
Can you make this a member of DevToolsSanityTest so this generic name isn't in
the global scope?

[Jói, 2011-07-06 18:04:52]

Moved the class, plan to commit once trybots are happy.

http://codereview.chromium.org/7309011/diff/4002/content/browser/debugger/dev...
File content/browser/debugger/devtools_sanity_unittest.h (right):

http://codereview.chromium.org/7309011/diff/4002/content/browser/debugger/dev...
content/browser/debugger/devtools_sanity_unittest.h:18: class CancelableQuitTask
: public Task {
On 2011/07/06 16:56:32, brettw wrote:
> Can you make this a member of DevToolsSanityTest so this generic name isn't in
> the global scope?

Taking another look at it, it's only used in
chrome/browser/debugger/devtools_extension_debug_unittest.cc so I moved it
there.

[Jói, 2011-07-07 20:25:56]

I'm abandoning this change based on discussions with the devtools
team.  All of the legacy protocol stuff (which includes the
extension_ports_remote_service.cc and .h files) is going away soon,
and is not depended on by anything in content/, so we might as well
leave it all on the Chrome side, and this will be a separate
whole-sale move back of those files.

Sorry for the noise.

Cheers,
Jói


On Wed, Jul 6, 2011 at 2:04 PM,  <joi@chromium.org> wrote:
> Moved the class, plan to commit once trybots are happy.
>
>
>
http://codereview.chromium.org/7309011/diff/4002/content/browser/debugger/dev...
> File content/browser/debugger/devtools_sanity_unittest.h (right):
>
>
http://codereview.chromium.org/7309011/diff/4002/content/browser/debugger/dev...
> content/browser/debugger/devtools_sanity_unittest.h:18: class
> CancelableQuitTask : public Task {
> On 2011/07/06 16:56:32, brettw wrote:
>>
>> Can you make this a member of DevToolsSanityTest so this generic name
>
> isn't in
>>
>> the global scope?
>
> Taking another look at it, it's only used in
> chrome/browser/debugger/devtools_extension_debug_unittest.cc so I moved
> it there.
>
> http://codereview.chromium.org/7309011/
>