Base load signals on Blink loader events

This mainly remove the use of the LoadingStateChanged callback, which
is tied to DidStartLoading and DidStopLoading.
Those signals are handled from the browser process side, also wrapping
the time where the render process is initialized. We can't rely on
those signals for loadStarted, but afterward rely on the Blink loader
for loadFinished. We must use the same source for both.

Instead only rely on Blink callbacks ultimately related to network
events. This gives us a behavior closer to QtWebKit.

The major compromise that this forces us to to accept is that
loadStarted is now triggered asynchronously. This will basically break
anything expecting loadStarted to be emitted synchronously from the
load method.

This also adjust autotests to get a few more passing.

Initial-patch-by: Pierre Rossi <pierre.rossi@digia.com>
Change-Id: Ib6c0170df891d1b7f8ed4dc1d483985523e267dc
Reviewed-by: Pierre Rossi <pierre.rossi@gmail.com>
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

9 files changed, 71 insertions, 55 deletions

diff --git a/src/core/web_contents_adapter_client.h b/src/core/web_contents_adapter_client.h
index e436fd61d..0cb4e3d10 100644
--- a/src/core/web_contents_adapter_client.h
+++ b/src/core/web_contents_adapter_client.h
@@ -119,11 +119,12 @@ public:
     virtual void titleChanged(const QString&) = 0;
     virtual void urlChanged(const QUrl&) = 0;
     virtual void iconChanged(const QUrl&) = 0;
-    virtual void loadingStateChanged() = 0;
     virtual void loadProgressChanged(int progress) = 0;
     virtual void selectionChanged() = 0;
     virtual QRectF viewportRect() const = 0;
     virtual qreal dpiScale() const = 0;
+    virtual void loadStarted(const QUrl &provisionalUrl) = 0;
+    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;
diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp
index c0b050de6..e57ee4890 100644
--- a/src/core/web_contents_delegate_qt.cpp
+++ b/src/core/web_contents_delegate_qt.cpp
@@ -89,32 +89,38 @@ void WebContentsDelegateQt::CloseContents(content::WebContents *source)
     GetJavaScriptDialogManager()->CancelActiveAndPendingDialogs(source);
 }
 
-void WebContentsDelegateQt::LoadingStateChanged(content::WebContents* source)
-{
-    m_viewClient->loadingStateChanged();
-}
-
 void WebContentsDelegateQt::LoadProgressChanged(content::WebContents* source, double progress)
 {
     m_viewClient->loadProgressChanged(qRound(progress * 100));
 }
 
-void WebContentsDelegateQt::DidFailLoad(int64 frame_id, const GURL &validated_url, bool is_main_frame, int error_code, const string16 &error_description, content::RenderViewHost *render_view_host)
+void WebContentsDelegateQt::DidStartProvisionalLoadForFrame(int64, int64, bool is_main_frame, const GURL &validated_url, bool, bool, content::RenderViewHost*)
 {
     if (is_main_frame)
-        m_viewClient->loadFinished(false, error_code, toQt(error_description));
+        m_viewClient->loadStarted(toQt(validated_url));
+}
+
+void WebContentsDelegateQt::DidCommitProvisionalLoadForFrame(int64, bool is_main_frame, const GURL& url, content::PageTransition transition_type, content::RenderViewHost *render_view_host)
+{
+    // 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();
 }
 
-void WebContentsDelegateQt::DidFinishLoad(int64 frame_id, const GURL &validated_url, bool is_main_frame, content::RenderViewHost *render_view_host)
+void WebContentsDelegateQt::DidFailProvisionalLoad(int64 frame_id, bool is_main_frame, const GURL& validated_url, int error_code, const string16& error_description, content::RenderViewHost *render_view_host)
+{
+    DidFailLoad(frame_id, validated_url, is_main_frame, error_code, error_description, render_view_host);
+}
+
+void WebContentsDelegateQt::DidFailLoad(int64, const GURL&, bool is_main_frame, int error_code, const string16 &error_description, content::RenderViewHost*)
 {
     if (is_main_frame)
-        m_viewClient->loadFinished(true);
+        m_viewClient->loadFinished(false, error_code, toQt(error_description));
 }
 
