Replace the inspectable property with an environment variable

The current implementation would enable or disable the inspector
globally when the inspectable property was set on a WebEngineView,
overwriting the value previously set by other pages.

Instead of havind default port for the debugging server and having
to enable debugging on individual pages, use an environment variable,
QTWEBENGINE_REMOTE_DEBUGGING, to enable the debugging server for the
whole application at the same time as specifying the port. The format
is the same as for QTWEBKIT_INSPECTOR_SERVER in QtWebKit.

QTWEBENGINE_REMOTE_DEBUGGING is set by default in quicktestbrowser
to ease development.

This also keeps the input reading from the --remote-debugging-port
command line switch for convenience, but its usage should be
considered internal.

This patch also take the opportunity to remove the unused
DevToolsHttpHandlerDelegateQt::m_browserContext and to move the
ownership from ContentBrowserClientQt to WebEngineContext since
the list of inspectable pages isn't bound to the BrowserContext
anyway.

Change-Id: I772687f88f4feee0cc14dd182b0129cc0ea384dd
Reviewed-by: Pierre Rossi <pierre.rossi@theqtcompany.com>

8 files changed, 49 insertions, 34 deletions

diff --git a/src/core/content_browser_client_qt.cpp b/src/core/content_browser_client_qt.cpp
index ef034efbc..6ebaadd87 100644
--- a/src/core/content_browser_client_qt.cpp
+++ b/src/core/content_browser_client_qt.cpp
@@ -366,15 +366,6 @@ net::URLRequestContextGetter* ContentBrowserClientQt::CreateRequestContext(conte
     return static_cast<BrowserContextQt*>(browser_context)->CreateRequestContext(protocol_handlers);
 }
 
-void ContentBrowserClientQt::enableInspector(bool enable, content::BrowserContext* browser_context)
-{
-    if (enable && !m_devtools) {
-        m_devtools.reset(new DevToolsHttpHandlerDelegateQt(browser_context));
-    } else if (!enable && m_devtools) {
-        m_devtools.reset();
-    }
-}
-
 content::QuotaPermissionContext *ContentBrowserClientQt::CreateQuotaPermissionContext()
 {
     return new QuotaPermissionContextQt;
diff --git a/src/core/content_browser_client_qt.h b/src/core/content_browser_client_qt.h
index 0a3acd9cf..4862ca704 100644
--- a/src/core/content_browser_client_qt.h
+++ b/src/core/content_browser_client_qt.h
@@ -66,7 +66,6 @@ class GLShareGroup;
 
 class BrowserContextQt;
 class BrowserMainPartsQt;
-class DevToolsHttpHandlerDelegateQt;
 class ResourceDispatcherHostDelegateQt;
 class ShareGroupQtQuick;
 
@@ -111,11 +110,8 @@ public:
 
     virtual std::string GetApplicationLocale() Q_DECL_OVERRIDE;
 
-    void enableInspector(bool enable, content::BrowserContext *browser_context);
-
 private:
     BrowserMainPartsQt* m_browserMainParts;
-    scoped_ptr<DevToolsHttpHandlerDelegateQt> m_devtools;
     scoped_ptr<ResourceDispatcherHostDelegateQt> m_resourceDispatcherHostDelegate;
     scoped_refptr<ShareGroupQtQuick> m_shareGroupQtQuick;
 };
diff --git a/src/core/dev_tools_http_handler_delegate_qt.cpp b/src/core/dev_tools_http_handler_delegate_qt.cpp
index 561a86f2a..4b866a6bc 100644
--- a/src/core/dev_tools_http_handler_delegate_qt.cpp
+++ b/src/core/dev_tools_http_handler_delegate_qt.cpp
@@ -40,6 +40,8 @@
 
 #include "dev_tools_http_handler_delegate_qt.h"
 
+#include "type_conversion.h"
+
 #include <QByteArray>
 #include <QFile>
 
@@ -56,6 +58,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/common/content_switches.h"
+#include "net/base/ip_endpoint.h"
 #include "net/socket/stream_listen_socket.h"
 #include "net/socket/tcp_server_socket.h"
 
@@ -140,26 +143,50 @@ bool Target::Close() const {
 
 }  // namespace
 
