webbrowser-app

Merge lp:~osomon/webbrowser-app/historyUpdateOnLoadCommitted into lp:webbrowser-app

historyUpdateOnLoadCommitted
Merge into trunk

Proposed by Olivier Tilloy on 2015-05-18

Status:

Merged

Approved by:

Olivier Tilloy on 2016-03-04

Approved revision:

1029

Merged at revision:

1372

Proposed branch:

lp:~osomon/webbrowser-app/historyUpdateOnLoadCommitted

Merge into:

lp:webbrowser-app

Diff against target:

391 lines (+229/-40)

6 files modified

src/app/webbrowser/Browser.qml (+29/-2)
src/app/webbrowser/history-model.cpp (+37/-1)
src/app/webbrowser/history-model.h (+2/-1)
tests/autopilot/webbrowser_app/tests/http_server.py (+32/-1)
tests/autopilot/webbrowser_app/tests/test_history.py (+80/-34)
tests/unittests/history-model/tst_HistoryModelTests.cpp (+49/-1)

To merge this branch:

bzr merge lp:~osomon/webbrowser-app/historyUpdateOnLoadCommitted

Related bugs:

Bug #1455858: History AWOL with YouTube "get more videos" button.	Medium	Invalid
Bug #1549780: [regression] favicons not stored in the history database	High	Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Riccardo Padovani (community)			Approve on 2016-03-01
PS Jenkins bot	continuous-integration		Needs Fixing on 2016-03-01
Ubuntu Phablet Team		2015-05-18	Pending
Review via email: mp+259361@code.launchpad.net

Commit message

Store entries in the history database on load committed, not load succeeded.
This ensures that content-initiated navigations are also stored.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-05-18:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1778/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-05-23:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1807/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2015-06-11:

+ browser.historyModel.add(webviewInternal.storedUrl, webviewInternal.storedTitle, icon)

Why this and not

browser.historyModel.add(webviewInternal.storedUrl, title, icon)

I mean, we're updating the database, it's better to do it with last data we have, isn't it?

review: Needs Information

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-06-11:

> + browser.historyModel.add(webviewInternal.storedUrl,
> webviewInternal.storedTitle, icon)
>
> Why this and not
>
> browser.historyModel.add(webviewInternal.storedUrl, title, icon)
>
> ?
>
> I mean, we're updating the database, it's better to do it with last data we
> have, isn't it?

Some pages dynamically update the title to convey live information (like the ugly "scrolling title" effect, see e.g. http://htmlmarquee.com/title.html). In that case we don’t want to update the history DB everytime the title changes. And if the title has changed when we get an icon, we don’t want to update it either, because it’s most likely a dynamic change.
This is consistent with how chromium on desktop behaves.

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2015-06-11:

Looks good to me now :-)

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-06-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1879/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-13:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2084/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-13:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2085/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-14:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2092/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-14:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2096/rebuild

review: Needs Fixing (continuous-integration)

lp:~osomon/webbrowser-app/historyUpdateOnLoadCommitted updated on 2016-02-26

1021. By Olivier Tilloy on 2016-02-26: Merge the latest changes from trunk and resolve conflicts.
1022. By Olivier Tilloy on 2016-02-26: Add an update(…) method to HistoryModel, to allow updating the title and icon of an entry without incrementing the corresponding number of visits.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-26:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2638/rebuild

review: Needs Fixing (continuous-integration)

lp:~osomon/webbrowser-app/historyUpdateOnLoadCommitted updated on 2016-02-26

1023. By Olivier Tilloy on 2016-02-26: Simplify code a bit.
Do not prevent dynamic favicon updates from updating the history database, to be consistent with what chromium does.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-26:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2639/rebuild

review: Needs Fixing (continuous-integration)

lp:~osomon/webbrowser-app/historyUpdateOnLoadCommitted updated on 2016-02-29

1024. By Olivier Tilloy on 2016-02-29: Add some autopilot tests to verify that favicons are stored and updated in the history database.
1025. By Olivier Tilloy on 2016-02-29: Factor out common code.
1026. By Olivier Tilloy on 2016-02-29: Add autopilot tests to verify that a page’s title is stored in history, but not dynamically updated.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-29:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2646/rebuild

review: Needs Fixing (continuous-integration)

lp:~osomon/webbrowser-app/historyUpdateOnLoadCommitted updated on 2016-02-29

