[DevTools] Remote frontend is now embedded to iframe. (issue 730433003 by dgozman@chromium.org)

[dgo...@chromium.org]

Reviewers: pfeldman,

Message:
Take a look please.

Description:
[DevTools] Remote frontend is now embedded to iframe.

Remote protocol handler should now report "inspector.html".

BUG=431697

Please review this at https://codereview.chromium.org/730433003/

Base URL: https://chromium.googlesource.com/chromium/src.git@master

Affected files (+20, -17 lines):
M android_webview/native/aw_dev_tools_server.cc
M chrome/browser/android/dev_tools_server.cc
M chrome/browser/devtools/devtools_window.h
M chrome/browser/devtools/devtools_window.cc
M chrome/browser/ui/webui/devtools_ui.cc
M chromecast/browser/devtools/remote_debugging_server.cc
M content/browser/devtools/devtools_http_handler_impl.cc
M content/shell/browser/shell_devtools_manager_delegate.cc


Index: android_webview/native/aw_dev_tools_server.cc
diff --git a/android_webview/native/aw_dev_tools_server.cc
b/android_webview/native/aw_dev_tools_server.cc
index
b5ce81e870db95fe6308354d0b2dba7922acd178..7d8d6ee07d4f4c8ca769b7f3590996551e1d8d80
100644
--- a/android_webview/native/aw_dev_tools_server.cc
+++ b/android_webview/native/aw_dev_tools_server.cc
@@ -29,7 +29,7 @@ using content::WebContents;
namespace {

const char kFrontEndURL[] =
- "http://chrome-devtools-frontend.appspot.com/serve_rev/%s/devtools.html";
+ "http://chrome-devtools-frontend.appspot.com/serve_rev/%s/inspector.html";
const char kSocketNameFormat[] = "webview_devtools_remote_%d";
const char kTetheringSocketName[] = "webview_devtools_tethering_%d_%d";

Index: chrome/browser/android/dev_tools_server.cc
diff --git a/chrome/browser/android/dev_tools_server.cc
b/chrome/browser/android/dev_tools_server.cc
index
96fe4735cb988ca6bf8832d4962fe8500510a68a..e6192a3813207b9ad1442e85942788e7c10e0010
100644
--- a/chrome/browser/android/dev_tools_server.cc
+++ b/chrome/browser/android/dev_tools_server.cc
@@ -64,7 +64,7 @@ namespace {
const char kDevToolsChannelNameFormat[] = "%s_devtools_remote";

const char kFrontEndURL[] =
- "http://chrome-devtools-frontend.appspot.com/serve_rev/%s/devtools.html";
+ "http://chrome-devtools-frontend.appspot.com/serve_rev/%s/inspector.html";
const char kTetheringSocketName[] = "chrome_devtools_tethering_%d_%d";

const int kBackLog = 10;
Index: chrome/browser/devtools/devtools_window.cc
diff --git a/chrome/browser/devtools/devtools_window.cc
b/chrome/browser/devtools/devtools_window.cc
index
0b23a79b04523c74af88fb5f88792fd3b99246d6..2527e47055773922280cf2fe7f890fc2c1796f3f
100644
--- a/chrome/browser/devtools/devtools_window.cc
+++ b/chrome/browser/devtools/devtools_window.cc
@@ -45,6 +45,7 @@
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_client.h"
#include "content/public/common/url_constants.h"
+#include "net/base/escape.h"
#include "third_party/WebKit/public/web/WebInputEvent.h"
#include "ui/base/page_transition_types.h"
#include "ui/events/keycodes/keyboard_codes.h"
@@ -418,7 +419,7 @@ DevToolsWindow*
DevToolsWindow::OpenDevToolsWindowForWorker(
DevToolsWindow* DevToolsWindow::CreateDevToolsWindowForWorker(
Profile* profile) {
content::RecordAction(base::UserMetricsAction("DevTools_InspectWorker"));
- return Create(profile, GURL(), NULL, true, false, false, "");
+ return Create(profile, GURL(), NULL, true, std::string(), false, "");
}

// static
@@ -459,8 +460,8 @@ void DevToolsWindow::OpenExternalFrontend(
bool isWorker) {
DevToolsWindow* window = FindDevToolsWindow(agent_host.get());
if (!window) {
- window = Create(profile, DevToolsUI::GetProxyURL(frontend_url), NULL,
- isWorker, true, false, "");
+ window = Create(profile, GURL(), NULL, isWorker,
+ DevToolsUI::GetProxyURL(frontend_url).spec(),
false, "");
window->bindings_->AttachTo(agent_host);
}
window->ScheduleShow(DevToolsToggleAction::Show());
@@ -481,8 +482,8 @@ DevToolsWindow* DevToolsWindow::ToggleDevToolsWindow(
inspected_web_contents->GetBrowserContext());
content::RecordAction(
base::UserMetricsAction("DevTools_InspectRenderer"));
- window = Create(
- profile, GURL(), inspected_web_contents, false, false, true,
settings);
+ window = Create(profile, GURL(), inspected_web_contents,
+ false, std::string(), true, settings);
window->bindings_->AttachTo(agent.get());
do_open = true;
}
@@ -722,7 +723,7 @@ DevToolsWindow* DevToolsWindow::Create(
const GURL& frontend_url,
content::WebContents* inspected_web_contents,
bool shared_worker_frontend,
- bool external_frontend,
+ const std::string& remote_frontend,
bool can_dock,
const std::string& settings) {
if (inspected_web_contents) {
@@ -739,7 +740,7 @@ DevToolsWindow* DevToolsWindow::Create(
// Create WebContents with devtools.
GURL url(GetDevToolsURL(profile, frontend_url,
shared_worker_frontend,
- external_frontend,
+ remote_frontend,
can_dock, settings));
return new DevToolsWindow(profile, url, inspected_web_contents,
can_dock);
}
@@ -748,7 +749,7 @@ DevToolsWindow* DevToolsWindow::Create(
GURL DevToolsWindow::GetDevToolsURL(Profile* profile,
const GURL& base_url,
bool shared_worker_frontend,
- bool external_frontend,
+ const std::string& remote_frontend,
bool can_dock,
const std::string& settings) {
// Compatibility errors are encoded with data urls, pass them
@@ -763,8 +764,10 @@ GURL DevToolsWindow::GetDevToolsURL(Profile* profile,
((frontend_url.find("?") == std::string::npos) ? "?" : "&"));
if (shared_worker_frontend)
url_string += "&isSharedWorker=true";
- if (external_frontend)
+ if (remote_frontend.size()) {
url_string += "&remoteFrontend=true";
+ url_string += "&remoteFrontendUrl=" + net::EscapePath(remote_frontend);
+ }
if (can_dock)
url_string += "&can_dock=true";
if (settings.size())
Index: chrome/browser/devtools/devtools_window.h
diff --git a/chrome/browser/devtools/devtools_window.h
b/chrome/browser/devtools/devtools_window.h
index
449eac8f3c000a1896fef074993285176c365a1f..d35e8cd8664d7983b55bf1c6c4185a00e95cf546
100644
--- a/chrome/browser/devtools/devtools_window.h
+++ b/chrome/browser/devtools/devtools_window.h
@@ -207,13 +207,13 @@ class DevToolsWindow : public
DevToolsUIBindings::Delegate,
const GURL& frontend_url,
content::WebContents*
inspected_web_contents,
bool shared_worker_frontend,
- bool external_frontend,
+ const std::string& remote_frontend,
bool can_dock,
const std::string& settings);
static GURL GetDevToolsURL(Profile* profile,
const GURL& base_url,
bool shared_worker_frontend,
- bool external_frontend,
+ const std::string& remote_frontend,
bool can_dock,
const std::string& settings);
static DevToolsWindow* FindDevToolsWindow(content::DevToolsAgentHost*);
Index: chrome/browser/ui/webui/devtools_ui.cc
diff --git a/chrome/browser/ui/webui/devtools_ui.cc
b/chrome/browser/ui/webui/devtools_ui.cc
index
a8f74b88041c5e82e7ddb6bb7c31011ab2054f04..4e06c256faf17dc29a71dd59d0a95db66be8f0f4
100644
--- a/chrome/browser/ui/webui/devtools_ui.cc
+++ b/chrome/browser/ui/webui/devtools_ui.cc
@@ -47,7 +47,7 @@ const char kHttpNotFound[] = "HTTP/1.1 404 Not Found\n\n";
#if defined(DEBUG_DEVTOOLS)
// Local frontend url provided by InspectUI.
const char kFallbackFrontendURL[] =
- "chrome-devtools://devtools/bundled/devtools.html";
+ "chrome-devtools://devtools/bundled/inspector.html";
#else
// URL causing the DevTools window to display a plain text warning.
const char kFallbackFrontendURL[] =
Index: chromecast/browser/devtools/remote_debugging_server.cc
diff --git a/chromecast/browser/devtools/remote_debugging_server.cc
b/chromecast/browser/devtools/remote_debugging_server.cc
index
e0abdc072d809f1ad95240596c0aebf611e41328..de68f8b276b2a0a72377acbd537957f82db6904b
100644
--- a/chromecast/browser/devtools/remote_debugging_server.cc
+++ b/chromecast/browser/devtools/remote_debugging_server.cc
@@ -30,7 +30,7 @@ namespace shell {
namespace {

const char kFrontEndURL[] =
- "https://chrome-devtools-frontend.appspot.com/serve_rev/%s/devtools.html";
+ "https://chrome-devtools-frontend.appspot.com/serve_rev/%s/inspector.html";
const int kDefaultRemoteDebuggingPort = 9222;

#if defined(OS_ANDROID)
Index: content/browser/devtools/devtools_http_handler_impl.cc
diff --git a/content/browser/devtools/devtools_http_handler_impl.cc
b/content/browser/devtools/devtools_http_handler_impl.cc
index
42e6f5e069f9e5fd4e307815884705c3186581fa..1104ee5051cf028e9cf913d013d9a1061fee4bfc
100644
--- a/content/browser/devtools/devtools_http_handler_impl.cc
+++ b/content/browser/devtools/devtools_http_handler_impl.cc
@@ -979,7 +979,7 @@ DevToolsHttpHandlerImpl::DevToolsHttpHandlerImpl(
delegate_(delegate),
weak_factory_(this) {
if (frontend_url_.empty())
- frontend_url_ = "/devtools/devtools.html";
+ frontend_url_ = "/devtools/inspector.html";

BrowserThread::PostTask(
BrowserThread::FILE, FROM_HERE,
Index: content/shell/browser/shell_devtools_manager_delegate.cc
diff --git a/content/shell/browser/shell_devtools_manager_delegate.cc
b/content/shell/browser/shell_devtools_manager_delegate.cc
index
8ae3a2e9873a797644bc5aab60d9200c69683461..80c37a04ab8811b25027230c4fc71be1dcefe7ec
100644
--- a/content/shell/browser/shell_devtools_manager_delegate.cc
+++ b/content/shell/browser/shell_devtools_manager_delegate.cc
@@ -38,7 +38,7 @@ namespace {

#if defined(OS_ANDROID)
const char kFrontEndURL[] =
- "http://chrome-devtools-frontend.appspot.com/serve_rev/%s/devtools.html";
+ "http://chrome-devtools-frontend.appspot.com/serve_rev/%s/inspector.html";
#endif
const char kTargetTypePage[] = "page";
const char kTargetTypeServiceWorker[] = "service_worker";

[mnag...@chromium.org]

Please postpone landing until I check what we need to do on the "cloud
frontend"
side for AppCache. Currently we need a magic "FALLBACK" entry in the
manifest
for "devtools.html", to enforce all requests that have query parameters
"..devtools.html?blah-blah" to go directly to AppCache instead of checking
the
cloud first. Probably, we will now need an entry for "inspector.html" too.

See e.g.
https://chrome-devtools-frontend.appspot.com/serve_rev/@183548/183548.manifest :

CACHE MANIFEST
FALLBACK:
devtools.html devtools.html

https://codereview.chromium.org/730433003/

[dgo...@chromium.org]

Sure. In fact, we will only need inspector.html in the future (at least,
that's
the plan), but for now we should mention both there.

https://codereview.chromium.org/730433003/

[mnag...@chromium.org]

Yes. Adding both of them works OK, no problem with that. I will also update
the
script that injects the reference to the manifest into the main .html file
--
inspector.html in this case. So go ahead.

https://codereview.chromium.org/730433003/

[pfel...@chromium.org]

lgtm



https://codereview.chromium.org/730433003/

[dgo...@chromium.org]

Hello folks,

Could you please take a look at:

mnaganov@ - at android_webview/;
gunsch@ - at chromecast/;
dtrainor@ - at chrome/browser/android;

Thanks,
Dmitry


https://codereview.chromium.org/730433003/

[mnag...@chromium.org]

android_webview LGTM

https://codereview.chromium.org/730433003/

[gun...@chromium.org]

chromecast/ lgtm

https://codereview.chromium.org/730433003/

[dtra...@chromium.org]

chrome/browser/android lgtm

https://chromiumcodereview.appspot.com/730433003/