Don't send duplicate load progress values

Suppress duplicated progress values coming from chromium.
Verify this behavior reliably (and not only 0 and 100 value) by loading html
with subresources with minor delay through test http server.

Change-Id: Id034dda9012212d54d12fc95d5939ba301577c1c
Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>

13 files changed, 80 insertions, 22 deletions

diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp
index f13621b99..7822d7a41 100644
--- a/src/core/web_contents_delegate_qt.cpp
+++ b/src/core/web_contents_delegate_qt.cpp
@@ -259,9 +259,12 @@ void WebContentsDelegateQt::LoadProgressChanged(double progress)
         return;
     if (m_lastLoadProgress < 0) // suppress signals that aren't between loadStarted and loadFinished
         return;
-    m_lastLoadProgress = qMax(m_lastLoadProgress, qRound(progress * 100)); // ensure monotonicity
-    m_lastLoadProgress = qMin(m_lastLoadProgress, 100);
-    m_viewClient->loadProgressChanged(m_lastLoadProgress);
+
+    int p = qMin(qRound(progress * 100), 100);
+    if (p > m_lastLoadProgress) { // ensure strict monotonic increase
+        m_lastLoadProgress = p;
+        m_viewClient->loadProgressChanged(p);
+    }
 }
 
 bool WebContentsDelegateQt::HandleKeyboardEvent(content::WebContents *, const content::NativeWebKeyboardEvent &event)