1027. By Olivier Tilloy on 2016-02-29: Add an autopilot test to verify that history.pushState() inserts a new entry in the history database.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-29:

FAILED: Continuous integration, rev:1026
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2647/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6678
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1400
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1400/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1400
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/5068
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6689
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6689/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27945

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2647/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-29:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2649/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2016-03-01:

I only read the code and seems good to me, just a couple of very little improvements inline.

I'll test asap.

review: Needs Fixing

lp:~osomon/webbrowser-app/historyUpdateOnLoadCommitted updated on 2016-03-01

1028. By Olivier Tilloy on 2016-03-01: Swap the order in which conditions are evaluated, from cheapest to most expensive.
1029. By Olivier Tilloy on 2016-03-01: Move code out of else block to avoid compiler warning (control reaches end of non-void function [-Wreturn-type]).

Revision history for this message

Olivier Tilloy (osomon) wrote on 2016-03-01:

Thanks, those were very good points, I addressed them.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-03-01:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2651/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2016-03-01:

Thanks! Tested a bit and seems to work like a charm.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Olivier Tilloy

Ubuntu Phablet Team

to status/vote changes:

Rachel Liu

 === modified file 'src/app/webbrowser/Browser.qml'
 --- src/app/webbrowser/Browser.qml	2016-02-23 11:24:33 +0000
 +++ src/app/webbrowser/Browser.qml	2016-03-01 09:31:14 +0000
