[DevTools] Move DevToolsClientHost functionality out of DevToolsFrontendHost.

[DevTools] Move DevToolsClientHost functionality out of DevToolsFrontendHost.

DevToolsFrontendHost is just a transport now.

Next step is to remove HandleMessageFromDevToolsFrontendToBackend,
make it one of the embedder messages, and allow embedder to talk
to backend in itself.

BUG=398046
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287027

[dgozman, 2014-07-30 14:17:28]

Take a look please.

[pfeldman, 2014-07-31 11:47:25]

https://codereview.chromium.org/427143003/diff/1/chrome/browser/devtools/devt...
File chrome/browser/devtools/devtools_ui_bindings.cc (right):

https://codereview.chromium.org/427143003/diff/1/chrome/browser/devtools/devt...
chrome/browser/devtools/devtools_ui_bindings.cc:371: return
url.SchemeIs(content::kChromeDevToolsScheme);
It seems like we can make this decision in content.

https://codereview.chromium.org/427143003/diff/1/chrome/browser/devtools/devt...
chrome/browser/devtools/devtools_ui_bindings.cc:413:
frontend_host_->DispatchOnDevToolsFrontend(message);
Can we use a CallClientFunction here?

https://codereview.chromium.org/427143003/diff/1/chrome/browser/devtools/devt...
File chrome/browser/devtools/devtools_ui_bindings.h (right):

https://codereview.chromium.org/427143003/diff/1/chrome/browser/devtools/devt...
chrome/browser/devtools/devtools_ui_bindings.h:83: virtual bool
ShouldSetupDevToolsFrontendForUrl(const GURL& url) OVERRIDE;
Can there be a devtools frontend host that does not need a client?

https://codereview.chromium.org/427143003/diff/1/chrome/browser/devtools/devt...
chrome/browser/devtools/devtools_ui_bindings.h:86: virtual void
HandleMessageFromDevToolsFrontendToBackend(
New naming is really confusing.

https://codereview.chromium.org/427143003/diff/1/content/browser/devtools/dev...
File content/browser/devtools/devtools_frontend_host_impl.cc (right):

https://codereview.chromium.org/427143003/diff/1/content/browser/devtools/dev...
content/browser/devtools/devtools_frontend_host_impl.cc:28: if
(delegate_->ShouldSetupDevToolsFrontendForUrl(url)) {
Why would you do that here? You already do this unconditionally in the about to
navigate.

[dgozman, 2014-07-31 13:05:31]

PTAL

https://codereview.chromium.org/427143003/diff/1/chrome/browser/devtools/devt...
File chrome/browser/devtools/devtools_ui_bindings.cc (right):

https://codereview.chromium.org/427143003/diff/1/chrome/browser/devtools/devt...
chrome/browser/devtools/devtools_ui_bindings.cc:413:
frontend_host_->DispatchOnDevToolsFrontend(message);
On 2014/07/31 11:47:25, pfeldman wrote:
> Can we use a CallClientFunction here?

There is a TODO for that. It requires some blink-side changes.

https://codereview.chromium.org/427143003/diff/1/chrome/browser/devtools/devt...
File chrome/browser/devtools/devtools_ui_bindings.h (right):

https://codereview.chromium.org/427143003/diff/1/chrome/browser/devtools/devt...
chrome/browser/devtools/devtools_ui_bindings.h:83: virtual bool
ShouldSetupDevToolsFrontendForUrl(const GURL& url) OVERRIDE;
On 2014/07/31 11:47:25, pfeldman wrote:
> Can there be a devtools frontend host that does not need a client?

Removed.

https://codereview.chromium.org/427143003/diff/1/chrome/browser/devtools/devt...
chrome/browser/devtools/devtools_ui_bindings.h:86: virtual void
HandleMessageFromDevToolsFrontendToBackend(
On 2014/07/31 11:47:25, pfeldman wrote:
> New naming is really confusing.

Next step is removing this method (see TODO in devtools_frontend_host.h), so it
will not live long. And I can't come up with much better name.

https://codereview.chromium.org/427143003/diff/1/content/browser/devtools/dev...
File content/browser/devtools/devtools_frontend_host_impl.cc (right):

https://codereview.chromium.org/427143003/diff/1/content/browser/devtools/dev...
content/browser/devtools/devtools_frontend_host_impl.cc:28: if
(delegate_->ShouldSetupDevToolsFrontendForUrl(url)) {
On 2014/07/31 11:47:25, pfeldman wrote:
> Why would you do that here? You already do this unconditionally in the about
to
> navigate.

As discussed offline, this check is left in embedder (as it was before).

[dgozman, 2014-07-31 13:06:14]

+jam for content/public review.

[pfeldman, 2014-07-31 14:22:49]

lgtm

[jam, 2014-07-31 22:05:48]

lgtm

https://codereview.chromium.org/427143003/diff/20001/content/public/browser/d...
File content/public/browser/devtools_frontend_host.h (right):

https://codereview.chromium.org/427143003/diff/20001/content/public/browser/d...
content/public/browser/devtools_frontend_host.h:34: virtual void
HandleMessageFromDevToolsFrontendToBackend(
nit: once this is removed, please change the single-method Delegate interface to
a callback per the content api guidelines

[dgozman, 2014-08-01 10:01:25]

https://codereview.chromium.org/427143003/diff/20001/content/public/browser/d...
File content/public/browser/devtools_frontend_host.h (right):

https://codereview.chromium.org/427143003/diff/20001/content/public/browser/d...
content/public/browser/devtools_frontend_host.h:34: virtual void
HandleMessageFromDevToolsFrontendToBackend(
On 2014/07/31 22:05:47, jam wrote:
> nit: once this is removed, please change the single-method Delegate interface
to
> a callback per the content api guidelines

Sure!

[dgozman, 2014-08-01 10:01:31]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2014-08-01 10:03:46]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dgozman@chromium.org/427143003/20001

[commit-bot: I haz the power, 2014-08-01 17:14:45]

Change committed as 287027