QQuickWebEngineView new window API refactoring

Improve the code and API in a few ways:
- Expose a more discoverable "request" argument in the signal.
- Use the request as the carrier of the backend WebContentsAdapter
  and get rid of our handle.
- Put the adoption method (renamed to openIn) on the request object
  and keep the view API clean of a context-specific adoptHandle method.
- Use an enum instead of strings for the new view destination.
- Do not let JavaScript own the request object since it won't be
  necessary until we want to support asynchronous view attachment.
  We can create the request object on the heap and let the JavaScript
  engine own the object once we want to support it.
- Move the request class to its own header.
- Replace tabs.currentView by currentWebView in the quicknanobrowser
  qml code since we now need this property on the root object anyway.

Change-Id: I40d7d15255f516ead9f3e414dd587bf345e6ca4b
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

7 files changed, 196 insertions, 81 deletions

diff --git a/src/webengine/api/qquickwebenginenewviewrequest.cpp b/src/webengine/api/qquickwebenginenewviewrequest.cpp
new file mode 100644
index 000000000..8af8f5b49
--- /dev/null
+++ b/src/webengine/api/qquickwebenginenewviewrequest.cpp
@@ -0,0 +1,71 @@
+/****************************************************************************
+**
+** Copyright (C) 2013 Digia Plc and/or its subsidiary(-ies).
+** Contact: http://www.qt-project.org/legal
+**
+** This file is part of the QtWebEngine module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and Digia.  For licensing terms and
+** conditions see http://qt.digia.com/licensing.  For further information
+** use the contact form at http://qt.digia.com/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Digia gives you certain additional
+** rights.  These rights are described in the Digia Qt LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3.0 as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU General Public License version 3.0 requirements will be
+** met: http://www.gnu.org/copyleft/gpl.html.
+**
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include "qquickwebenginenewviewrequest_p.h"
+
+#include "qquickwebengineview_p_p.h"
+#include "web_contents_adapter.h"
+
+QQuickWebEngineNewViewRequest::QQuickWebEngineNewViewRequest()
+{
+}
+
+QQuickWebEngineNewViewRequest::~QQuickWebEngineNewViewRequest()
+{
+}
+
+QQuickWebEngineView::NewViewDestination QQuickWebEngineNewViewRequest::destination() const
+{
+    return m_destination;
+}
+
+bool QQuickWebEngineNewViewRequest::isPopup() const
+{
+    return m_isPopup;
+}
+
+void QQuickWebEngineNewViewRequest::openIn(QQuickWebEngineView *view)
+{
+    if (view) {
+        view->d_func()->adoptWebContents(m_adapter.data());
+        m_adapter.reset();
+    }
+}
diff --git a/src/webengine/api/qquickwebenginenewviewrequest_p.h b/src/webengine/api/qquickwebenginenewviewrequest_p.h
new file mode 100644
index 000000000..ed762cd39
--- /dev/null
+++ b/src/webengine/api/qquickwebenginenewviewrequest_p.h
@@ -0,0 +1,75 @@
+/****************************************************************************
+**
+** Copyright (C) 2013 Digia Plc and/or its subsidiary(-ies).
+** Contact: http://www.qt-project.org/legal
+**
+** This file is part of the QtWebEngine module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and Digia.  For licensing terms and
+** conditions see http://qt.digia.com/licensing.  For further information
+** use the contact form at http://qt.digia.com/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Digia gives you certain additional
+** rights.  These rights are described in the Digia Qt LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3.0 as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU General Public License version 3.0 requirements will be
+** met: http://www.gnu.org/copyleft/gpl.html.
+**
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef QQUICKWEBENGINENEWVIEWREQUEST_P_H
+#define QQUICKWEBENGINENEWVIEWREQUEST_P_H
+
+#include "qtwebengineglobal_p.h"
+#include "qquickwebengineview_p.h"
+
+class WebContentsAdapter;
+
+QT_BEGIN_NAMESPACE
+
+class Q_WEBENGINE_PRIVATE_EXPORT QQuickWebEngineNewViewRequest : public QObject {
+    Q_OBJECT
+    Q_PROPERTY(QQuickWebEngineView::NewViewDestination destination READ destination CONSTANT FINAL)
+    Q_PROPERTY(bool popup READ isPopup CONSTANT FINAL)
+public:
+    ~QQuickWebEngineNewViewRequest();
+
+    QQuickWebEngineView::NewViewDestination destination() const;
+    bool isPopup() const;
+    Q_INVOKABLE void openIn(QQuickWebEngineView *view);
+
+private:
+    QQuickWebEngineNewViewRequest();
+    QQuickWebEngineView::NewViewDestination m_destination;
+    bool m_isPopup;
+    QExplicitlySharedDataPointer<WebContentsAdapter> m_adapter;
+    friend class QQuickWebEngineViewPrivate;
+};
+
+QT_END_NAMESPACE
+
+QML_DECLARE_TYPE(QQuickWebEngineNewViewRequest)
+
+#endif // QQUICKWEBENGINENEWVIEWREQUEST_P_H
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index 6693c8c79..727b5661b 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -44,6 +44,7 @@
 
 #include "javascript_dialog_controller.h"
 #include "qquickwebengineloadrequest_p.h"
+#include "qquickwebenginenewviewrequest_p.h"
 #include "render_widget_host_view_qt_delegate_quick.h"
 #include "ui_delegates_manager.h"
 #include "web_contents_adapter.h"
@@ -280,48 +281,28 @@ void QQuickWebEngineViewPrivate::focusContainer()
 
 void QQuickWebEngineViewPrivate::adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, const QRect &)
 {
-    Q_Q(QQuickWebEngineView);
-    QQmlEngine *engine = QtQml::qmlEngine(q);
-    // This is currently only supported for QML instantiated WebEngineViews.
-    // We could emit a QObject* and set JavaScriptOwnership explicitly on it
-    // but this would make the signal cumbersome to use in C++ where one, and
-    // only one, of the connected slots would have to destroy the given handle.
-    // A virtual method instead of a signal would work better in this case.
-    if (!engine)
-        return;
-    static const QMetaMethod createWindowSignal = QMetaMethod::fromSignal(&QQuickWebEngineViewExperimental::createWindow);
-    if (!e->isSignalConnected(createWindowSignal))
-        return;
-
-    QQuickWebEngineViewHandle *handle = new QQuickWebEngineViewHandle;
+    QQuickWebEngineNewViewRequest request;
     // This increases the ref-count of newWebContents and will tell Chromium
     // to start loading it and possibly return it to its parent page window.open().
-    handle->adapter = newWebContents;
-    // Clearly mark our wrapper as owned by JavaScript, we then depend on it
-    // being adopted or else eventually cleaned up by the GC.
-    QJSValue jsHandle = engine->newQObject(handle);
+    request.m_adapter = newWebContents;
+    request.m_isPopup = false;
 
-    QString dispositionString;
     switch (disposition) {
+    case WebContentsAdapterClient::NewPopupDisposition:
+        request.m_isPopup = true;
+        // fall through
     case WebContentsAdapterClient::NewForegroundTabDisposition:
     case WebContentsAdapterClient::NewBackgroundTabDisposition:
-        dispositionString = QStringLiteral("tab");
-        break;
-    case WebContentsAdapterClient::NewPopupDisposition:
-        dispositionString = QStringLiteral("popup");
+        request.m_destination = QQuickWebEngineView::NewViewInTab;
         break;
     case WebContentsAdapterClient::NewWindowDisposition:
-        dispositionString = QStringLiteral("window");
+        request.m_destination = QQuickWebEngineView::NewViewInWindow;
         break;
     default:
         Q_UNREACHABLE();
     }
 
-    emit e->createWindow(jsHandle, dispositionString);
-
-    // We currently require the adoption to happen within the signal handler to avoid having
-    // to support a null WebContentsAdapterClient for too long after having returned.
-    handle->adapter.reset();
+    emit e->newViewRequested(&request);
 }
 
 void QQuickWebEngineViewPrivate::close()
@@ -353,6 +334,32 @@ void QQuickWebEngineViewPrivate::setDevicePixelRatio(qreal devicePixelRatio)
     m_dpiScale = devicePixelRatio / screen->devicePixelRatio();
 }
 
+void QQuickWebEngineViewPrivate::adoptWebContents(WebContentsAdapter *webContents)
+{
+    if (!webContents) {
+        qWarning("Trying to open an empty request, it was either already used or was invalidated."
+            "\nYou must complete the request synchronously within the newViewRequested signal handler."
+            " If a view hasn't been adopted before returning, the request will be invalidated.");
+        return;
+    }
+
+    Q_Q(QQuickWebEngineView);
+    // This throws away the WebContentsAdapter that has been used until now.
+    // All its states, particularly the loading URL, are replaced by the adopted WebContentsAdapter.
+    adapter = webContents;
+    adapter->initialize(this);
+
+    // Emit signals for values that might be different from the previous WebContentsAdapter.
+    emit q->titleChanged();
+    emit q->urlChanged();
+    emit q->iconChanged();
+    // FIXME: The current loading state should be stored in the WebContentAdapter
+    // and it should be checked here if the signal emission is really necessary.
+    QQuickWebEngineLoadRequest loadRequest(adapter->activeUrl(), QQuickWebEngineView::LoadSucceededStatus);
+    emit q->loadingStateChanged(&loadRequest);
+    emit q->loadProgressChanged();
+}
+
 QQuickWebEngineView::QQuickWebEngineView(QQuickItem *parent)
     : QQuickItem(*(new QQuickWebEngineViewPrivate), parent)
 {
@@ -509,14 +516,6 @@ void QQuickWebEngineView::itemChange(ItemChange change, const ItemChangeData &va
     QQuickItem::itemChange(change, value);
 }
 
-QQuickWebEngineViewHandle::QQuickWebEngineViewHandle()
-{
-}
-
-QQuickWebEngineViewHandle::~QQuickWebEngineViewHandle()
-{
-}
-
 QQuickWebEngineViewExperimental::QQuickWebEngineViewExperimental(QQuickWebEngineViewPrivate *viewPrivate)
     : q_ptr(0)
     , d_ptr(viewPrivate)
@@ -552,36 +551,6 @@ void QQuickWebEngineViewport::setDevicePixelRatio(qreal devicePixelRatio)
     Q_EMIT devicePixelRatioChanged();
 }
 
-void QQuickWebEngineViewExperimental::adoptHandle(QQuickWebEngineViewHandle *viewHandle)
-{
-    if (!viewHandle || !viewHandle->adapter) {
-        qWarning("Trying to adopt an empty handle, it was either already adopted or was invalidated."
-            "\nYou must do the adoption synchronously within the createWindow signal handler."
-            " If the handle hasn't been adopted before returning, it will be invalidated.");
-        return;
-    }
-
-    Q_Q(QQuickWebEngineView);
-    Q_D(QQuickWebEngineView);
-
-    // This throws away the WebContentsAdapter that has been used until now.
-    // All its states, particularly the loading URL, are replaced by the adopted WebContentsAdapter.
-    d->adapter = viewHandle->adapter;
-    viewHandle->adapter.reset();
-
-    d->adapter->initialize(d);
-
-    // Emit signals for values that might be different from the previous WebContentsAdapter.
-    emit q->titleChanged();
-    emit q->urlChanged();
-    emit q->iconChanged();
-    // FIXME: The current loading state should be stored in the WebContentAdapter
-    // and it should be checked here if the signal emission is really necessary.
-    QQuickWebEngineLoadRequest loadRequest(d->adapter->activeUrl(), QQuickWebEngineView::LoadSucceededStatus);
-    emit q->loadingStateChanged(&loadRequest);
-    emit q->loadProgressChanged();
-}
-
 QT_END_NAMESPACE
 
 #include "moc_qquickwebengineview_p.cpp"
diff --git a/src/webengine/api/qquickwebengineview_p.h b/src/webengine/api/qquickwebengineview_p.h
index 77d5b9828..66dc37622 100644
--- a/src/webengine/api/qquickwebengineview_p.h
+++ b/src/webengine/api/qquickwebengineview_p.h
@@ -62,6 +62,7 @@ class Q_WEBENGINE_PRIVATE_EXPORT QQuickWebEngineView : public QQuickItem {
     Q_PROPERTY(bool inspectable READ inspectable WRITE setInspectable)
     Q_ENUMS(LoadStatus);
     Q_ENUMS(ErrorDomain);
+    Q_ENUMS(NewViewDestination);
 
 public:
     QQuickWebEngineView(QQuickItem *parent = 0);
@@ -95,6 +96,11 @@ public:
          DnsErrorDomain
     };
 
+    enum NewViewDestination {
+        NewViewInWindow,
+        NewViewInTab
+    };
+
 public Q_SLOTS:
     void loadHtml(const QString &html, const QUrl &baseUrl = QUrl(), const QUrl &unreachableUrl = QUrl());
     void goBack();
@@ -118,6 +124,7 @@ private:
     Q_DECLARE_PRIVATE(QQuickWebEngineView)
     friend class QQuickWebEngineViewExperimental;
     friend class QQuickWebEngineViewExperimentalExtension;
+    friend class QQuickWebEngineNewViewRequest;
 };
 
 QT_END_NAMESPACE
diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h
index c7fecd477..4e0e8115d 100644
--- a/src/webengine/api/qquickwebengineview_p_p.h
+++ b/src/webengine/api/qquickwebengineview_p_p.h
@@ -55,6 +55,7 @@ class WebContentsAdapter;
 class UIDelegatesManager;
 
 QT_BEGIN_NAMESPACE
+class QQuickWebEngineNewViewRequest;
 class QQuickWebEngineView;
 class QQmlComponent;
 class QQmlContext;
@@ -77,18 +78,6 @@ private:
     Q_DECLARE_PRIVATE(QQuickWebEngineView)
 };
 
-class Q_WEBENGINE_PRIVATE_EXPORT QQuickWebEngineViewHandle : public QObject {
-    Q_OBJECT
-public:
-    QQuickWebEngineViewHandle();
-    ~QQuickWebEngineViewHandle();
-
-private:
-    QExplicitlySharedDataPointer<WebContentsAdapter> adapter;
-    friend class QQuickWebEngineViewExperimental;
-    friend class QQuickWebEngineViewPrivate;
-};
-
 class Q_WEBENGINE_PRIVATE_EXPORT QQuickWebEngineViewExperimental : public QObject {
     Q_OBJECT
     Q_PROPERTY(QQuickWebEngineViewport *viewport READ viewport)
@@ -101,12 +90,11 @@ public:
 
 public Q_SLOTS:
     QQuickWebEngineViewport *viewport() const;
-    Q_INVOKABLE void adoptHandle(QQuickWebEngineViewHandle *viewHandle);
     void setExtraContextMenuEntriesComponent(QQmlComponent *);
     QQmlComponent *extraContextMenuEntriesComponent() const;
 
 Q_SIGNALS:
-    void createWindow(const QJSValue &newViewHandle, const QString &newViewDisposition);
+    void newViewRequested(QQuickWebEngineNewViewRequest *request);
     void fullScreenRequested(bool fullScreen);
     void isFullScreenChanged();
     void extraContextMenuEntriesComponentChanged();
@@ -156,6 +144,7 @@ public:
     virtual void javaScriptConsoleMessage(int level, const QString& message, int lineNumber, const QString& sourceID) Q_DECL_OVERRIDE;
 
     void setDevicePixelRatio(qreal);
+    void adoptWebContents(WebContentsAdapter *webContents);
 
     QExplicitlySharedDataPointer<WebContentsAdapter> adapter;
     QScopedPointer<QQuickWebEngineViewExperimental> e;
diff --git a/src/webengine/plugin/plugin.cpp b/src/webengine/plugin/plugin.cpp
index dd36bc6b7..2408e5c8e 100644
--- a/src/webengine/plugin/plugin.cpp
+++ b/src/webengine/plugin/plugin.cpp
@@ -43,6 +43,7 @@
 
 #include "qquickwebengineview_p.h"
 #include "qquickwebengineloadrequest_p.h"
+#include "qquickwebenginenewviewrequest_p.h"
 
 QT_BEGIN_NAMESPACE
 
@@ -57,6 +58,7 @@ public:
 
         qmlRegisterType<QQuickWebEngineView>(uri, 1, 0, "WebEngineView");
         qmlRegisterUncreatableType<QQuickWebEngineLoadRequest>(uri, 1, 0, "WebEngineLoadRequest", QObject::tr("Cannot create separate instance of WebEngineLoadRequest"));
+        qmlRegisterUncreatableType<QQuickWebEngineNewViewRequest>(uri, 1, 0, "WebEngineNewViewRequest", QObject::tr("Cannot create separate instance of WebEngineNewViewRequest"));
     }
 };
 
diff --git a/src/webengine/webengine.pro b/src/webengine/webengine.pro
index 877c515a9..dc95844a7 100644
--- a/src/webengine/webengine.pro
+++ b/src/webengine/webengine.pro
@@ -21,6 +21,7 @@ QMAKE_RPATHDIR += $$LIBPATH
 
 SOURCES = \
         api/qquickwebengineloadrequest.cpp \
+        api/qquickwebenginenewviewrequest.cpp \
         api/qquickwebengineview.cpp \
         render_widget_host_view_qt_delegate_quick.cpp \
         ui_delegates_manager.cpp
@@ -29,6 +30,7 @@ HEADERS = \
         api/qtwebengineglobal.h \
         api/qtwebengineglobal_p.h \
         api/qquickwebengineloadrequest_p.h \
+        api/qquickwebenginenewviewrequest_p.h \
         api/qquickwebengineview_p.h \
         api/qquickwebengineview_p_p.h \
         render_widget_host_view_qt_delegate_quick.h \