@@ -1263,20 +1263,47 @@
+                     }
+                 }
++                QtObject {
++                    id: webviewInternal
++                    property url storedUrl: ""
++                    property bool titleSet: false
++                    property string title: ""
++                }
                  onLoadEvent: {
                      if (event.type == Oxide.LoadEvent.TypeCommitted) {
                          chrome.findInPageMode = false
++                        webviewInternal.titleSet = false
++                        webviewInternal.title = title
+                     }
                      if (webviewimpl.incognito) {
                          return
+                     }
--                    if (event.type == Oxide.LoadEvent.TypeSucceeded &&
--                        300 > event.httpStatusCode && event.httpStatusCode >= 200) {
++                    if ((event.type == Oxide.LoadEvent.TypeCommitted) &&
++                        !event.isError &&
++                        (300 > event.httpStatusCode) && (event.httpStatusCode >= 200)) {
++                        webviewInternal.storedUrl = event.url
                          HistoryModel.add(event.url, title, icon)
+                     }
+                 }
++                onTitleChanged: {
++                    if (!webviewInternal.titleSet && webviewInternal.storedUrl.toString()) {
++                        // Record the title to avoid updating the history database
++                        // every time the page dynamically updates its title.
++                        // We don’t want pages that update their title every second
++                        // to achieve an ugly "scrolling title" effect to flood the
++                        // history database with updates.
++                        webviewInternal.titleSet = true
++                        webviewInternal.title = title
++                        HistoryModel.update(webviewInternal.storedUrl, title, icon)
++                    }
++                }
++                onIconChanged: {
++                    if (webviewInternal.storedUrl.toString()) {
++                        HistoryModel.update(webviewInternal.storedUrl, webviewInternal.title, icon)
++                    }
++                }
                  onGeolocationPermissionRequested: requestGeolocationPermission(request)
 === modified file 'src/app/webbrowser/history-model.cpp'
 --- src/app/webbrowser/history-model.cpp	2015-10-07 11:10:14 +0000
 +++ src/app/webbrowser/history-model.cpp	2016-03-01 09:31:14 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright 2013-2015 Canonical Ltd.
++ * Copyright 2013-2016 Canonical Ltd.
+  *
   * This file is part of webbrowser-app.
+  *
@@ -301,6 +301,42 @@
+ }
  /*!
++    Update an existing entry in the model.
++
++    If no entry with the same URL exists yet, do nothing (and return false).
++    Otherwise the title and icon of the existing entry are updated (the number
++    of visits remains unchanged).
++
++    Return true if an update actually happened, false otherwise.
++*/
++bool HistoryModel::update(const QUrl& url, const QString& title, const QUrl& icon)
++{
++    if (url.isEmpty()) {
++        return false;
++    }
++    int index = getEntryIndex(url);
++    if (index == -1) {
++        return false;
++    }
++    QVector<int> roles;
++    const HistoryEntry& entry = m_entries.at(index);
++    if (title != entry.title) {
++        m_entries[index].title = title;
++        roles << Title;
++    }
++    if (icon != entry.icon) {
++        m_entries[index].icon = icon;
++        roles << Icon;
++    }
++    if (roles.isEmpty()) {
++        return false;
++    }
++    Q_EMIT dataChanged(this->index(index, 0), this->index(index, 0), roles);
++    updateExistingEntryInDatabase(entry);
++    return true;
++}
++
++/*!
      Remove a given URL from the history model.
      If the URL was not previously visited, do nothing.
 === modified file 'src/app/webbrowser/history-model.h'
 --- src/app/webbrowser/history-model.h	2015-10-07 11:10:14 +0000
 +++ src/app/webbrowser/history-model.h	2016-03-01 09:31:14 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright 2013-2015 Canonical Ltd.
++ * Copyright 2013-2016 Canonical Ltd.
+  *
   * This file is part of webbrowser-app.
+  *
@@ -62,6 +62,7 @@
      void setDatabasePath(const QString& path);
      Q_INVOKABLE int add(const QUrl& url, const QString& title, const QUrl& icon);
++    Q_INVOKABLE bool update(const QUrl& url, const QString& title, const QUrl& icon);
      Q_INVOKABLE void removeEntryByUrl(const QUrl& url);
      Q_INVOKABLE void removeEntriesByDate(const QDate& date);
      Q_INVOKABLE void removeEntriesByDomain(const QString& domain);
 === modified file 'tests/autopilot/webbrowser_app/tests/http_server.py'
 --- tests/autopilot/webbrowser_app/tests/http_server.py	2015-10-26 11:09:25 +0000
 +++ tests/autopilot/webbrowser_app/tests/http_server.py	2016-03-01 09:31:14 +0000
@@ -1,6 +1,6 @@
  # -*- coding: utf-8 -*-
+ #
--# Copyright 2013-2015 Canonical
++# Copyright 2013-2016 Canonical
+ #
  # This program is free software: you can redistribute it and/or modify it
  # under the terms of the GNU General Public License version 3, as published
@@ -215,6 +215,37 @@
                  "}, function() { location.href = '/test1' } " +
                  ", function() { location.href = '/test2' })</script>"
+             )
++        elif self.path == "/favicon":
++            self.send_response(200)
++            html = '<html><head><link rel="icon" type="image/png" '
++            html += 'href="/assets/icon1.png"></head>'
++            html += '<body>favicon</body></html>'
++            self.send_html(html)
++        elif self.path == "/changingfavicon":
++            self.send_response(200)
++            html = '<html><head><link id="favicon" rel="shortcut icon" '
++            html += 'type="image/png" href="icon0.png"></head><body><script>'
++            html += 'var i = 0; window.setInterval(function() {'
++            html += 'document.getElementById("favicon").href = ++i + '
++            html += '".png"; }, 1000);</script></body></html>'
++            self.send_html(html)
++        elif self.path == "/changingtitle":
++            self.send_response(200)
++            html = '<html><head><title>title0</title></head><body><script>'
++            html += 'var i = 0; window.setInterval(function() { '
++            html += 'document.title = "title" + ++i; }, 500);</script></body>'
++            html += '</html>'
++            self.send_html(html)
++        elif self.path == "/pushstate":
++            self.send_response(200)
++            html = '<html><head><title>push state</title></head>'
++            html += '<body style="margin: 0"><a id="link">'
++            html += '<div style="height: 100%"></div></a><script>'
++            html += 'document.getElementById("link").addEventListener("click",'
++            html += ' function(e) { document.title = "state pushed"; '
++            html += 'history.pushState(null, null, "/statepushed"); });'
++            html += '</script></body></html>'
++            self.send_html(html)
          else:
              self.send_error(404)
 === modified file 'tests/autopilot/webbrowser_app/tests/test_history.py'
 --- tests/autopilot/webbrowser_app/tests/test_history.py	2015-11-23 10:25:26 +0000
 +++ tests/autopilot/webbrowser_app/tests/test_history.py	2016-03-01 09:31:14 +0000
@@ -1,6 +1,6 @@
  # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
+ #
--# Copyright 2015 Canonical
++# Copyright 2015-2016 Canonical
+ #
  # This program is free software: you can redistribute it and/or modify it
  # under the terms of the GNU General Public License version 3, as published
@@ -16,7 +16,7 @@
  import time
--from testtools.matchers import Equals
++from testtools.matchers import EndsWith, Equals, StartsWith
  from autopilot.matchers import Eventually
  from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
@@ -24,45 +24,36 @@
  class TestHistory(StartOpenRemotePageTestCaseBase):
--    def test_history_not_save_404(self):
--        url = self.base_url + "/404page"
--        self.main_window.go_to_url(url)
--        self.main_window.wait_until_page_loaded(url)
--
--        # A valid url to be sure the fact the 404 page isn't present in the
--        # history view isn't a timing issue.
--        url = self.base_url + "/test2"
--        self.main_window.go_to_url(url)
--        self.main_window.wait_until_page_loaded(url)
--
--        history = self.open_history()
--
--        # We have domains with subsections only on mobiles.
--        # On wide we take all the entries directly
++    def expect_history_entries(self, ordered_urls):
++        history = self.main_window.get_history_view()
          if self.main_window.wide:
--            delegates = history.get_entries()
--
--            # 2 addresses: /test1 and /test2
              self.assertThat(lambda: len(history.get_entries()),
--                            Eventually(Equals(2)))
--            self.assertThat(sorted([delegate.url for delegate in delegates]),
--                            Equals(sorted([self.url, url])))
++                            Eventually(Equals(len(ordered_urls))))
++            entries = history.get_entries()
          else:
--            # 1 domain: the local one
--            domain_entries = history.get_domain_entries()
              self.assertThat(lambda: len(history.get_domain_entries()),
                              Eventually(Equals(1)))
--
--            self.pointing_device.click_object(domain_entries[0])
++            self.pointing_device.click_object(history.get_domain_entries()[0])
              expanded_history = self.main_window.get_expanded_history_view()
--
--            # 2 addresses: /test1 and /test2
              self.assertThat(lambda: len(expanded_history.get_entries()),
--                            Eventually(Equals(2)))
--
--            delegates = expanded_history.get_entries()
--            self.assertThat(sorted([delegate.url for delegate in delegates]),
--                            Equals(sorted([self.url, url])))
++                            Eventually(Equals(len(ordered_urls))))
++            entries = expanded_history.get_entries()
++        self.assertThat([entry.url for entry in entries], Equals(ordered_urls))
++        return entries
++
++    def test_404_not_saved(self):
++        url = self.base_url + "/notfound"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++
++        # A valid url to be sure the fact the 404 page isn't present in the
++        # history view isn't a timing issue.
++        url = self.base_url + "/test2"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++
++        self.open_history()
++        self.expect_history_entries([url, self.url])
      def test_expanded_history_view_header_swallows_clicks(self):
          # Regression test for https://launchpad.net/bugs/1518904
@@ -78,3 +69,58 @@
          # There should be only one instance on the expanded history view.
          # If there’s more, the following call will raise an exception.
          self.main_window.get_expanded_history_view()
++
++    def test_favicon_saved(self):
++        # Regression test for https://launchpad.net/bugs/1549780
++        url = self.base_url + "/favicon"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++        self.open_history()
++        first = self.expect_history_entries([url, self.url])[0]
++        self.assertThat(first.url, Equals(url))
++        favicon = self.base_url + "/assets/icon1.png"
++        self.assertThat(first.icon, Equals(favicon))
++
++    def test_favicon_updated(self):
++        # Verify that a page dynamically updating its favicon
++        # triggers an update in the history database.
++        url = self.base_url + "/changingfavicon"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++        self.open_history()
++        first = self.expect_history_entries([url, self.url])[0]
++        indexes = set()
++        while len(indexes) < 3:
++            self.assertThat(first.url, Equals(url))
++            icon = first.icon
++            self.assertThat(icon, StartsWith(self.base_url))
++            self.assertThat(icon, EndsWith(".png"))
++            indexes.add(int(first.icon[(len(self.base_url)+1):-4]))
++
++    def test_title_saved(self):
++        self.open_history()
++        entry = self.expect_history_entries([self.url])[0]
++        self.assertThat(entry.title, Equals("test page 1"))
++
++    def test_title_not_updated(self):
++        # Verify that a page dynamically updating its title
++        # does NOT trigger an update in the history database.
++        url = self.base_url + "/changingtitle"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++        self.open_history()
++        first = self.expect_history_entries([url, self.url])[0]
++        for i in range(10):
++            self.assertThat(first.title, Equals("title0"))
++            time.sleep(0.5)
++
++    def test_pushstate_updates_history(self):
++        url = self.base_url + "/pushstate"
++        self.main_window.go_to_url(url)
++        self.main_window.wait_until_page_loaded(url)
++        webview = self.main_window.get_current_webview()
++        self.pointing_device.click_object(webview)
++        pushed = self.base_url + "/statepushed"
++        self.main_window.wait_until_page_loaded(pushed)
++        self.open_history()
++        self.expect_history_entries([pushed, url, self.url])
 === modified file 'tests/unittests/history-model/tst_HistoryModelTests.cpp'
 --- tests/unittests/history-model/tst_HistoryModelTests.cpp	2015-10-07 11:10:14 +0000
 +++ tests/unittests/history-model/tst_HistoryModelTests.cpp	2016-03-01 09:31:14 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright 2013 Canonical Ltd.
++ * Copyright 2013-2016 Canonical Ltd.
+  *
   * This file is part of webbrowser-app.
+  *
@@ -136,6 +136,54 @@
          QVERIFY(roles.contains(HistoryModel::Visits));
+     }
++    void shouldUpdateAttributes()
++    {
++        qRegisterMetaType<QVector<int> >();
++        QSignalSpy spyChanged(model, SIGNAL(dataChanged(const QModelIndex&, const QModelIndex&, const QVector<int>&)));
++        QUrl url(QStringLiteral("http://example.org/"));
++        QCOMPARE(model->add(url, QStringLiteral(""), QUrl()), 1);
++
++        QVERIFY(!model->update(QUrl(), QStringLiteral(""), QUrl()));
++        QVERIFY(spyChanged.isEmpty());
++
++        QVERIFY(!model->update(QUrl(QStringLiteral("http://example.com/")), QStringLiteral(""), QUrl()));
++        QVERIFY(spyChanged.isEmpty());
++
++        QVERIFY(!model->update(url, QStringLiteral(""), QUrl()));
++        QVERIFY(spyChanged.isEmpty());
++
++        QVERIFY(model->update(url, QStringLiteral("title update"), QUrl()));
++        QCOMPARE(spyChanged.count(), 1);
++        QList<QVariant> args = spyChanged.takeFirst();
++        QCOMPARE(args.at(0).toModelIndex().row(), 0);
++        QCOMPARE(args.at(1).toModelIndex().row(), 0);
++        QVector<int> roles = args.at(2).value<QVector<int> >();
++        QCOMPARE(roles.size(), 1);
++        QVERIFY(roles.contains(HistoryModel::Title));
++        QCOMPARE(model->get(0)[QStringLiteral("visits")].toInt(), 1);
++
++        QVERIFY(model->update(url, QStringLiteral("title update"), QUrl(QStringLiteral("image://webicon/123"))));
++        QCOMPARE(spyChanged.count(), 1);
++        args = spyChanged.takeFirst();
++        QCOMPARE(args.at(0).toModelIndex().row(), 0);
++        QCOMPARE(args.at(1).toModelIndex().row(), 0);
++        roles = args.at(2).value<QVector<int> >();
++        QCOMPARE(roles.size(), 1);
++        QVERIFY(roles.contains(HistoryModel::Icon));
++        QCOMPARE(model->get(0)[QStringLiteral("visits")].toInt(), 1);
++
++        QVERIFY(model->update(url, QStringLiteral("title update 2"), QUrl(QStringLiteral("image://webicon/1234"))));
++        QCOMPARE(spyChanged.count(), 1);
++        args = spyChanged.takeFirst();
++        QCOMPARE(args.at(0).toModelIndex().row(), 0);
++        QCOMPARE(args.at(1).toModelIndex().row(), 0);
++        roles = args.at(2).value<QVector<int> >();
++        QCOMPARE(roles.size(), 2);
++        QVERIFY(roles.contains(HistoryModel::Title));
++        QVERIFY(roles.contains(HistoryModel::Icon));
++        QCOMPARE(model->get(0)[QStringLiteral("visits")].toInt(), 1);
++    }
++
      void shouldNotifyWhenHidingOrUnHidingExistingEntry()
+     {
          qRegisterMetaType<QVector<int> >();

webbrowser-app

Merge lp:~osomon/webbrowser-app/historyUpdateOnLoadCommitted into lp:webbrowser-app

Commit message

Description of the change

Preview Diff

Subscribers