Fix iconUrl getter and changed signal

We should only emit iconUrlChanged when iconUrl changes, and the return
value of iconUrl should match the value of the last emitted change.

Change-Id: I70f4d6a11b96d83278dddc4d85e1c78ca82bf5cb
Reviewed-by: Joerg Bornemann <joerg.bornemann@theqtcompany.com>
Reviewed-by: Peter Varga <pvarga@inf.u-szeged.hu>

3 files changed, 20 insertions, 24 deletions

diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index eecd40708..13dee09f4 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -146,7 +146,10 @@ void QWebEnginePagePrivate::urlChanged(const QUrl &url)
 void QWebEnginePagePrivate::iconChanged(const QUrl &url)
 {
     Q_Q(QWebEnginePage);
-    Q_EMIT q->iconUrlChanged(url);
+    if (iconUrl == url)
+        return;
+    iconUrl = url;
+    Q_EMIT q->iconUrlChanged(iconUrl);
 }
 
 void QWebEnginePagePrivate::loadProgressChanged(int progress)
@@ -1452,7 +1455,7 @@ QUrl QWebEnginePage::requestedUrl() const
 QUrl QWebEnginePage::iconUrl() const
 {
     Q_D(const QWebEnginePage);
-    return d->adapter->iconUrl();
+    return d->iconUrl;
 }
 
 qreal QWebEnginePage::zoomFactor() const
diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h
index ce769169e..6d4b857eb 100644
--- a/src/webenginewidgets/api/qwebenginepage_p.h
+++ b/src/webenginewidgets/api/qwebenginepage_p.h
@@ -163,6 +163,7 @@ public:
     bool fullscreenMode;
     QWebChannel *webChannel;
     unsigned int webChannelWorldId;
+    QUrl iconUrl;
 
     mutable QtWebEngineCore::CallbackDirectory m_callbacks;
     mutable QAction *actions[QWebEnginePage::WebActionCount];
diff --git a/tests/auto/widgets/qwebenginefaviconmanager/tst_qwebenginefaviconmanager.cpp b/tests/auto/widgets/qwebenginefaviconmanager/tst_qwebenginefaviconmanager.cpp
index ececb0efd..f435288f7 100644
--- a/tests/auto/widgets/qwebenginefaviconmanager/tst_qwebenginefaviconmanager.cpp
+++ b/tests/auto/widgets/qwebenginefaviconmanager/tst_qwebenginefaviconmanager.cpp
@@ -131,6 +131,7 @@ void tst_QWebEngineFaviconManager::faviconLoadEncodedUrl()
     QTRY_COMPARE(iconUrlChangedSpy.count(), 1);
 
     QUrl iconUrl = iconUrlChangedSpy.at(0).at(0).toString();
+    QCOMPARE(m_page->iconUrl(), iconUrl);
     QCOMPARE(iconUrl, QUrl::fromLocalFile(TESTS_SOURCE_DIR + QLatin1String("qwebenginefaviconmanager/resources/icons/qt32.ico")));
 }
 
@@ -146,10 +147,9 @@ void tst_QWebEngineFaviconManager::noFavicon()
     m_page->load(url);
 
     QTRY_COMPARE(loadFinishedSpy.count(), 1);
-    QTRY_COMPARE(iconUrlChangedSpy.count(), 1);
+    QCOMPARE(iconUrlChangedSpy.count(), 0);
 
-    QUrl iconUrl = iconUrlChangedSpy.at(0).at(0).toString();
-    QVERIFY(iconUrl.isEmpty());
+    QVERIFY(m_page->iconUrl().isEmpty());
 }
 
 void tst_QWebEngineFaviconManager::aboutBlank()
@@ -161,10 +161,9 @@ void tst_QWebEngineFaviconManager::aboutBlank()
     m_page->load(url);
 
     QTRY_COMPARE(loadFinishedSpy.count(), 1);
