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>

1 files changed, 36 insertions, 67 deletions

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"