Fix how NewPopupDisposition is exposed in QQuickWebEngineView

I initially misintepreted the meaning of the enum, assuming that it
meant that the window should possibly be blocked. The user_gesture
parameter in WebContentsDelegate::AddNewContents is actually doing
this, while the popup disposition means that JavaScript requested
the window to be opened without one of the standard decoration (i.e.
status bar, menu bar, tool bar, etc.).

Update the QtQuick API to reflect this, renaming the "isPopup"
parameter to a more familiar "isUserInitiated".

The popup disposition is named "dialog" to match the previous
QWebPage::WebModalDialog enum.

Change-Id: Ib0c4bc53671fcf0dd9499aa1be2bbc8c494ba49e
Reviewed-by: Michael Bruning <michael.bruning@digia.com>

10 files changed, 36 insertions, 17 deletions

diff --git a/src/core/web_contents_adapter_client.h b/src/core/web_contents_adapter_client.h
index f24629234..7deb55686 100644
--- a/src/core/web_contents_adapter_client.h
+++ b/src/core/web_contents_adapter_client.h
@@ -129,7 +129,7 @@ public:
     virtual void loadCommitted() = 0;
     virtual void loadFinished(bool success, int error_code = 0, const QString &error_description = QString()) = 0;
     virtual void focusContainer() = 0;
-    virtual void adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, const QRect & initialGeometry) = 0;
+    virtual void adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect & initialGeometry) = 0;
     virtual void close() = 0;
     virtual bool contextMenuRequested(const WebEngineContextMenuData&) = 0;
     virtual void requestFullScreen(bool) = 0;
diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp
index dba63296b..8c5646ab5 100644
--- a/src/core/web_contents_delegate_qt.cpp
+++ b/src/core/web_contents_delegate_qt.cpp
@@ -86,7 +86,7 @@ void WebContentsDelegateQt::AddNewContents(content::WebContents* source, content
     // Do the first ref-count manually to be able to know if the application is handling adoptNewWindow through the public API.
     newAdapter->ref.ref();
 
-    m_viewClient->adoptNewWindow(newAdapter, static_cast<WebContentsAdapterClient::WindowOpenDisposition>(disposition), toQt(initial_pos));
+    m_viewClient->adoptNewWindow(newAdapter, static_cast<WebContentsAdapterClient::WindowOpenDisposition>(disposition), user_gesture, toQt(initial_pos));
 
     if (!newAdapter->ref.deref()) {
         // adoptNewWindow didn't increase the ref-count, new_contents needs to be discarded.
diff --git a/src/webengine/api/qquickwebenginenewviewrequest.cpp b/src/webengine/api/qquickwebenginenewviewrequest.cpp
index 8af8f5b49..2aa45228c 100644
--- a/src/webengine/api/qquickwebenginenewviewrequest.cpp
+++ b/src/webengine/api/qquickwebenginenewviewrequest.cpp
@@ -57,9 +57,9 @@ QQuickWebEngineView::NewViewDestination QQuickWebEngineNewViewRequest::destinati
     return m_destination;
 }
 
-bool QQuickWebEngineNewViewRequest::isPopup() const
+bool QQuickWebEngineNewViewRequest::isUserInitiated() const
 {
-    return m_isPopup;
+    return m_isUserInitiated;
 }
 
 void QQuickWebEngineNewViewRequest::openIn(QQuickWebEngineView *view)
diff --git a/src/webengine/api/qquickwebenginenewviewrequest_p.h b/src/webengine/api/qquickwebenginenewviewrequest_p.h
index ed762cd39..390ca278b 100644
--- a/src/webengine/api/qquickwebenginenewviewrequest_p.h
+++ b/src/webengine/api/qquickwebenginenewviewrequest_p.h
@@ -52,18 +52,18 @@ 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)
+    Q_PROPERTY(bool userInitiated READ isUserInitiated CONSTANT FINAL)
 public:
     ~QQuickWebEngineNewViewRequest();
 
     QQuickWebEngineView::NewViewDestination destination() const;
-    bool isPopup() const;
+    bool isUserInitiated() const;
     Q_INVOKABLE void openIn(QQuickWebEngineView *view);
 
 private:
     QQuickWebEngineNewViewRequest();
     QQuickWebEngineView::NewViewDestination m_destination;
-    bool m_isPopup;
+    bool m_isUserInitiated;
     QExplicitlySharedDataPointer<WebContentsAdapter> m_adapter;
     friend class QQuickWebEngineViewPrivate;
 };
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index c8c8bfac0..e3838255e 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -295,22 +295,22 @@ void QQuickWebEngineViewPrivate::focusContainer()
     q->forceActiveFocus();
 }
 