-DevToolsHttpHandlerDelegateQt::DevToolsHttpHandlerDelegateQt(BrowserContext* browser_context)
-    : m_browserContext(browser_context)
+DevToolsHttpHandlerDelegateQt::DevToolsHttpHandlerDelegateQt()
+    : m_devtoolsHttpHandler(0)
+    , m_bindAddress(QLatin1String("127.0.0.1"))
+    , m_port(0)
 {
-    const int defaultPort = 1337;
-    int listeningPort = defaultPort;
+    const QString inspectorEnv = QString::fromUtf8(qgetenv("QTWEBENGINE_REMOTE_DEBUGGING"));
     const CommandLine &commandLine = *CommandLine::ForCurrentProcess();
+    QString portStr;
+
     if (commandLine.HasSwitch(switches::kRemoteDebuggingPort)) {
-        std::string portString =
-            commandLine.GetSwitchValueASCII(switches::kRemoteDebuggingPort);
-        int portInt = 0;
-        if (base::StringToInt(portString, &portInt) && portInt > 0 && portInt < 65535)
-            listeningPort = portInt;
-    }
-    scoped_ptr<content::DevToolsHttpHandler::ServerSocketFactory> factory(new TCPServerSocketFactory("0.0.0.0", listeningPort, 1));
-    m_devtoolsHttpHandler = DevToolsHttpHandler::Start(factory.Pass(), std::string(), this, base::FilePath());
+        portStr = QString::fromStdString(commandLine.GetSwitchValueASCII(switches::kRemoteDebuggingPort));
+    } else if (!inspectorEnv.isEmpty()) {
+        int portColonPos = inspectorEnv.lastIndexOf(':');
+        if (portColonPos != -1) {
+            portStr = inspectorEnv.mid(portColonPos + 1);
+            m_bindAddress = inspectorEnv.mid(0, portColonPos);
+        } else
+            portStr = inspectorEnv;
+    } else
+        return;
+
+    bool ok = false;
+    m_port = portStr.toInt(&ok);
+    if (ok && m_port > 0 && m_port < 65535) {
+        scoped_ptr<content::DevToolsHttpHandler::ServerSocketFactory> factory(new TCPServerSocketFactory(m_bindAddress.toStdString(), m_port, 1));
+        m_devtoolsHttpHandler = DevToolsHttpHandler::Start(factory.Pass(), std::string(), this, base::FilePath());
+    } else
+        qWarning("Invalid port given for the inspector server \"%s\". Examples of valid input: \"12345\" or \"192.168.2.14:12345\" (with the address of one of this host's network interface).", qPrintable(portStr));
 }
 
 DevToolsHttpHandlerDelegateQt::~DevToolsHttpHandlerDelegateQt()
 {
-    m_devtoolsHttpHandler->Stop();
+    // Stop() takes care of deleting the DevToolsHttpHandler.
+    if (m_devtoolsHttpHandler)
+        m_devtoolsHttpHandler->Stop();
+}
+
+void DevToolsHttpHandlerDelegateQt::Initialized(const net::IPEndPoint& ip_address)
+{
+    if (ip_address.address().size()) {
+        QString addressAndPort = QString::fromStdString(ip_address.ToString());
+        qWarning("Remote debugging server started successfully. Try pointing a Chromium-based browser to http://%s", qPrintable(addressAndPort));
+    } else
+        qWarning("Couldn't start the inspector server on bind address \"%s\" and port \"%d\". In case of invalid input, try something like: \"12345\" or \"192.168.2.14:12345\" (with the address of one of this host's interface).", qPrintable(m_bindAddress), m_port);
 }
 
 std::string DevToolsHttpHandlerDelegateQt::GetDiscoveryPageHTML()
diff --git a/src/core/dev_tools_http_handler_delegate_qt.h b/src/core/dev_tools_http_handler_delegate_qt.h
index bbee613a2..8a8a658a6 100644
--- a/src/core/dev_tools_http_handler_delegate_qt.h
+++ b/src/core/dev_tools_http_handler_delegate_qt.h
@@ -40,6 +40,7 @@
 #include "content/public/browser/devtools_http_handler_delegate.h"
 #include "content/public/browser/devtools_manager_delegate.h"
 