@@ -359,8 +362,9 @@ void WebContentsDelegateQt::EmitLoadFinished(bool success, const QUrl &url, bool
 {
     if (m_lastLoadProgress < 0) // not currently running
         return;
+    if (m_lastLoadProgress < 100)
+        m_viewClient->loadProgressChanged(100);
     m_lastLoadProgress = -1;
-    m_viewClient->loadProgressChanged(100);
     m_viewClient->loadFinished(success, url, isErrorPage, errorCode, errorDescription);
     m_viewClient->updateNavigationActions();
 }
diff --git a/tests/auto/quick/qmltests/data/tst_loadProgress.qml b/tests/auto/quick/qmltests/data/tst_loadProgress.qml
index 15058cb8f..7bfe1d9e9 100644
--- a/tests/auto/quick/qmltests/data/tst_loadProgress.qml
+++ b/tests/auto/quick/qmltests/data/tst_loadProgress.qml
@@ -30,6 +30,8 @@ import QtQuick 2.0
 import QtTest 1.0
 import QtWebEngine 1.2
 
+import Test.Shared 1.0 as Shared
+
 TestWebEngineView {
     id: webEngineView
     width: 400
@@ -55,10 +57,14 @@ TestWebEngineView {
             compare(spyProgress.count, 0)
             loadProgressArray = []
 
-            webEngineView.url = Qt.resolvedUrl("test1.html")
+            verify(Shared.HttpServer.start())
+            Shared.HttpServer.newRequest.connect(request => {
+                wait(250) // just add delay to trigger some progress for every sub resource
+            })
+            webEngineView.url = Shared.HttpServer.url('/loadprogress/main.html')
             // Wait for the first loadProgressChanged signal, which have to be non-negative
             spyProgress.wait()
-            verify(loadProgressArray[0] >= 0)
+            compare(loadProgressArray[0], 0)
             verify(webEngineView.loadProgress >= 0)
 
             // Wait for the last loadProgressChanged signal, which have to be 100%
@@ -67,13 +73,13 @@ TestWebEngineView {
             compare(loadProgressArray[loadProgressArray.length - 1], 100)
             compare(webEngineView.loadProgress, 100)
 
-            // Test whether the chromium emits progress numbers in ascending order
-            var loadProgressMin = 0
-            for (var i in loadProgressArray) {
-                var loadProgress = loadProgressArray[i]
-                if (loadProgressMin > loadProgress)
+            // Test whether the chromium emits progress numbers in strict monotonic ascending order
+            let progress = 0
+            for (let i = 1; i < loadProgressArray.length; ++i) {
+                let nextProgress = loadProgressArray[i]
+                if (nextProgress <= progress)
                     fail("Invalid sequence of progress-values: " + loadProgressArray)
-                loadProgressMin = loadProgress
+                progress = nextProgress
             }
         }
     }
diff --git a/tests/auto/quick/qmltests/qmltests.pro b/tests/auto/quick/qmltests/qmltests.pro
index 5c57f7ad9..a50bfd7e6 100644
--- a/tests/auto/quick/qmltests/qmltests.pro
+++ b/tests/auto/quick/qmltests/qmltests.pro
@@ -1,4 +1,5 @@
 include(../tests.pri)
+include(../../shared/http.pri)
 
 QT += qmltest
 
diff --git a/tests/auto/quick/qmltests/tst_qmltests.cpp b/tests/auto/quick/qmltests/tst_qmltests.cpp
index 819f0b07c..5677f9047 100644
--- a/tests/auto/quick/qmltests/tst_qmltests.cpp
+++ b/tests/auto/quick/qmltests/tst_qmltests.cpp
@@ -26,6 +26,8 @@
 **
 ****************************************************************************/
 
+#include <httpserver.h>
+
 #include <QtCore/QScopedPointer>
 #include <QTemporaryDir>
 #include <QtQuickTest/quicktest.h>
@@ -143,6 +145,12 @@ int main(int argc, char **argv)
     qmlRegisterType<TempDir>("Test.util", 1, 0, "TempDir");
 
     QTEST_SET_MAIN_SOURCE_PATH
+    qmlRegisterSingletonType<HttpServer>("Test.Shared", 1, 0, "HttpServer", [&] (QQmlEngine *, QJSEngine *) {
+        auto server = new HttpServer;
+        server->setResourceDirs({ TESTS_SHARED_DATA_DIR, QUICK_TEST_SOURCE_DIR });
+        return server;
+    });
+
     int i = quick_test_main(argc, argv, "qmltests", QUICK_TEST_SOURCE_DIR);
     return i;
 }
diff --git a/tests/auto/widgets/loadsignals/resources/downloadable.tar.gz b/tests/auto/shared/data/loadprogress/downloadable.tar.gz
index 741cb8ca6..741cb8ca6 100644
--- a/tests/auto/widgets/loadsignals/resources/downloadable.tar.gz
+++ b/tests/auto/shared/data/loadprogress/downloadable.tar.gz
diff --git a/tests/auto/shared/data/loadprogress/main.html b/tests/auto/shared/data/loadprogress/main.html
new file mode 100644
index 000000000..3b7d2034b
--- /dev/null
+++ b/tests/auto/shared/data/loadprogress/main.html
@@ -0,0 +1,30 @@
+<html>
+<head><title>Load Progress Test Page</title>
+	<style>
+	    .monospace { font-family: "Lucida Console", Courier, monospace; }
+	</style>
+    <title>page1</title>
+    <script>
+        function addP(t) {
+            var p = document.createElement('p')
+            p.class = 'monospace'
+            p.innerHTML = t
+            var d = document.createElement('div')
+            d.appendChild(p)
+            document.body.appendChild(d)
+        }
+        window.addEventListener('DOMContentLoaded', (event) => { addP('DOMContentLoaded') })
+    </script>
+</head>
+<body>
+    <h1>Hello.</h1>
+    <script>
+        addP('sometext')
+    </script>
+    <p class="monospace">body in monospace</p>
+    <iframe id="page1" src="page1.html"></iframe>
+    <iframe id="page2" src="page2.html"></iframe>
+    <iframe id="page3" src="page3.html"></iframe>
+    <iframe id="page4" src="page4.html"></iframe>
+</body>
+</html>
diff --git a/tests/auto/widgets/loadsignals/resources/page1.html b/tests/auto/shared/data/loadprogress/page1.html
index 5cd479ab6..5cd479ab6 100644
--- a/tests/auto/widgets/loadsignals/resources/page1.html
+++ b/tests/auto/shared/data/loadprogress/page1.html
diff --git a/tests/auto/widgets/loadsignals/resources/page2.html b/tests/auto/shared/data/loadprogress/page2.html
index e3031f56a..e3031f56a 100644
--- a/tests/auto/widgets/loadsignals/resources/page2.html
+++ b/tests/auto/shared/data/loadprogress/page2.html
diff --git a/tests/auto/widgets/loadsignals/resources/page3.html b/tests/auto/shared/data/loadprogress/page3.html
index d38ca31f0..d38ca31f0 100644
--- a/tests/auto/widgets/loadsignals/resources/page3.html
+++ b/tests/auto/shared/data/loadprogress/page3.html
diff --git a/tests/auto/widgets/loadsignals/resources/page4.html b/tests/auto/shared/data/loadprogress/page4.html
index 61976b4fb..61976b4fb 100644
--- a/tests/auto/widgets/loadsignals/resources/page4.html
+++ b/tests/auto/shared/data/loadprogress/page4.html
diff --git a/tests/auto/widgets/loadsignals/loadsignals.pro b/tests/auto/widgets/loadsignals/loadsignals.pro
index e99c7f493..9c239f1a7 100644
--- a/tests/auto/widgets/loadsignals/loadsignals.pro
+++ b/tests/auto/widgets/loadsignals/loadsignals.pro
@@ -1 +1,2 @@
 include(../tests.pri)
+include(../../shared/http.pri)
diff --git a/tests/auto/widgets/loadsignals/tst_loadsignals.cpp b/tests/auto/widgets/loadsignals/tst_loadsignals.cpp
index 20e5fbf0d..8eeadeeb8 100644
--- a/tests/auto/widgets/loadsignals/tst_loadsignals.cpp
+++ b/tests/auto/widgets/loadsignals/tst_loadsignals.cpp
@@ -28,6 +28,7 @@
 
 #include <QtTest/QtTest>
 
+#include "httpserver.h"
 #include "../util.h"
 #include "qdebug.h"
 #include "qwebenginepage.h"
@@ -133,18 +134,25 @@ void tst_LoadSignals::loadStartedAndFinishedCount()
   */
 void tst_LoadSignals::monotonicity()
 {
-    view->load(QUrl("qrc:///resources/page1.html"));
+    HttpServer server;
+    server.setResourceDirs({ TESTS_SHARED_DATA_DIR });
+    connect(&server, &HttpServer::newRequest, [] (HttpReqRep *) {
+         QTest::qWait(250); // just add delay to trigger some progress for every sub resource
+    });
+    QVERIFY(server.start());
+
+    view->load(server.url("/loadprogress/main.html"));
     QTRY_COMPARE(loadFinishedSpy->size(), 1);
     bool loadSucceeded = (*loadFinishedSpy)[0][0].toBool();
     QVERIFY(loadSucceeded);
 
     // first loadProgress should have 0% progress
-    QCOMPARE(loadProgressSpy->first()[0].toInt(), 0);
+    QCOMPARE(loadProgressSpy->takeFirst()[0].toInt(), 0);
 
-    // every loadProgress should have at least as much progress as the one before
+    // every loadProgress should have more progress than the one before
     int progress = 0;
     for (auto item : *loadProgressSpy) {
-        QVERIFY(item[0].toInt() >= progress);
+        QVERIFY(progress < item[0].toInt());
         progress = item[0].toInt();
     }
 
diff --git a/tests/auto/widgets/loadsignals/tst_loadsignals.qrc b/tests/auto/widgets/loadsignals/tst_loadsignals.qrc
index 316deecb8..21c517154 100644
--- a/tests/auto/widgets/loadsignals/tst_loadsignals.qrc
+++ b/tests/auto/widgets/loadsignals/tst_loadsignals.qrc
@@ -1,9 +1,9 @@
 <RCC>
-    <qresource prefix="/">
-        <file>resources/page1.html</file>
-        <file>resources/page2.html</file>
-        <file>resources/page3.html</file>
-        <file>resources/page4.html</file>
-        <file>resources/downloadable.tar.gz</file>
+    <qresource prefix="/resources">
+        <file alias="page1.html">../../shared/data/loadprogress/page1.html</file>
+        <file alias="page2.html">../../shared/data/loadprogress/page2.html</file>
+        <file alias="page3.html">../../shared/data/loadprogress/page3.html</file>
+        <file alias="page4.html">../../shared/data/loadprogress/page4.html</file>
+        <file alias="downloadable.tar.gz">../../shared/data/loadprogress/downloadable.tar.gz</file>
     </qresource>
 </RCC>