-void WebContentsDelegateQt::DidFailProvisionalLoad(int64 frame_id, bool is_main_frame, const GURL& validated_url, int error_code, const string16& error_description, content::RenderViewHost* render_view_host)
+void WebContentsDelegateQt::DidFinishLoad(int64, const GURL&, bool is_main_frame, content::RenderViewHost*)
 {
     if (is_main_frame)
-        DidFailLoad(frame_id, validated_url, is_main_frame, error_code, error_description, render_view_host);
+        m_viewClient->loadFinished(true);
 }
 
 void WebContentsDelegateQt::DidUpdateFaviconURL(int32 page_id, const std::vector<content::FaviconURL>& candidates)
diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h
index 4c8701945..ac787f50c 100644
--- a/src/core/web_contents_delegate_qt.h
+++ b/src/core/web_contents_delegate_qt.h
@@ -66,12 +66,13 @@ public:
     virtual void NavigationStateChanged(const content::WebContents* source, unsigned changed_flags) Q_DECL_OVERRIDE;
     virtual void AddNewContents(content::WebContents* source, content::WebContents* new_contents, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture, bool* was_blocked) Q_DECL_OVERRIDE;
     virtual void CloseContents(content::WebContents *source) Q_DECL_OVERRIDE;
-    virtual void LoadingStateChanged(content::WebContents* source) Q_DECL_OVERRIDE;
     virtual void LoadProgressChanged(content::WebContents* source, double progress) Q_DECL_OVERRIDE;
+    virtual void DidStartProvisionalLoadForFrame(int64 frame_id, int64 parent_frame_id, bool is_main_frame, const GURL &validated_url, bool is_error_page, bool is_iframe_srcdoc, content::RenderViewHost *render_view_host) Q_DECL_OVERRIDE;
+    virtual void DidCommitProvisionalLoadForFrame(int64 frame_id, bool is_main_frame, const GURL &url, content::PageTransition transition_type, content::RenderViewHost *render_view_host) Q_DECL_OVERRIDE;
+    virtual void DidFailProvisionalLoad(int64 frame_id, bool is_main_frame, const GURL &validated_url, int error_code, const string16& error_description, content::RenderViewHost *render_view_host) Q_DECL_OVERRIDE;
     virtual void DidFailLoad(int64 frame_id, const GURL &validated_url, bool is_main_frame, int error_code, const string16 &error_description, content::RenderViewHost *render_view_host) Q_DECL_OVERRIDE;
     virtual void DidFinishLoad(int64 frame_id, const GURL &validated_url, bool is_main_frame, content::RenderViewHost *render_view_host) Q_DECL_OVERRIDE;
     virtual void DidUpdateFaviconURL(int32 page_id, const std::vector<content::FaviconURL>& candidates) Q_DECL_OVERRIDE;
-    virtual void DidFailProvisionalLoad(int64 frame_id, bool is_main_frame, const GURL& validated_url, int error_code, const string16& error_description, content::RenderViewHost* render_view_host) Q_DECL_OVERRIDE;
     virtual content::JavaScriptDialogManager *GetJavaScriptDialogManager() Q_DECL_OVERRIDE;
     virtual void ToggleFullscreenModeForTab(content::WebContents* web_contents, bool enter_fullscreen) Q_DECL_OVERRIDE;
     virtual bool IsFullscreenForTabOrPending(const content::WebContents* web_contents) const Q_DECL_OVERRIDE;
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index 58262cd3f..27282e111 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -72,7 +72,6 @@ QQuickWebEngineViewPrivate::QQuickWebEngineViewPrivate()
     , contextMenuExtraItems(0)
     , loadProgress(0)
     , inspectable(false)
-    , m_isLoading(false)
     , m_isFullScreen(false)
     , devicePixelRatio(QGuiApplication::primaryScreen()->devicePixelRatio())
     , m_dpiScale(1.0)
@@ -213,17 +212,6 @@ void QQuickWebEngineViewPrivate::iconChanged(const QUrl &url)
     Q_EMIT q->iconChanged();
 }
 
-void QQuickWebEngineViewPrivate::loadingStateChanged()
-{
-    Q_Q(QQuickWebEngineView);
-    const bool wasLoading = m_isLoading;
-    m_isLoading = adapter->isLoading();
-    if (m_isLoading && !wasLoading) {
-        QQuickWebEngineLoadRequest loadRequest(q->url(), QQuickWebEngineView::LoadStartedStatus);
-        Q_EMIT q->loadingChanged(&loadRequest);
-    }
-}
-
 void QQuickWebEngineViewPrivate::loadProgressChanged(int progress)
 {
     Q_Q(QQuickWebEngineView);
@@ -242,6 +230,13 @@ qreal QQuickWebEngineViewPrivate::dpiScale() const
     return m_dpiScale;
 }
 