+#include <QString>
 #include <QtCore/qcompilerdetection.h> // needed for Q_DECL_OVERRIDE
 
 namespace net {
@@ -55,10 +56,11 @@ class RenderViewHost;
 class DevToolsHttpHandlerDelegateQt : public content::DevToolsHttpHandlerDelegate {
 public:
 
-    explicit DevToolsHttpHandlerDelegateQt(content::BrowserContext* browser_context);
+    DevToolsHttpHandlerDelegateQt();
     virtual ~DevToolsHttpHandlerDelegateQt();
 
     // content::DevToolsHttpHandlerDelegate Overrides
+    virtual void Initialized(const net::IPEndPoint &ip_address) Q_DECL_OVERRIDE;
     virtual std::string GetDiscoveryPageHTML() Q_DECL_OVERRIDE;
     virtual bool BundlesFrontendResources() Q_DECL_OVERRIDE;
     virtual base::FilePath GetDebugFrontendDir() Q_DECL_OVERRIDE;
@@ -67,8 +69,9 @@ public:
     virtual scoped_ptr<net::StreamListenSocket> CreateSocketForTethering(net::StreamListenSocket::Delegate *delegate, std::string *name) Q_DECL_OVERRIDE;
 
 private:
-    content::BrowserContext* m_browserContext;
     content::DevToolsHttpHandler *m_devtoolsHttpHandler;
+    QString m_bindAddress;
+    int m_port;
 };
 
 class DevToolsManagerDelegateQt : public content::DevToolsManagerDelegate {
diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp
index b9c2018aa..3f6d7060c 100644
--- a/src/core/web_contents_adapter.cpp
+++ b/src/core/web_contents_adapter.cpp
@@ -664,11 +664,6 @@ qreal WebContentsAdapter::currentZoomFactor() const
     return content::ZoomLevelToZoomFactor(content::HostZoomMap::GetZoomLevel(d->webContents.get()));
 }
 
-void WebContentsAdapter::enableInspector(bool enable)
-{
-    ContentBrowserClientQt::Get()->enableInspector(enable, browserContext());
-}
-
 BrowserContextQt* WebContentsAdapter::browserContext()
 {
     Q_D(WebContentsAdapter);
diff --git a/src/core/web_contents_adapter.h b/src/core/web_contents_adapter.h
index 6dc8c2193..5b887c614 100644
--- a/src/core/web_contents_adapter.h
+++ b/src/core/web_contents_adapter.h
@@ -96,7 +96,6 @@ public:
     void serializeNavigationHistory(QDataStream &output);
     void setZoomFactor(qreal);
     qreal currentZoomFactor() const;
-    void enableInspector(bool);
     void filesSelectedInChooser(const QStringList &fileList, WebContentsAdapterClient::FileChooserMode);
     void runJavaScript(const QString &javaScript);
     quint64 runJavaScriptCallbackResult(const QString &javaScript);
diff --git a/src/core/web_engine_context.cpp b/src/core/web_engine_context.cpp
index 73ca9c603..d0a8bb84f 100644
--- a/src/core/web_engine_context.cpp
+++ b/src/core/web_engine_context.cpp
@@ -70,6 +70,7 @@
 #include "content_browser_client_qt.h"
 #include "content_client_qt.h"
 #include "content_main_delegate_qt.h"
+#include "dev_tools_http_handler_delegate_qt.h"
 #include "gl_context_qt.h"
 #include "media_capture_devices_dispatcher.h"
 #include "type_conversion.h"
@@ -262,6 +263,7 @@ WebEngineContext::WebEngineContext()
     m_runLoop.reset(new base::RunLoop);
     m_runLoop->BeforeRun();
 
+    m_devtools.reset(new DevToolsHttpHandlerDelegateQt);
     // Force the initialization of MediaCaptureDevicesDispatcher on the UI
     // thread to avoid a thread check assertion in its constructor when it
     // first gets referenced on the IO thread.
diff --git a/src/core/web_engine_context.h b/src/core/web_engine_context.h
index a44a48d3a..80ce65507 100644
--- a/src/core/web_engine_context.h
+++ b/src/core/web_engine_context.h
@@ -53,6 +53,7 @@ class ContentMainRunner;
 
 class BrowserContextAdapter;
 class ContentMainDelegateQt;
+class DevToolsHttpHandlerDelegateQt;
 class SurfaceFactoryQt;
 
 class WebEngineContext : public base::RefCounted<WebEngineContext> {
@@ -76,6 +77,7 @@ private:
 #endif
     QExplicitlySharedDataPointer<BrowserContextAdapter> m_defaultBrowserContext;
     QExplicitlySharedDataPointer<BrowserContextAdapter> m_offTheRecordBrowserContext;
+    scoped_ptr<DevToolsHttpHandlerDelegateQt> m_devtools;
 };
 
 #endif // WEB_ENGINE_CONTEXT_H