-    QTRY_COMPARE(iconUrlChangedSpy.count(), 1);
+    QTRY_COMPARE(iconUrlChangedSpy.count(), 0);
 
-    QUrl iconUrl = iconUrlChangedSpy.at(0).at(0).toString();
-    QVERIFY(iconUrl.isEmpty());
+    QVERIFY(m_page->iconUrl().isEmpty());
 }
 
 void tst_QWebEngineFaviconManager::unavailableFavicon()
@@ -182,6 +181,7 @@ void tst_QWebEngineFaviconManager::unavailableFavicon()
     QTRY_COMPARE(iconUrlChangedSpy.count(), 1);
 
     QUrl iconUrl = iconUrlChangedSpy.at(0).at(0).toString();
+    QCOMPARE(m_page->iconUrl(), iconUrl);
     QCOMPARE(iconUrl, QUrl::fromLocalFile(TESTS_SOURCE_DIR + QLatin1String("qwebenginefaviconmanager/resources/icons/unavailable.ico")));
 }
 
@@ -196,15 +196,9 @@ void tst_QWebEngineFaviconManager::errorPageEnabled()
     m_page->load(url);
 
     QTRY_COMPARE(loadFinishedSpy.count(), 1);
-    // Icon is reseted at load start.
-    // Load is started twice: once for unavailale page then error page
-    QTRY_COMPARE(iconUrlChangedSpy.count(), 2);
+    QCOMPARE(iconUrlChangedSpy.count(), 0);
 
-    QUrl iconUrl;
-    iconUrl = iconUrlChangedSpy.at(0).at(0).toString();
-    QVERIFY(iconUrl.isEmpty());
-    iconUrl = iconUrlChangedSpy.at(1).at(0).toString();
-    QVERIFY(iconUrl.isEmpty());
+    QVERIFY(m_page->iconUrl().isEmpty());
 }
 
 void tst_QWebEngineFaviconManager::errorPageDisabled()
@@ -218,10 +212,9 @@ void tst_QWebEngineFaviconManager::errorPageDisabled()
     m_page->load(url);
 
     QTRY_COMPARE(loadFinishedSpy.count(), 1);
-    QTRY_COMPARE(iconUrlChangedSpy.count(), 1);
+    QTRY_COMPARE(iconUrlChangedSpy.count(), 0);
 
-    QUrl iconUrl = iconUrlChangedSpy.at(0).at(0).toString();
-    QVERIFY(iconUrl.isEmpty());
+    QVERIFY(m_page->iconUrl().isEmpty());
 }
 
 void tst_QWebEngineFaviconManager::bestFavicon()
@@ -250,9 +243,9 @@ void tst_QWebEngineFaviconManager::bestFavicon()
     m_page->load(url);
 
     QTRY_COMPARE(loadFinishedSpy.count(), 1);
-    QTRY_COMPARE(iconUrlChangedSpy.count(), 1);
+    QTRY_VERIFY(iconUrlChangedSpy.count() >= 1);
 
-    iconUrl = iconUrlChangedSpy.at(0).at(0).toString();
+    iconUrl = iconUrlChangedSpy.last().at(0).toString();
     QCOMPARE(iconUrl, QUrl::fromLocalFile(TESTS_SOURCE_DIR + QLatin1String("qwebenginefaviconmanager/resources/icons/qt144.png")));
 }
 
@@ -268,10 +261,9 @@ void tst_QWebEngineFaviconManager::touchIcon()
     m_page->load(url);
 
     QTRY_COMPARE(loadFinishedSpy.count(), 1);
-    QTRY_COMPARE(iconUrlChangedSpy.count(), 1);
+    QTRY_COMPARE(iconUrlChangedSpy.count(), 0);
 
-    QUrl iconUrl = iconUrlChangedSpy.at(0).at(0).toString();
-    QVERIFY(iconUrl.isEmpty());
+    QVERIFY(m_page->iconUrl().isEmpty());
 }
 
 QTEST_MAIN(tst_QWebEngineFaviconManager)