+void QQuickWebEngineViewPrivate::loadStarted(const QUrl &provisionalUrl)
+{
+    Q_Q(QQuickWebEngineView);
+    QQuickWebEngineLoadRequest loadRequest(provisionalUrl, QQuickWebEngineView::LoadStartedStatus);
+    Q_EMIT q->loadingChanged(&loadRequest);
+}
+
 void QQuickWebEngineViewPrivate::loadFinished(bool success, int error_code, const QString &error_description)
 {
     Q_Q(QQuickWebEngineView);
diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h
index 8b489138c..564287114 100644
--- a/src/webengine/api/qquickwebengineview_p_p.h
+++ b/src/webengine/api/qquickwebengineview_p_p.h
@@ -131,11 +131,12 @@ public:
     virtual void titleChanged(const QString&) Q_DECL_OVERRIDE;
     virtual void urlChanged(const QUrl&) Q_DECL_OVERRIDE;
     virtual void iconChanged(const QUrl&) Q_DECL_OVERRIDE;
-    virtual void loadingStateChanged() Q_DECL_OVERRIDE;
     virtual void loadProgressChanged(int progress) Q_DECL_OVERRIDE;
     virtual void selectionChanged() Q_DECL_OVERRIDE { }
     virtual QRectF viewportRect() const Q_DECL_OVERRIDE;
     virtual qreal dpiScale() const Q_DECL_OVERRIDE;
+    virtual void loadStarted(const QUrl &provisionalUrl) Q_DECL_OVERRIDE;
+    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;
@@ -164,7 +165,6 @@ public:
     QUrl icon;
     int loadProgress;
     bool inspectable;
-    bool m_isLoading;
     bool m_isFullScreen;
     qreal devicePixelRatio;
     QMap<quint64, QJSValue> m_callbacks;
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index 4c4243ccc..a514882d6 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -151,7 +151,6 @@ QWebEnginePagePrivate::QWebEnginePagePrivate()
     , adapter(new WebContentsAdapter(SoftwareRenderingMode))
     , history(new QWebEngineHistory(new QWebEngineHistoryPrivate(adapter.data())))
     , view(0)
-    , m_isLoading(false)
 {
     memset(actions, 0, sizeof(actions));
 }
@@ -191,18 +190,6 @@ void QWebEnginePagePrivate::iconChanged(const QUrl &url)
     Q_UNUSED(url)
 }
 
-void QWebEnginePagePrivate::loadingStateChanged()
-{
-    Q_Q(QWebEnginePage);
-    const bool wasLoading = m_isLoading;
-    m_isLoading = adapter->isLoading();
-    if (m_isLoading != wasLoading) {
-        if (m_isLoading)
-            Q_EMIT q->loadStarted();
-    }
-    updateNavigationActions();
-}
-
 void QWebEnginePagePrivate::loadProgressChanged(int progress)
 {
     Q_Q(QWebEnginePage);
@@ -228,15 +215,28 @@ qreal QWebEnginePagePrivate::dpiScale() const
     return 1.0;
 }
 
+void QWebEnginePagePrivate::loadStarted(const QUrl &provisionalUrl)
+{
+    Q_UNUSED(provisionalUrl)
+    Q_Q(QWebEnginePage);
+    Q_EMIT q->loadStarted();
+    updateNavigationActions();
+}
+
+void QWebEnginePagePrivate::loadCommitted()
+{
+    updateNavigationActions();
+}
+
 void QWebEnginePagePrivate::loadFinished(bool success, int error_code, const QString &error_description)
 {
     Q_Q(QWebEnginePage);
     Q_UNUSED(error_code);
     Q_UNUSED(error_description);
-    m_isLoading = adapter->isLoading();
     if (success)
         m_explicitUrl = QUrl();
     Q_EMIT q->loadFinished(success);
+    updateNavigationActions();
 }
 
 void QWebEnginePagePrivate::focusContainer()
diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h
index 42f66cd01..0e06f1685 100644
--- a/src/webenginewidgets/api/qwebenginepage_p.h
+++ b/src/webenginewidgets/api/qwebenginepage_p.h
@@ -114,11 +114,12 @@ public:
     virtual void titleChanged(const QString&) Q_DECL_OVERRIDE;
     virtual void urlChanged(const QUrl&) Q_DECL_OVERRIDE;
     virtual void iconChanged(const QUrl&) Q_DECL_OVERRIDE;
-    virtual void loadingStateChanged() Q_DECL_OVERRIDE;
     virtual void loadProgressChanged(int progress) Q_DECL_OVERRIDE;
     virtual void selectionChanged() Q_DECL_OVERRIDE;
     virtual QRectF viewportRect() const Q_DECL_OVERRIDE;
     virtual qreal dpiScale() const Q_DECL_OVERRIDE;
+    virtual void loadStarted(const QUrl &provisionalUrl) Q_DECL_OVERRIDE;
+    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;
@@ -144,7 +145,6 @@ public:
     QWebEngineHistory *history;
     QWebEngineView *view;
     QSize viewportSize;
-    bool m_isLoading;
     QUrl m_explicitUrl;
     WebEngineContextMenuData m_menuData;
     QPointer<RenderWidgetHostViewQtDelegateWebPage> m_rwhvDelegate;
diff --git a/tests/auto/widgets/qwebengineframe/tst_qwebengineframe.cpp b/tests/auto/widgets/qwebengineframe/tst_qwebengineframe.cpp
index 52e422cb9..448f488f6 100644
--- a/tests/auto/widgets/qwebengineframe/tst_qwebengineframe.cpp
+++ b/tests/auto/widgets/qwebengineframe/tst_qwebengineframe.cpp
@@ -1516,7 +1516,7 @@ void tst_QWebEngineFrame::setUrlThenLoads()
     QSignalSpy finishedSpy(m_page, SIGNAL(loadFinished(bool)));
 
     m_page->setUrl(url);
-    QCOMPARE(startedSpy.count(), 1);
+    QTRY_COMPARE(startedSpy.count(), 1);
     QTRY_COMPARE(urlChangedSpy.count(), 1);
     QTRY_COMPARE(finishedSpy.count(), 1);
     QVERIFY(finishedSpy.at(0).first().toBool());
@@ -1529,12 +1529,12 @@ void tst_QWebEngineFrame::setUrlThenLoads()
 
     // Just after first load. URL didn't changed yet.
     m_page->load(urlToLoad1);
-    QTRY_COMPARE(startedSpy.count(), 2);
     QEXPECT_FAIL("", "Slight change: url() will return the loaded URL immediately.", Continue);
     QCOMPARE(m_page->url(), url);
     QCOMPARE(m_page->requestedUrl(), urlToLoad1);
     // baseUrlSync spins an event loop and this sometimes return the next result.
     // QCOMPARE(baseUrlSync(m_page), baseUrl);
+    QTRY_COMPARE(startedSpy.count(), 2);
 
     // After first URL changed.
     QTRY_COMPARE(urlChangedSpy.count(), 2);
@@ -1546,11 +1546,11 @@ void tst_QWebEngineFrame::setUrlThenLoads()
 
     // Just after second load. URL didn't changed yet.
     m_page->load(urlToLoad2);
-    QTRY_COMPARE(startedSpy.count(), 3);
     QEXPECT_FAIL("", "Slight change: url() will return the loaded URL immediately.", Continue);
     QCOMPARE(m_page->url(), urlToLoad1);
     QCOMPARE(m_page->requestedUrl(), urlToLoad2);
     QCOMPARE(baseUrlSync(m_page), extractBaseUrl(urlToLoad1));
+    QTRY_COMPARE(startedSpy.count(), 3);
 
     // After second URL changed.
     QTRY_COMPARE(urlChangedSpy.count(), 3);
diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
index bb07e2e00..7e1d811e8 100644
--- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
+++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
@@ -362,21 +362,23 @@ void tst_QWebEnginePage::geolocationRequestJS()
 
 void tst_QWebEnginePage::loadFinished()
 {
-    qRegisterMetaType<QNetworkRequest*>("QNetworkRequest*");
-    QSignalSpy spyLoadStarted(m_view, SIGNAL(loadStarted()));
-    QSignalSpy spyLoadFinished(m_view, SIGNAL(loadFinished(bool)));
+    QWebEnginePage page;
+    QSignalSpy spyLoadStarted(&page, SIGNAL(loadStarted()));
+    QSignalSpy spyLoadFinished(&page, SIGNAL(loadFinished(bool)));
 
-    m_view->page()->load(QUrl("data:text/html,<frameset cols=\"25%,75%\"><frame src=\"data:text/html,"
+    page.load(QUrl("data:text/html,<frameset cols=\"25%,75%\"><frame src=\"data:text/html,"
                                            "<head><meta http-equiv='refresh' content='1'></head>foo \">"
                                            "<frame src=\"data:text/html,bar\"></frameset>"));
     QTRY_COMPARE(spyLoadFinished.count(), 1);
 
-    QTRY_VERIFY(spyLoadStarted.count() > 1);
-    QTRY_VERIFY(spyLoadFinished.count() > 1);
+    QEXPECT_FAIL("", "Behavior change: Load signals are emitted only for the main frame in QtWebEngine.", Continue);
+    QTRY_VERIFY_WITH_TIMEOUT(spyLoadStarted.count() > 1, 100);
+    QEXPECT_FAIL("", "Behavior change: Load signals are emitted only for the main frame in QtWebEngine.", Continue);
+    QTRY_VERIFY_WITH_TIMEOUT(spyLoadFinished.count() > 1, 100);
 
     spyLoadFinished.clear();
 
-    m_view->page()->load(QUrl("data:text/html,<frameset cols=\"25%,75%\"><frame src=\"data:text/html,"
+    page.load(QUrl("data:text/html,<frameset cols=\"25%,75%\"><frame src=\"data:text/html,"
                                            "foo \"><frame src=\"data:text/html,bar\"></frameset>"));
     QTRY_COMPARE(spyLoadFinished.count(), 1);
     QCOMPARE(spyLoadFinished.count(), 1);
@@ -1604,6 +1606,9 @@ void tst_QWebEnginePage::textEditing()
 
 void tst_QWebEnginePage::requestCache()
 {
+#if !defined(ACCEPTNAVIGATIONREQUEST)
+    QSKIP("ACCEPTNAVIGATIONREQUEST");
+#else
     TestPage page;
     QSignalSpy loadSpy(&page, SIGNAL(loadFinished(bool)));
 
@@ -1627,6 +1632,7 @@ void tst_QWebEnginePage::requestCache()
              (int)QNetworkRequest::PreferNetwork);
     QCOMPARE(page.navigations.at(2).request.attribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::PreferNetwork).toInt(),
              (int)QNetworkRequest::PreferCache);
+#endif
 }
 
 void tst_QWebEnginePage::loadCachedPage()
@@ -1676,7 +1682,8 @@ void tst_QWebEnginePage::backActionUpdate()
     QTRY_COMPARE(loadSpy.count(), 1);
     QVERIFY(!action->isEnabled());
     QTest::mouseClick(&view, Qt::LeftButton, 0, QPoint(10, 10));
-    QTRY_COMPARE(loadSpy.count(), 2);
+    QEXPECT_FAIL("", "Behavior change: Load signals are emitted only for the main frame in QtWebEngine.", Continue);
+    QTRY_COMPARE_WITH_TIMEOUT(loadSpy.count(), 2, 100);
 
     QVERIFY(action->isEnabled());
 }
@@ -2723,6 +2730,7 @@ void tst_QWebEnginePage::defaultTextEncoding()
 #endif
 }
 
+#if defined(QWEBENGINEPAGE_ERRORPAGEEXTENSION)
 class ErrorPage : public QWebEnginePage
 {
 public:
@@ -2745,6 +2753,7 @@ public:
         return true;
     }
 };
+#endif
 
 void tst_QWebEnginePage::errorPageExtension()
 {
@@ -2827,6 +2836,9 @@ void tst_QWebEnginePage::errorPageExtensionInFrameset()
 
 void tst_QWebEnginePage::errorPageExtensionLoadFinished()
 {
+#if !defined(QWEBENGINEPAGE_ERRORPAGEEXTENSION)
+    QSKIP("QWEBENGINEPAGE_ERRORPAGEEXTENSION");
+#else
     ErrorPage page;
     m_view->setPage(&page);
 
@@ -2852,6 +2864,7 @@ void tst_QWebEnginePage::errorPageExtensionLoadFinished()
     QVERIFY(nonExistantFrameLoadSucceded);
 
     m_view->setPage(0);
+#endif
 }
 
 class FriendlyWebPage : public QWebEnginePage