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>

12 files changed, 52 insertions, 55 deletions

diff --git a/src/3rdparty b/src/3rdparty
-Subproject 76233a4b9575db4ddc7ec421aee44cb355bfa71
+Subproject 8cc991607e5b4f92c912a28fbcea4cf319b97c2
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
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index 1b95f86b4..92807380f 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -84,7 +84,6 @@ QQuickWebEngineViewPrivate::QQuickWebEngineViewPrivate()
     , m_settings(new QQuickWebEngineSettings)
     , contextMenuExtraItems(0)
     , loadProgress(0)
-    , inspectable(false)
     , m_isFullScreen(false)
     , isLoading(false)
     , devicePixelRatio(QGuiApplication::primaryScreen()->devicePixelRatio())
@@ -534,7 +533,6 @@ void QQuickWebEngineViewPrivate::ensureContentsAdapter()
     if (!adapter) {
         adapter = new WebContentsAdapter();
         adapter->initialize(this);
-        adapter->enableInspector(inspectable);
         if (explicitUrl.isValid())
             adapter->load(explicitUrl);
     }
@@ -724,21 +722,6 @@ qreal QQuickWebEngineView::zoomFactor() const
     return d->adapter->currentZoomFactor();
 }
 
-bool QQuickWebEngineViewExperimental::inspectable() const
-{
-    Q_D(const QQuickWebEngineView);
-    return d->inspectable;
-}
-
-void QQuickWebEngineViewExperimental::setInspectable(bool enable)
-{
-    Q_D(QQuickWebEngineView);
-    d->inspectable = enable;
-    if (!d->adapter)
-        return;
-    d->adapter->enableInspector(enable);
-}
-
 void QQuickWebEngineViewExperimental::setIsFullScreen(bool fullscreen)
 {
     d_ptr->m_isFullScreen = fullscreen;
diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h
index f773b3d28..f97f2a8cf 100644
--- a/src/webengine/api/qquickwebengineview_p_p.h
+++ b/src/webengine/api/qquickwebengineview_p_p.h
@@ -79,7 +79,6 @@ class Q_WEBENGINE_PRIVATE_EXPORT QQuickWebEngineViewExperimental : public QObjec
     Q_OBJECT
     Q_PROPERTY(QQuickWebEngineViewport *viewport READ viewport)
     Q_PROPERTY(QQmlComponent *extraContextMenuEntriesComponent READ extraContextMenuEntriesComponent WRITE setExtraContextMenuEntriesComponent NOTIFY extraContextMenuEntriesComponentChanged)
-    Q_PROPERTY(bool inspectable READ inspectable WRITE setInspectable)
     Q_PROPERTY(bool isFullScreen READ isFullScreen WRITE setIsFullScreen NOTIFY isFullScreenChanged)
     Q_PROPERTY(QQuickWebEngineHistory *navigationHistory READ navigationHistory CONSTANT FINAL)
     Q_PROPERTY(QQuickWebEngineSettings *settings READ settings)
@@ -100,8 +99,6 @@ public:
     };
     Q_DECLARE_FLAGS(FindFlags, FindFlag)
 
-    bool inspectable() const;
-    void setInspectable(bool);
     void setIsFullScreen(bool fullscreen);
     bool isFullScreen() const;
     QQuickWebEngineViewport *viewport() const;
@@ -199,7 +196,6 @@ public:
     QUrl explicitUrl;
     QUrl icon;
     int loadProgress;
-    bool inspectable;
     bool m_isFullScreen;
     bool isLoading;
     qreal devicePixelRatio;
diff --git a/tests/quicktestbrowser/main.cpp b/tests/quicktestbrowser/main.cpp
index d8fe01aa1..d44f644c3 100644
--- a/tests/quicktestbrowser/main.cpp
+++ b/tests/quicktestbrowser/main.cpp
@@ -53,6 +53,9 @@ int main(int argc, char **argv)
 {
     Application app(argc, argv);
 
+    // Enable dev tools by default for the test browser
+    if (qgetenv("QTWEBENGINE_REMOTE_DEBUGGING").isNull())
+        qputenv("QTWEBENGINE_REMOTE_DEBUGGING", "1337");
     QtWebEngine::initialize();
 
     ApplicationEngine appEngine;