From 80d73da6e89dc06e2d4a16df14df3e2a1119cce1 Mon Sep 17 00:00:00 2001
From: Szabolcs David <davidsz@inf.u-szeged.hu>
Date: Tue, 25 Sep 2018 12:57:35 +0200
Subject: [PATCH] Unify updating navigation actions

On the Quick side, navigation actions were dependent on the context menu.
They were only updated when requesting a new context menu and this is
obviously wrong if an action is tied to a button or another type of UI
element.

Change-Id: I5f14b019b66215f16d027fb57d76f052b1604365
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
 src/core/web_contents_adapter_client.h      |  1 +
 src/core/web_contents_delegate_qt.cpp       | 23 +++++++++++++--------
 src/core/web_contents_delegate_qt.h         |  1 +
 src/webengine/api/qquickwebengineview.cpp   |  9 ++++++++
 src/webengine/api/qquickwebengineview_p_p.h |  1 +
 src/webenginewidgets/api/qwebenginepage.cpp |  7 -------
 src/webenginewidgets/api/qwebenginepage_p.h |  4 ++--
 7 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/src/core/web_contents_adapter_client.h b/src/core/web_contents_adapter_client.h
index 6fd29ec61..514d3afb4 100644
--- a/src/core/web_contents_adapter_client.h
+++ b/src/core/web_contents_adapter_client.h
@@ -465,6 +465,7 @@ public:
     virtual void selectClientCert(const QSharedPointer<ClientCertSelectController> &selectController) = 0;
     virtual void updateScrollPosition(const QPointF &position) = 0;
     virtual void updateContentsSize(const QSizeF &size) = 0;
+    virtual void updateNavigationActions() = 0;
     virtual void startDragging(const content::DropData &dropData, Qt::DropActions allowedActions,
                                const QPixmap &pixmap, const QPoint &offset) = 0;
     virtual bool supportsDragging() const = 0;
diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp
index 569b939d8..9472c0be9 100644
--- a/src/core/web_contents_delegate_qt.cpp
+++ b/src/core/web_contents_delegate_qt.cpp
@@ -263,6 +263,7 @@ void WebContentsDelegateQt::EmitLoadStarted(const QUrl &url, bool isErrorPage)
     if (m_lastLoadProgress >= 0 && m_lastLoadProgress < 100) // already running
         return;
     m_viewClient->loadStarted(url, isErrorPage);
+    m_viewClient->updateNavigationActions();
     m_viewClient->loadProgressChanged(0);
     m_lastLoadProgress = 0;
 }
@@ -287,6 +288,16 @@ void WebContentsDelegateQt::EmitLoadFinished(bool success, const QUrl &url, bool
     m_lastLoadProgress = -1;
     m_viewClient->loadProgressChanged(100);
     m_viewClient->loadFinished(success, url, isErrorPage, errorCode, errorDescription);
+    m_viewClient->updateNavigationActions();
+}
+
+void WebContentsDelegateQt::EmitLoadCommitted()
+{
+    // Make sure that we don't set the findNext WebFindOptions on a new frame.
+    m_lastSearchedString = QString();
+
+    m_viewClient->loadCommitted();
+    m_viewClient->updateNavigationActions();
 }
 
 void WebContentsDelegateQt::DidFinishNavigation(content::NavigationHandle *navigation_handle)
@@ -302,11 +313,7 @@ void WebContentsDelegateQt::DidFinishNavigation(content::NavigationHandle *navig
                 profileAdapter->visitedLinksManager()->addUrl(url);
         }
 
-        // Make sure that we don't set the findNext WebFindOptions on a new frame.
-        m_lastSearchedString = QString();
-
-        // This is currently used for canGoBack/Forward values, which is flattened across frames. For other purposes we might have to pass is_main_frame.
-        m_viewClient->loadCommitted();
+        EmitLoadCommitted();
     }
     // Success is reported by DidFinishLoad, but DidFailLoad is now dead code and needs to be handled below
     if (navigation_handle->GetNetErrorCode() == net::OK)
@@ -325,10 +332,8 @@ void WebContentsDelegateQt::DidFinishNavigation(content::NavigationHandle *navig
         EmitLoadStarted(toQt(GURL(content::kUnreachableWebDataURL)), true);
 
         // If it is already committed we will not see another DidFinishNavigation call or a DidFinishLoad call.
-        if (navigation_handle->HasCommitted()) {
-            m_lastSearchedString = QString();
-            m_viewClient->loadCommitted();
-        }
+        if (navigation_handle->HasCommitted())
+            EmitLoadCommitted();
     }
 }
 
diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h
index 124250a40..966494ec7 100644
--- a/src/core/web_contents_delegate_qt.h
+++ b/src/core/web_contents_delegate_qt.h
@@ -159,6 +159,7 @@ private:
     QWeakPointer<WebContentsAdapter> createWindow(std::unique_ptr<content::WebContents> new_contents, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture);
     void EmitLoadStarted(const QUrl &url, bool isErrorPage = false);
     void EmitLoadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString());
+    void EmitLoadCommitted();
 
     WebContentsAdapterClient *m_viewClient;
     QString m_lastSearchedString;
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index 116f49c3c..a3a12819b 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -878,6 +878,15 @@ void QQuickWebEngineViewPrivate::updateAction(QQuickWebEngineView::WebAction act
     a->d_ptr->setEnabled(enabled);
 }
 
+void QQuickWebEngineViewPrivate::updateNavigationActions()
+{
+    updateAction(QQuickWebEngineView::Back);
+    updateAction(QQuickWebEngineView::Forward);
+    updateAction(QQuickWebEngineView::Stop);
+    updateAction(QQuickWebEngineView::Reload);
+    updateAction(QQuickWebEngineView::ReloadAndBypassCache);
+    updateAction(QQuickWebEngineView::ViewSource);
+}
 
 QUrl QQuickWebEngineView::url() const
 {
diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h
index ee38ece6b..d20bfca46 100644
--- a/src/webengine/api/qquickwebengineview_p_p.h
+++ b/src/webengine/api/qquickwebengineview_p_p.h
@@ -145,6 +145,7 @@ public:
     void requestGeometryChange(const QRect &geometry, const QRect &frameGeometry) override;
     void updateScrollPosition(const QPointF &position) override;
     void updateContentsSize(const QSizeF &size) override;
+    void updateNavigationActions() override;
     void startDragging(const content::DropData &dropData, Qt::DropActions allowedActions,
                        const QPixmap &pixmap, const QPoint &offset) override;
     bool supportsDragging() const override;
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index 953cb4168..3a2a07520 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -368,12 +368,6 @@ void QWebEnginePagePrivate::loadStarted(const QUrl &provisionalUrl, bool isError
 
     isLoading = true;
     QTimer::singleShot(0, q, &QWebEnginePage::loadStarted);
-    updateNavigationActions();
-}
-
-void QWebEnginePagePrivate::loadCommitted()
-{
-    updateNavigationActions();
 }
 
 void QWebEnginePagePrivate::loadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, const QString &errorDescription)
@@ -401,7 +395,6 @@ void QWebEnginePagePrivate::loadFinished(bool success, const QUrl &url, bool isE
             emit q->loadFinished(success);
         });
     }
-    updateNavigationActions();
 }
 
 void QWebEnginePagePrivate::didPrintPageToPdf(const QString &filePath, bool success)
diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h
index 66a92dec9..dc8a34af6 100644
--- a/src/webenginewidgets/api/qwebenginepage_p.h
+++ b/src/webenginewidgets/api/qwebenginepage_p.h
@@ -100,7 +100,7 @@ public:
     qreal dpiScale() const override;
     QColor backgroundColor() const override;
     void loadStarted(const QUrl &provisionalUrl, bool isErrorPage = false) override;
-    void loadCommitted() override;
+    void loadCommitted() override { }
     void loadVisuallyCommitted() override { }
     void loadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString()) override;
     void focusContainer() override;
@@ -141,6 +141,7 @@ public:
     void requestGeometryChange(const QRect &geometry, const QRect &frameGeometry) override;
     void updateScrollPosition(const QPointF &position) override;
     void updateContentsSize(const QSizeF &size) override;
+    void updateNavigationActions() override;
     void startDragging(const content::DropData &dropData, Qt::DropActions allowedActions,
                        const QPixmap &pixmap, const QPoint &offset) override;
     bool supportsDragging() const override;
@@ -154,7 +155,6 @@ public:
     QtWebEngineCore::WebContentsAdapter *webContentsAdapter() override;
 
     void updateAction(QWebEnginePage::WebAction) const;
-    void updateNavigationActions();
     void _q_webActionTriggered(bool checked);
 
     void wasShown();
-- 
GitLab