-void QQuickWebEngineViewPrivate::adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, const QRect &)
+void QQuickWebEngineViewPrivate::adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &)
 {
     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().
     request.m_adapter = newWebContents;
-    request.m_isPopup = false;
+    request.m_isUserInitiated = userGesture;
 
     switch (disposition) {
-    case WebContentsAdapterClient::NewPopupDisposition:
-        request.m_isPopup = true;
-        // fall through
     case WebContentsAdapterClient::NewForegroundTabDisposition:
     case WebContentsAdapterClient::NewBackgroundTabDisposition:
         request.m_destination = QQuickWebEngineView::NewViewInTab;
         break;
+    case WebContentsAdapterClient::NewPopupDisposition:
+        request.m_destination = QQuickWebEngineView::NewViewInDialog;
+        break;
     case WebContentsAdapterClient::NewWindowDisposition:
         request.m_destination = QQuickWebEngineView::NewViewInWindow;
         break;
diff --git a/src/webengine/api/qquickwebengineview_p.h b/src/webengine/api/qquickwebengineview_p.h
index 40bc59f4e..d9284e739 100644
--- a/src/webengine/api/qquickwebengineview_p.h
+++ b/src/webengine/api/qquickwebengineview_p.h
@@ -100,7 +100,8 @@ public:
 
     enum NewViewDestination {
         NewViewInWindow,
-        NewViewInTab
+        NewViewInTab,
+        NewViewInDialog
     };
 
     // must match WebContentsAdapterClient::JavaScriptConsoleMessageLevel
diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h
index 3a84eae5c..4cbca6594 100644
--- a/src/webengine/api/qquickwebengineview_p_p.h
+++ b/src/webengine/api/qquickwebengineview_p_p.h
@@ -139,7 +139,7 @@ public:
     virtual void loadCommitted() Q_DECL_OVERRIDE;
     virtual void loadFinished(bool success, int error_code = 0, const QString &error_description = QString()) Q_DECL_OVERRIDE;
     virtual void focusContainer() Q_DECL_OVERRIDE;
-    virtual void adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, const QRect &) Q_DECL_OVERRIDE;
+    virtual void adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &) Q_DECL_OVERRIDE;
     virtual void close() Q_DECL_OVERRIDE;
     virtual void requestFullScreen(bool) Q_DECL_OVERRIDE;
     virtual bool isFullScreen() const Q_DECL_OVERRIDE;
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index 70f2097a0..04f24c62a 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -241,9 +241,10 @@ void QWebEnginePagePrivate::focusContainer()
         view->setFocus();
 }
 
-void QWebEnginePagePrivate::adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, const QRect &initialGeometry)
+void QWebEnginePagePrivate::adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry)
 {
     Q_Q(QWebEnginePage);
+    Q_UNUSED(userGesture);
     QWebEnginePage *newPage = q->createWindow(disposition == WebContentsAdapterClient::NewPopupDisposition ? QWebEnginePage::WebModalDialog : QWebEnginePage::WebBrowserWindow);
     // Overwrite the new page's WebContents with ours.
     if (newPage && newPage->d_func() != this) {
diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h
index b24489385..ddc7c51e7 100644
--- a/src/webenginewidgets/api/qwebenginepage_p.h
+++ b/src/webenginewidgets/api/qwebenginepage_p.h
@@ -122,7 +122,7 @@ public:
     virtual void loadCommitted() Q_DECL_OVERRIDE;
     virtual void loadFinished(bool success, int error_code, const QString &error_description = QString()) Q_DECL_OVERRIDE;
     virtual void focusContainer() Q_DECL_OVERRIDE;
-    virtual void adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, const QRect &initialGeometry) Q_DECL_OVERRIDE;
+    virtual void adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry) Q_DECL_OVERRIDE;
     virtual void close() Q_DECL_OVERRIDE;
     virtual bool contextMenuRequested(const WebEngineContextMenuData &data) Q_DECL_OVERRIDE;
     virtual void requestFullScreen(bool) Q_DECL_OVERRIDE { };
diff --git a/tests/quicktestbrowser/quickwindow.qml b/tests/quicktestbrowser/quickwindow.qml
index 891ab5792..f77fec582 100644
--- a/tests/quicktestbrowser/quickwindow.qml
+++ b/tests/quicktestbrowser/quickwindow.qml
@@ -235,6 +235,20 @@ ApplicationWindow {
         Component.onCompleted: createEmptyTab()
 
         Component {
+            id: dialogComponent
+            Window {
+                property Item webView: _webView
+                width: 800
+                height: 600
+                visible: true
+                WebEngineView {
+                    id: _webView
+                    anchors.fill: parent
+                }
+            }
+        }
+
+        Component {
             id: tabComponent
             WebEngineView {
                 id: webEngineView
@@ -268,11 +282,14 @@ ApplicationWindow {
                     }
 
                     onNewViewRequested: {
-                        if (request.popup)
+                        if (!request.userInitiated)
                             print("Warning: Blocked a popup window.")
                         else if (request.destination == WebEngineView.NewViewInTab) {
                             var tab = tabs.createEmptyTab()
                             request.openIn(tab.item)
+                        } else if (request.destination == WebEngineView.NewViewInDialog) {
+                            var dialog = dialogComponent.createObject()
+                            request.openIn(dialog.webView)
                         } else {
                             var component = Qt.createComponent("quickwindow.qml")
                             var window = component.createObject()