Merge into trunk : webbrowser-app-bookmarks_top

Status:	Merged
Approved by:	Olivier Tilloy on 2014-07-04
Approved revision:	592
Merged at revision:	602
Proposed branch:	lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites
Merge into:	lp:webbrowser-app
Diff against target:	1331 lines (+1080/-12) 19 files modified po/webbrowser-app.pot (+19/-6) src/app/webbrowser/BookmarksList.qml (+75/-0) src/app/webbrowser/Browser.qml (+29/-4) src/app/webbrowser/CMakeLists.txt (+2/-0) src/app/webbrowser/NewTabView.qml (+176/-0) src/app/webbrowser/UrlDelegate.qml (+73/-0) src/app/webbrowser/history-byvisits-model.cpp (+50/-0) src/app/webbrowser/history-byvisits-model.h (+43/-0) src/app/webbrowser/limit-proxy-model.cpp (+268/-0) src/app/webbrowser/limit-proxy-model.h (+70/-0) src/app/webbrowser/webbrowser-app.cpp (+4/-0) tests/autopilot/webbrowser_app/emulators/browser.py (+6/-0) tests/autopilot/webbrowser_app/tests/__init__.py (+8/-2) tests/autopilot/webbrowser_app/tests/test_tabs.py (+10/-0) tests/unittests/CMakeLists.txt (+2/-0) tests/unittests/history-byvisits-model/CMakeLists.txt (+6/-0) tests/unittests/history-byvisits-model/tst_HistoryByVisitsModelTests.cpp (+89/-0) tests/unittests/limit-proxy-model/CMakeLists.txt (+6/-0) tests/unittests/limit-proxy-model/tst_LimitProxyModelTests.cpp (+144/-0)
To merge this branch:	bzr merge lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2014-07-05
Olivier Tilloy		2014-06-26	Approve on 2014-07-04
Review via email: mp+224592@code.launchpad.net

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-26:

#

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

review: Approve (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-27:

#

Can you back out the changes to the bookmarks model? Ugo has been working on a branch to add the timestamp separately (see https://code.launchpad.net/~phablet-team/webbrowser-app/webbrowser-app-bookmark-timestamp/+merge/224117). Additionally, I don’t think we’ll need BookmarksChronologicalModel as we will ensure the original BookmarksModel is always sorted by creation date (AFAIK there is no use case for sorting them otherwise).

So until we merge Ugo’s branch (which needs some rework that I’ll probably take on), bookmarks would not be ordered by creation date in the new tab page, but that shouldn’t be too bad for a start.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-27:

#

Functional issue: I have two bookmarks in my database, and when opening the new tab page, there is a "see more" link, which, when clicked, expands the bookmarks section to take up the whole screen, thus hiding the history entries. I think the "see more" link should be visible only when there are actually more bookmarks to display.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-27:

#

Functional regression: if I bookmark new pages (the verify that they appear in the bookmarks view), then exit the browser and launch it again, the new bookmarks haven’t been stored in the database.

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-27:

#

As pointed out in a previous review round: the "see more/see less" action should probably trigger a smooth transition.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-27:

#

The copyright notice for tests/unittests/bookmarks-chronological-model/tst_BookmarksChronologicalModelTests.cpp should be dated 2014.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-27:

#

What purpose does the TopSitesSheet component serve? It seems all it does is wrap a TopSitesView and proxy some of its properties and signals, so it looks like it could be removed.

On a related note, the naming of TopSitesView is slightly confusing, as it contains both top sites and bookmarks. A better name would probably be "NewTabPage" or "NewTabView".

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-27:

#

It looks like this could be simplified:

249 + start: {
250 + var date = new Date()
251 + date.setDate(date.getDate() - 60)
252 + date.setHours(0)
253 + date.setMinutes(0)
254 + date.setSeconds(0)
255 + date.setMilliseconds(0)
256 + return date
257 + }
258 + end: {
259 + var date = new Date()
260 + date.setDate(date.getDate())
261 + date.setHours(23)
262 + date.setMinutes(59)
263 + date.setSeconds(59)
264 + date.setMilliseconds(999)
265 + return date
266 + }

For start, you don’t need to call setHours, setMinutes, setSeconds and setMilliseconds.
For end, you can just return a new Date(), i.e.: "end: new Date()"

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-27:

#

As already discussed, in the current implementation TopSitesSheet is always instantiated. Instead, it should be loaded on demand (encapsulating it in a Component, or using a Loader), only when the user requests a new tab. It should probably be destroyed as soon as the URL of the current webview becomes non empty, and instantiated again when switching to a webview with an empty URL.

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-06-27

564. By Arthur Mello on 2014-06-27: Merge with trunk
565. By Arthur Mello on 2014-06-27: Back out the changes to the bookmarks model.
We are not using an ordered sort model anymore.

Revision history for this message

Arthur Mello (artmello) wrote on 2014-06-27:

#

> Can you back out the changes to the bookmarks model? Ugo has been working on a
> branch to add the timestamp separately (see https://code.launchpad.net
> /~phablet-team/webbrowser-app/webbrowser-app-bookmark-
> timestamp/+merge/224117). Additionally, I don’t think we’ll need
> BookmarksChronologicalModel as we will ensure the original BookmarksModel is
> always sorted by creation date (AFAIK there is no use case for sorting them
> otherwise).
>
> So until we merge Ugo’s branch (which needs some rework that I’ll probably
> take on), bookmarks would not be ordered by creation date in the new tab page,
> but that shouldn’t be too bad for a start.

Fixed on revision 565

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-06-27

566. By Arthur Mello on 2014-06-27: Make "see more" link visible only when there are actually more bookmarks to display

Revision history for this message

Arthur Mello (artmello) wrote on 2014-06-27:

#

> Functional issue: I have two bookmarks in my database, and when opening the
> new tab page, there is a "see more" link, which, when clicked, expands the
> bookmarks section to take up the whole screen, thus hiding the history
> entries. I think the "see more" link should be visible only when there are
> actually more bookmarks to display.

Fixed on revision 566

Revision history for this message

Arthur Mello (artmello) wrote on 2014-06-27:

#

> Functional regression: if I bookmark new pages (the verify that they appear in
> the bookmarks view), then exit the browser and launch it again, the new
> bookmarks haven’t been stored in the database.

I was not able to reproduce the problem after rev 565, when we removed the changes on bookmarks model. Probably there was something wrong with the changes on bookmarks model or on bookmarks chronological model. If you still face the issue, please, let me know.

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-06-28

567. By Arthur Mello on 2014-06-28: Make the "see more/see less" action trigger a transition

Revision history for this message

Arthur Mello (artmello) wrote on 2014-06-28:

#

> As pointed out in a previous review round: the "see more/see less" action
> should probably trigger a smooth transition.

Fixed on revision 567

Revision history for this message

Arthur Mello (artmello) wrote on 2014-06-28:

#

> The copyright notice for tests/unittests/bookmarks-chronological-
> model/tst_BookmarksChronologicalModelTests.cpp should be dated 2014.

The unittest was removed since the bookmarks chronological model is not used anymore

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-06-28

568. By Arthur Mello on 2014-06-28: Remove unnecessary TopSitesSheet
Rename TopSitesView to NewTabView

Revision history for this message

Arthur Mello (artmello) wrote on 2014-06-28:

#

> What purpose does the TopSitesSheet component serve? It seems all it does is
> wrap a TopSitesView and proxy some of its properties and signals, so it looks
> like it could be removed.
>
> On a related note, the naming of TopSitesView is slightly confusing, as it
> contains both top sites and bookmarks. A better name would probably be
> "NewTabPage" or "NewTabView".

Fixed on revision 568

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-06-28

569. By Arthur Mello on 2014-06-28: Simplify bookmarks transition
570. By Arthur Mello on 2014-06-28: Simplify start and end date calculation for timeframe history model

Revision history for this message

Arthur Mello (artmello) wrote on 2014-06-28:

#

> It looks like this could be simplified:
>
> 249 + start: {
> 250 + var date = new Date()
> 251 + date.setDate(date.getDate() - 60)
> 252 + date.setHours(0)
> 253 + date.setMinutes(0)
> 254 + date.setSeconds(0)
> 255 + date.setMilliseconds(0)
> 256 + return date
> 257 + }
> 258 + end: {
> 259 + var date = new Date()
> 260 + date.setDate(date.getDate())
> 261 + date.setHours(23)
> 262 + date.setMinutes(59)
> 263 + date.setSeconds(59)
> 264 + date.setMilliseconds(999)
> 265 + return date
> 266 + }
>
> For start, you don’t need to call setHours, setMinutes, setSeconds and
> setMilliseconds.
> For end, you can just return a new Date(), i.e.: "end: new Date()"

Fixed on revision 570

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-06-28

571. By Arthur Mello on 2014-06-28: Make NewTabView be loaded on demand using a Loader

Revision history for this message

Arthur Mello (artmello) wrote on 2014-06-28:

#

> As already discussed, in the current implementation TopSitesSheet is always
> instantiated. Instead, it should be loaded on demand (encapsulating it in a
> Component, or using a Loader), only when the user requests a new tab. It
> should probably be destroyed as soon as the URL of the current webview becomes
> non empty, and instantiated again when switching to a webview with an empty
> URL.

Fixed on revision 571

Revision history for this message

Arthur Mello (artmello) wrote on 2014-06-28:

#

> As already discussed, in the current implementation TopSitesSheet is always
> instantiated. Instead, it should be loaded on demand (encapsulating it in a
> Component, or using a Loader), only when the user requests a new tab. It
> should probably be destroyed as soon as the URL of the current webview becomes
> non empty, and instantiated again when switching to a webview with an empty
> URL.

Fixed on revision 571

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-28:

#

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

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-28:

#

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

review: Needs Fixing (continuous-integration)

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-06-29

572. By Arthur Mello on 2014-06-29: Fix AP tests. Removed the validation for webview visible after closing Activity View.
Sometimes, the New Tab view will be visible after Activity View and not the webview

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-29:

#

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

review: Approve (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-01:

#

There is a trivial conflict in tests/autopilot/webbrowser_app/emulators/browser.py when merging into the latest trunk, can you please resolve it?

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-01:

#

For animations and behaviours, you’re using a few NumberAnimation with explicit durations. Please consider using UbuntuNumberAnimation (http://developer.ubuntu.com/api/qml/sdk-14.10/Ubuntu.Components.UbuntuNumberAnimation/) instead.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-01:

#

If I open a new empty tab, then type in a URL in the address bar and validate, I’m getting the following warning:

WARNING: Trying to pop an empty PageStack. Ignoring.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-01:

#

The "see more/see less" transition is looking better, but still not perfect IMHO.

When I click "see more", I’m seeing the "top sites" section being abruptly shifted downwards, then fade away. It should probably move downwards smoothly and fade away at the same time.

When I click "see less", the extra bookmarks disappear instantly. Ideally they should fade away progressively as the footer is moved upwards.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-01:

#

Is there a need for the newTabViewVisible property? If so, couldn’t it just be defined as "newTabViewLoader.status === Loader.Ready" ?

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-01

573. By Arthur Mello on 2014-07-01: Merge with trunk

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-01:

#

> There is a trivial conflict in
> tests/autopilot/webbrowser_app/emulators/browser.py when merging into the
> latest trunk, can you please resolve it?

Fixed on revision 573

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-01

574. By Arthur Mello on 2014-07-01: Using UbuntuNumberAnimation instead of NumberAnimation

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-01:

#

> For animations and behaviours, you’re using a few NumberAnimation with
> explicit durations. Please consider using UbuntuNumberAnimation (http://develo
> per.ubuntu.com/api/qml/sdk-14.10/Ubuntu.Components.UbuntuNumberAnimation/)
> instead.

Fixed on revision 574

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-01

575. By Arthur Mello on 2014-07-01: Move newTabViewLoader to be a child of webviewContainer and stop using pageStack

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-01:

#

> If I open a new empty tab, then type in a URL in the address bar and validate,
> I’m getting the following warning:
>
> WARNING: Trying to pop an empty PageStack. Ignoring.

Fixed on revision 575

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-01:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-01:

#

> Is there a need for the newTabViewVisible property? If so, couldn’t it just be
> defined as "newTabViewLoader.status === Loader.Ready" ?

The property was removed since it is not more nece

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-01:

#

> The "see more/see less" transition is looking better, but still not perfect
> IMHO.
>
> When I click "see more", I’m seeing the "top sites" section being abruptly
> shifted downwards, then fade away. It should probably move downwards smoothly
> and fade away at the same time.

The "bookmarks" and the "top sites" lists are inside a ListView. I tried to animate with Behavior on Y the delegate but with no success. My idea was to animate the move from the "top sites" component. Any suggestion?

> When I click "see less", the extra bookmarks disappear instantly. Ideally they
> should fade away progressively as the footer is moved upwards.

Right now the bookmarks list is a Repeater, not a ListView, inside a Column. I am animating the move transition of the Column. That is why we have the effect when we are "expanding" the Repeater. But to use the same effect before removing the entries of the Repeater I would need to use something like the delayRemove from the ListView. I didn't find a way to do that on a Repeater. Any suggestion? Should I change the Repeater to a ListView?

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-02:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-03:

#

114 + onUrlChanged: {
119 + function loadNewTabView() {
125 + function unloadNewTabView() {

There is a more declarative way of writing this: have a Component that contains a NewTabView inside the loader, and set the 'sourceComponent' property of the loader to "parent.url ? undefined : newTabViewComponent".

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-03:

#

Regarding the transitions/animations, they’re not perfect, but I think they’ll do for now, we can tweak them when we implement the new design.

I’m seeing a new issue though (or maybe it was there already and I hadn’t spotted it before): on desktop, if I have more than 5 bookmarks and more than 5 top sites, only the first row of the top sites category can be seen, I can see the top of the second row but I cannot drag it into view, it always springs back to the first row.

review: Needs Fixing

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-03

576. By Arthur Mello on 2014-07-03: Merge with trunk
577. By Arthur Mello on 2014-07-03: Improve code for newTabView loader

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-03:

#

> 114 + onUrlChanged: {
> 119 + function loadNewTabView() {
> 125 + function unloadNewTabView() {
>
> There is a more declarative way of writing this: have a Component that
> contains a NewTabView inside the loader, and set the 'sourceComponent'
> property of the loader to "parent.url ? undefined : newTabViewComponent".

Fixed on revision 577

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-03

578. By Arthur Mello on 2014-07-03: Stop changing the height property of the topSites Flow since that results on strange behavior

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-03:

#

> Regarding the transitions/animations, they’re not perfect, but I think they’ll
> do for now, we can tweak them when we implement the new design.
>
> I’m seeing a new issue though (or maybe it was there already and I hadn’t
> spotted it before): on desktop, if I have more than 5 bookmarks and more than
> 5 top sites, only the first row of the top sites category can be seen, I can
> see the top of the second row but I cannot drag it into view, it always
> springs back to the first row.

Fixed on revision 578

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-03

579. By Arthur Mello on 2014-07-03: Move newTabView inside a Component
580. By Arthur Mello on 2014-07-03: Fix url check to see if should load newTabComponent

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-03:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-03:

#

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

review: Approve (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-03:

#

It looks like the autopilot tests need updating, see https://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/1821/testReport/.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

Huh, nevermind that last comment, I was looking at a previous CI run.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

A tiny remark on semantics:

943 + def ensure_new_tab_view_visible(self):
947 + def ensure_new_tab_view_eventually_hidden(self):

Those two methods should be renamed:

assert_new_tab_view_eventually_visible()
assert_new_tab_view_eventually_hidden()

Methods that start with "ensure_" actually perform some actions to ensure that the result is achieved, as opposed to methods that start with "assert_" that merely wait until the condition is verified.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

971 + def test_hidden_new_tab_view(self):

I would rename the above method "test_new_tab_view()".

We could do with more autopilot tests (opening a bookmarked site, opening a top visited site), but this doesn’t have to be implemented now.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

1197 + void shouldLimitEntries()
1198 + {
1199 + model->setLimit(2);
1200 +
1201 + history->add(QUrl("http://example1.org/"), "Example 1 Domain", QUrl());
1202 + QTest::qWait(1001);
1203 + history->add(QUrl("http://example2.org/"), "Example 2 Domain", QUrl());
1204 + QTest::qWait(1001);
1205 + history->add(QUrl("http://example3.org/"), "Example 3 Domain", QUrl());
1206 +
1207 + QCOMPARE(model->rowCount(), 2);
1208 + QCOMPARE(model->unlimitedRowCount(), 3);
1209 + }

It would be useful to test additional conditions:
- checking that the initial value of the limit is -1
- setting the limit to -1
- setting the limit to a number greater than the number of entries
- setting the limit after populating the history model, to verify that it is correctly updated

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

Can you add a comment to state that LimitProxyModel was copied from unity8’s source tree?
Did you have to modify it, or is it a verbatim copy?

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

386 + property alias favIcon: favIcon.source
387 + property alias url: url.text
388 + property alias label: label.text

Can the 'favIcon' property be renamed 'icon', for consistency across the codebase?
Can the 'label' property be renamed 'title', to better express what it contains?

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

36 + width: parent.width

this property shouldn’t be defined here in the component, but rather at the place in the code where it is instantiated, i.e.:

    BookmarksList {
        width: parent.width
    }

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

223 + onVisibleChanged: {
224 + if (visible)
225 + updateSectionsDisplayed();
226 + }

Is this really needed? I have the impression that now that the view is instantiated and destroyed on demand, it could be removed.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

> Is this really needed? I have the impression that now that the view is
> instantiated and destroyed on demand, it could be removed.

Well, granted: sectionsModel would need to be populated when instantiating the view, but it could be done statically inside the ListModel instead of dynamically.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

209 + property int bookmarksCountLimit: 5
210 + property bool seeMoreBookmarksView: false

It looks like those two properties don’t need to be exposed publicly, as they’re not being used anywhere outside the component. Can they be made private, by moving them to a QtObject instance with id 'private'?

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

Can you please update the translations template?

cmake .
make webbrowser-app.pot

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

Apart from the handful of minor remarks above, the functionality looks good and works fine, so once all the comments are addressed, this will be ok to merge.

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-04

581. By Arthur Mello on 2014-07-04: Change AP test methods names for semantics reasons

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-04:

#

> A tiny remark on semantics:
>
> 943 + def ensure_new_tab_view_visible(self):
> 947 + def ensure_new_tab_view_eventually_hidden(self):
>
> Those two methods should be renamed:
>
> assert_new_tab_view_eventually_visible()
> assert_new_tab_view_eventually_hidden()
>
> Methods that start with "ensure_" actually perform some actions to ensure that
> the result is achieved, as opposed to methods that start with "assert_" that
> merely wait until the condition is verified.

Fixed on revision 581

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-04

582. By Arthur Mello on 2014-07-04: Rename AP method

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-04:

#

> 971 + def test_hidden_new_tab_view(self):
>
> I would rename the above method "test_new_tab_view()".
>
> We could do with more autopilot tests (opening a bookmarked site, opening a
> top visited site), but this doesn’t have to be implemented now.

Fixed on revision 582

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-04

583. By Arthur Mello on 2014-07-04: Add additional conditions to Limit Proxy Model unittest

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-04:

#

> 1197 + void shouldLimitEntries()
> 1198 + {
> 1199 + model->setLimit(2);
> 1200 +
> 1201 + history->add(QUrl("http://example1.org/"), "Example 1
> Domain", QUrl());
> 1202 + QTest::qWait(1001);
> 1203 + history->add(QUrl("http://example2.org/"), "Example 2
> Domain", QUrl());
> 1204 + QTest::qWait(1001);
> 1205 + history->add(QUrl("http://example3.org/"), "Example 3
> Domain", QUrl());
> 1206 +
> 1207 + QCOMPARE(model->rowCount(), 2);
> 1208 + QCOMPARE(model->unlimitedRowCount(), 3);
> 1209 + }
>
> It would be useful to test additional conditions:
> - checking that the initial value of the limit is -1
> - setting the limit to -1
> - setting the limit to a number greater than the number of entries
> - setting the limit after populating the history model, to verify that it is
> correctly updated

Fixed on revision 583

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-04

584. By Arthur Mello on 2014-07-04: State that LimitProxyModel was copied from unity8’s source tree with small changes

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-04:

#

> Can you add a comment to state that LimitProxyModel was copied from unity8’s
> source tree?
> Did you have to modify it, or is it a verbatim copy?

Fixed on revision 584

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-04

585. By Arthur Mello on 2014-07-04: Rename UrlDelegate properties

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-04:

#

> 386 + property alias favIcon: favIcon.source
> 387 + property alias url: url.text
> 388 + property alias label: label.text
>
> Can the 'favIcon' property be renamed 'icon', for consistency across the
> codebase?
> Can the 'label' property be renamed 'title', to better express what it
> contains?

Fixed on revision 585

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-04

586. By Arthur Mello on 2014-07-04: Fix width definition of component

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-04:

#

> 36 + width: parent.width
>
> this property shouldn’t be defined here in the component, but rather at the
> place in the code where it is instantiated, i.e.:
>
> BookmarksList {
> width: parent.width
> }

Fixed on revision 586

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-04

587. By Arthur Mello on 2014-07-04: Populate sectionsMode statically

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-04:

#

> > Is this really needed? I have the impression that now that the view is
> > instantiated and destroyed on demand, it could be removed.
>
> Well, granted: sectionsModel would need to be populated when instantiating the
> view, but it could be done statically inside the ListModel instead of
> dynamically.

Fixed on revision 587

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-04

588. By Arthur Mello on 2014-07-04: Change newTabView properties to private

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-04:

#

> 209 + property int bookmarksCountLimit: 5
> 210 + property bool seeMoreBookmarksView: false
>
> It looks like those two properties don’t need to be exposed publicly, as
> they’re not being used anywhere outside the component. Can they be made
> private, by moving them to a QtObject instance with id 'private'?

Fixed on revision 588

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-04

589. By Arthur Mello on 2014-07-04: Fix AP test
590. By Arthur Mello on 2014-07-04: Merge with trunk
591. By Arthur Mello on 2014-07-04: update the translations template

Revision history for this message

Arthur Mello (artmello) wrote on 2014-07-04:

#

> Can you please update the translations template?
>
> cmake .
> make webbrowser-app.pot

Fixed on revision 591

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

319 + end: new Date()

This is actually not needed at all, if no 'end' marker is specified, the timeframe will be open on this side, which suits our use case.

lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites updated on 2014-07-04

592. By Arthur Mello on 2014-07-04: Rmeove unnecessary code

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-04:

#

That looks good now. Thanks for your work on this!

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-05:

#

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

review: Needs Fixing (continuous-integration)

webbrowser-app

Merge lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_top_sites into lp:webbrowser-app

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'po/webbrowser-app.pot'
 --- po/webbrowser-app.pot	2014-06-25 16:34:08 +0000
 +++ po/webbrowser-app.pot	2014-07-04 14:19:02 +0000
@@ -8,7 +8,7 @@
  msgstr ""
  "Project-Id-Version: webbrowser-app\n"
  "Report-Msgid-Bugs-To: \n"
--"POT-Creation-Date: 2014-06-25 17:33+0100\n"
++"POT-Creation-Date: 2014-07-04 10:06-0300\n"
  "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
  "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
  "Language-Team: LANGUAGE <LL@li.org>\n"
@@ -81,15 +81,15 @@
  msgid "Back to safety"
  msgstr ""
--#: src/app/Chrome.qml:75 src/app/actions/Back.qml:23
++#: src/app/Chrome.qml:76 src/app/actions/Back.qml:23
  msgid "Back"
  msgstr ""
--#: src/app/Chrome.qml:94 src/app/actions/Forward.qml:23
++#: src/app/Chrome.qml:95 src/app/actions/Forward.qml:23
  msgid "Forward"
  msgstr ""
--#: src/app/Chrome.qml:157 src/app/webbrowser/ActivityView.qml:36
++#: src/app/Chrome.qml:158 src/app/webbrowser/ActivityView.qml:36
  msgid "Activity"
  msgstr ""
@@ -249,9 +249,22 @@
  msgstr ""
  #: src/app/webbrowser/ActivityView.qml:54
++#: src/app/webbrowser/NewTabView.qml:123
  msgid "Bookmarks"
  msgstr ""
++#: src/app/webbrowser/NewTabView.qml:125
++msgid "Top sites"
++msgstr ""
++
++#: src/app/webbrowser/NewTabView.qml:142
++msgid "see less"
++msgstr ""
++
++#: src/app/webbrowser/NewTabView.qml:142
++msgid "see more"
++msgstr ""
++
  #. TRANSLATORS: %1 refers to the number of open tabs
  #: src/app/webbrowser/TabsList.qml:37
  #, qt-format
@@ -307,13 +320,13 @@
  msgstr ""
  #. TRANSLATORS: %1 refers to the current page’s title
--#: src/app/webbrowser/webbrowser-app.qml:35
++#: src/app/webbrowser/webbrowser-app.qml:36
  #: src/app/webcontainer/webapp-container.qml:54
  #, qt-format
  msgid "%1 - Ubuntu Web Browser"
  msgstr ""
--#: src/app/webbrowser/webbrowser-app.qml:37
++#: src/app/webbrowser/webbrowser-app.qml:38
  #: src/app/webcontainer/webapp-container.qml:56
  msgid "Ubuntu Web Browser"
  msgstr ""
 === added file 'src/app/webbrowser/BookmarksList.qml'
 --- src/app/webbrowser/BookmarksList.qml	1970-01-01 00:00:00 +0000
 +++ src/app/webbrowser/BookmarksList.qml	2014-07-04 14:19:02 +0000
@@ -0,0 +1,75 @@
++/*
++ * Copyright 2014 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++import QtQuick 2.0
++import Ubuntu.Components 0.1
++
++Column {
++    id: bookmarksList
++
++    property alias model: bookmarksListRepeater.model
++    property alias footerLabelText: footerLabel.text
++    property alias footerLabelVisible: footerLabel.visible
++
++    signal bookmarkClicked(url url)
++    signal footerLabelClicked()
++
++    spacing: units.gu(1)
++
++    move: Transition { UbuntuNumberAnimation { properties: "x, y" } }
++
++    Repeater {
++        id: bookmarksListRepeater
++
++        delegate: UrlDelegate{
++            width: bookmarksList.width
++            height: units.gu(5)
++
++            icon: model.icon
++            title: model.title ? model.title : model.url
++            url: model.url
++
++            onClicked: bookmarkClicked(model.url)
++        }
++    }
++
++    Rectangle {
++        width: parent.width
++        height: footerLabel.visible ? footerLabel.height + units.gu(6) : 0
++
++        MouseArea {
++            anchors.centerIn: footerLabel
++
++            width: footerLabel.width + units.gu(4)
++            height: footerLabel.height + units.gu(4)
++
++            enabled: footerLabel.visible
++
++            onClicked: footerLabelClicked()
++        }
++
++        Label {
++            id: footerLabel
++            anchors.centerIn: parent
++
++            visible: true
++
++            font.bold: true
++        }
++    }
++}
 === modified file 'src/app/webbrowser/Browser.qml'
 --- src/app/webbrowser/Browser.qml	2014-06-30 10:40:59 +0000
 +++ src/app/webbrowser/Browser.qml	2014-07-04 14:19:02 +0000
@@ -48,7 +48,7 @@
          },
          Actions.Bookmark {
              enabled: currentWebview
--            onTriggered: bookmarksModel.add(currentWebview.url, currentWebview.title, "")//currentWebview.icon)
++            onTriggered: _bookmarksModel.add(currentWebview.url, currentWebview.title, "")//currentWebview.icon)
          },
          Actions.NewTab {
              onTriggered: openUrlInNewTab("", true)
@@ -118,6 +118,11 @@
              currentWebview.url = url
              toggleActivityView()
+         }
++
++        function onNewTabUrlRequested(url) {
++            currentWebview.url = url
++            currentWebview.forceActiveFocus()
++        }
+     }
      readonly property bool activityViewVisible: stack.depth > 0
@@ -126,7 +131,7 @@
          stack.push(Qt.resolvedUrl("ActivityView.qml"),
                     {tabsModel: tabsModel,
                      historyModel: _historyModel,
--                    bookmarksModel: bookmarksModel})
++                    bookmarksModel: _bookmarksModel})
          var view = stack.currentPage
          view.onHistoryEntryRequested.connect(internal.onHistoryEntryRequested)
          view.onNewTabRequested.connect(internal.onNewTabRequested)
@@ -209,7 +214,7 @@
+     }
      BookmarksModel {
--        id: bookmarksModel
++        id: _bookmarksModel
          databasePath: dataLocation + "/bookmarks.sqlite"
+     }
@@ -236,7 +241,7 @@
+                 }
                  Actions.BookmarkLink {
                      enabled: contextualData.href.toString()
--                    onTriggered: bookmarksModel.add(contextualData.href, contextualData.title, "")
++                    onTriggered: _bookmarksModel.add(contextualData.href, contextualData.title, "")
+                 }
                  Actions.CopyLink {
                      enabled: contextualData.href.toString()
@@ -276,6 +281,26 @@
                      visible = Qt.binding(function() { return current })
+                 }
+             }
++
++            Loader {
++                id: newTabViewLoader
++                anchors.fill: parent
++
++                sourceComponent: !parent.url.toString() ? newTabViewComponent : undefined
++
++                Component {
++                    id: newTabViewComponent
++
++                    NewTabView {
++                        anchors.fill: parent
++
++                        historyModel: _historyModel
++                        bookmarksModel: _bookmarksModel
++                        onBookmarkClicked: internal.onNewTabUrlRequested(url)
++                        onHistoryEntryClicked: internal.onNewTabUrlRequested(url)
++                    }
++                }
++            }
+         }
+     }
 === modified file 'src/app/webbrowser/CMakeLists.txt'
 --- src/app/webbrowser/CMakeLists.txt	2014-06-30 10:40:59 +0000
 +++ src/app/webbrowser/CMakeLists.txt	2014-07-04 14:19:02 +0000
@@ -13,7 +13,9 @@
      history-domainlist-model.cpp
      history-matches-model.cpp
      history-model.cpp
++    history-byvisits-model.cpp
      history-timeframe-model.cpp
++    limit-proxy-model.cpp
      tabs-model.cpp
+ )
 === added file 'src/app/webbrowser/NewTabView.qml'
 --- src/app/webbrowser/NewTabView.qml	1970-01-01 00:00:00 +0000
 +++ src/app/webbrowser/NewTabView.qml	2014-07-04 14:19:02 +0000
@@ -0,0 +1,176 @@
++/*
++ * Copyright 2014 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++import QtQuick 2.0
++import Ubuntu.Components 0.1
++import Ubuntu.Components.ListItems 0.1 as ListItem
++import webbrowserapp.private 0.1
++
++Item {
++    id: newTabView
++
++    property QtObject bookmarksModel
++    property QtObject historyModel
++
++    signal bookmarkClicked(url url)
++    signal historyEntryClicked(url url)
++
++    QtObject {
++        id: internal
++
++        property int bookmarksCountLimit: 5
++        property bool seeMoreBookmarksView: false
++    }
++
++    ListModel {
++        id: sectionsModel
++
++        Component.onCompleted: {
++            if (bookmarksListModel && bookmarksListModel.count !== 0)
++                sectionsModel.append({ section: "bookmarks" });
++            if (historyListModel && historyListModel.count !== 0 && !internal.seeMoreBookmarksView )
++                sectionsModel.append({ section: "topsites" });
++        }
++    }
++
++    LimitProxyModel {
++        id: bookmarksListModel
++
++        sourceModel: newTabView.bookmarksModel
++
++        limit: internal.seeMoreBookmarksView ? -1 : internal.bookmarksCountLimit
++    }
++
++    LimitProxyModel {
++        id: historyListModel
++
++        sourceModel: HistoryByVisitsModel {
++            sourceModel: HistoryTimeframeModel {
++                sourceModel: newTabView.historyModel
++                // We only show sites visited on the last 60 days
++                start: {
++                    var date = new Date()
++                    date.setDate(date.getDate() - 60)
++                    return date
++                }
++            }
++        }
++
++        limit: 10
++    }
++
++    Rectangle {
++        id: newTabBackground
++        anchors.fill: parent
++        color: "white"
++    }
++
++    ListView {
++        id: newTabListView
++        anchors.fill: parent
++
++        model: sectionsModel
++
++        delegate: Loader {
++            anchors {
++                left: parent.left
++                right: parent.right
++                margins: units.gu(2)
++            }
++
++            width: parent.width
++            height: children.height
++
++            sourceComponent: modelData == "bookmarks" ? bookmarksComponent : topSitesComponent
++        }
++
++        section.property: "section"
++        section.delegate: Rectangle {
++            anchors {
++                left: parent.left
++                right: parent.right
++            }
++
++            height: sectionHeader.height + units.gu(1)
++
++            opacity: section == "topsites" && internal.seeMoreBookmarksView ? 0.0 : 1.0
++
++            color: newTabBackground.color
++
++            Behavior on opacity { UbuntuNumberAnimation {} }
++
++            ListItem.Header {
++                id: sectionHeader
++
++                text: {
++                    if (section == "bookmarks") {
++                        return i18n.tr("Bookmarks")
++                    } else if (section == "topsites") {
++                        return i18n.tr("Top sites")
++                    }
++                }
++            }
++        }
++
++        section.labelPositioning: ViewSection.InlineLabels | ViewSection.CurrentLabelAtStart
++    }
++
++    Component {
++        id: bookmarksComponent
++
++        BookmarksList {
++            width: parent.width
++
++            model: bookmarksListModel
++
++            footerLabelText: internal.seeMoreBookmarksView ? i18n.tr("see less") : i18n.tr("see more")
++            footerLabelVisible: bookmarksListModel.unlimitedCount > internal.bookmarksCountLimit
++
++            onBookmarkClicked: newTabView.bookmarkClicked(url)
++            onFooterLabelClicked: internal.seeMoreBookmarksView = !internal.seeMoreBookmarksView
++        }
++    }
++
++    Component {
++        id: topSitesComponent
++
++        Flow {
++            width: parent.width
++
++            spacing: units.gu(1)
++
++            opacity: internal.seeMoreBookmarksView ? 0.0 : 1.0
++
++            Behavior on opacity { UbuntuNumberAnimation {} }
++
++            Repeater {
++                model: parent.opacity == 0.0 ? "" : historyListModel
++
++                delegate: PageDelegate{
++                    width: units.gu(18)
++                    height: units.gu(25)
++
++                    url: model.url
++                    label: model.title ? model.title : model.url
++
++                    onClicked: historyEntryClicked(model.url)
++                }
++            }
++        }
++    }
++}
 === added file 'src/app/webbrowser/UrlDelegate.qml'
 --- src/app/webbrowser/UrlDelegate.qml	1970-01-01 00:00:00 +0000
 +++ src/app/webbrowser/UrlDelegate.qml	2014-07-04 14:19:02 +0000
@@ -0,0 +1,73 @@
++/*
++ * Copyright 2014 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++import QtQuick 2.0
++import Ubuntu.Components 0.1
++
++Item {
++    id: urlDelegate
++
++    property alias icon: icon.source
++    property alias url: url.text
++    property alias title: title.text
++
++    signal clicked()
++
++    MouseArea {
++        anchors.fill: parent
++        onClicked: urlDelegate.clicked()
++    }
++
++    Row {
++        anchors.fill: parent
++        spacing: units.gu(1)
++
++        UbuntuShape {
++            id: iconShape
++            height: parent.height
++            width: parent.height
++
++            image: Image {
++                id: icon
++            }
++        }
++
++        Column {
++            width: parent.width - iconShape.width - spacing
++            Label {
++                id: title
++                width: parent.width
++                font.bold: true
++
++                wrapMode: Text.Wrap
++                elide: Text.ElideRight
++                maximumLineCount: 1
++            }
++
++            Label {
++                id: url
++                width: parent.width
++                fontSize: "small"
++
++                wrapMode: Text.Wrap
++                elide: Text.ElideRight
++                maximumLineCount: 1
++            }
++        }
++    }
++}
 === added file 'src/app/webbrowser/history-byvisits-model.cpp'
 --- src/app/webbrowser/history-byvisits-model.cpp	1970-01-01 00:00:00 +0000
 +++ src/app/webbrowser/history-byvisits-model.cpp	2014-07-04 14:19:02 +0000
@@ -0,0 +1,50 @@
++/*
++ * Copyright 2014 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++#include "history-byvisits-model.h"
++#include "history-timeframe-model.h"
++#include "history-model.h"
++
++/*!
++    \class HistoryByVisitsModel
++    \brief Proxy model that sorts a history model by number of visits
++
++    HistoryByVisitsModel is a proxy model that sorts a
++    HistoryTimeframeModel by the number of visits
++    (i.e. the history with the greatest number of visits first).
++*/
++HistoryByVisitsModel::HistoryByVisitsModel(QObject* parent)
++    : QSortFilterProxyModel(parent)
++{
++    setDynamicSortFilter(true);
++    setSortRole(HistoryModel::Visits);
++    sort(0, Qt::DescendingOrder);
++}
++
++HistoryTimeframeModel* HistoryByVisitsModel::sourceModel() const
++{
++    return qobject_cast<HistoryTimeframeModel*>(QSortFilterProxyModel::sourceModel());
++}
++
++void HistoryByVisitsModel::setSourceModel(HistoryTimeframeModel* sourceModel)
++{
++    if (sourceModel != this->sourceModel()) {
++        QSortFilterProxyModel::setSourceModel(sourceModel);
++        Q_EMIT sourceModelChanged();
++    }
++}
 === added file 'src/app/webbrowser/history-byvisits-model.h'
 --- src/app/webbrowser/history-byvisits-model.h	1970-01-01 00:00:00 +0000
 +++ src/app/webbrowser/history-byvisits-model.h	2014-07-04 14:19:02 +0000
@@ -0,0 +1,43 @@
++/*
++ * Copyright 2014 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++#ifndef __HISTORY_BYVISITS_MODEL_H__
++#define __HISTORY_BYVISITS_MODEL_H__
++
++// Qt
++#include <QtCore/QSortFilterProxyModel>
++
++class HistoryTimeframeModel;
++
++class HistoryByVisitsModel : public QSortFilterProxyModel
++{
++    Q_OBJECT
++
++    Q_PROPERTY(HistoryTimeframeModel* sourceModel READ sourceModel WRITE setSourceModel NOTIFY sourceModelChanged)
++
++public:
++    HistoryByVisitsModel(QObject* parent=0);
++
++    HistoryTimeframeModel* sourceModel() const;
++    void setSourceModel(HistoryTimeframeModel* sourceModel);
++
++Q_SIGNALS:
++    void sourceModelChanged() const;
++};
++
++#endif // __HISTORY_BYVISITS_MODEL_H__
 === added file 'src/app/webbrowser/limit-proxy-model.cpp'
 --- src/app/webbrowser/limit-proxy-model.cpp	1970-01-01 00:00:00 +0000
 +++ src/app/webbrowser/limit-proxy-model.cpp	2014-07-04 14:19:02 +0000
@@ -0,0 +1,268 @@
++/*
++ * Copyright 2014 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++#include <QtCore/QAbstractItemModel>
++
++#include "limit-proxy-model.h"
++
++/*!
++    \class LimitProxyModel
++    \brief Identity model that limits the number of rows returned by a model
++
++    LimitProxyModel is a proxy model that limits the number
++    of rows returned by a model
++    (i.e. only the first N entries are returned).
++
++    This proxy model was copied from unity8's source tree with small changes,
++    like the type of the sourceModel.
++*/
++LimitProxyModel::LimitProxyModel(QObject* parent)
++    : QIdentityProxyModel(parent),
++    m_limit(-1),
++    m_sourceInserting(false),
++    m_sourceRemoving(false),
++    m_dataChangedBegin(-1),
++    m_dataChangedEnd(-1)
++
++{
++    connect(this, SIGNAL(modelReset()), SIGNAL(countChanged()));
++    connect(this, SIGNAL(rowsInserted(QModelIndex,int,int)), SIGNAL(countChanged()));
++    connect(this, SIGNAL(rowsRemoved(QModelIndex,int,int)), SIGNAL(countChanged()));
++    connect(this, SIGNAL(rowsInserted(QModelIndex,int,int)), SIGNAL(unlimitedCountChanged()));
++    connect(this, SIGNAL(rowsRemoved(QModelIndex,int,int)), SIGNAL(unlimitedCountChanged()));
++}
++
++QAbstractItemModel* LimitProxyModel::sourceModel() const
++{
++    return qobject_cast<QAbstractItemModel*>(QIdentityProxyModel::sourceModel());
++}
++
++void LimitProxyModel::setSourceModel(QObject* sourceModel)
++{
++    QAbstractItemModel* model = qobject_cast<QAbstractItemModel*>(sourceModel);
++
++    if (model != this->sourceModel()) {
++        if (this->sourceModel() != NULL) {
++            this->sourceModel()->disconnect(this);
++        }
++
++        QIdentityProxyModel::setSourceModel(model);
++
++        if (this->sourceModel() != NULL) {
++            // Disconnect the QIdentityProxyModel handling for rows removed/added...
++            disconnect(this->sourceModel(), SIGNAL(rowsAboutToBeInserted(QModelIndex,int,int)), this, NULL);
++            disconnect(this->sourceModel(), SIGNAL(rowsInserted(QModelIndex,int,int)), this, NULL);
++            disconnect(this->sourceModel(), SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int)), this, NULL);
++            disconnect(this->sourceModel(), SIGNAL(rowsRemoved(QModelIndex,int,int)), this, NULL);
++
++            // ... and use our own
++            connect(this->sourceModel(), SIGNAL(rowsAboutToBeInserted(QModelIndex,int,int)),
++                    this, SLOT(sourceRowsAboutToBeInserted(QModelIndex,int,int)));
++            connect(this->sourceModel(), SIGNAL(rowsInserted(QModelIndex,int,int)),
++                    this, SLOT(sourceRowsInserted(QModelIndex,int,int)));
++            connect(this->sourceModel(), SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int)),
++                    this, SLOT(sourceRowsAboutToBeRemoved(QModelIndex,int,int)));
++            connect(this->sourceModel(), SIGNAL(rowsRemoved(QModelIndex,int,int)),
++                    this, SLOT(sourceRowsRemoved(QModelIndex,int,int)));
++        }
++        Q_EMIT sourceModelChanged();
++    }
++}
++
++int LimitProxyModel::rowCount(const QModelIndex &parent) const
++{
++    if (parent.isValid()) // We are not a tree
++        return 0;
++
++    const int unlimitedCount = QIdentityProxyModel::rowCount(parent);
++    return m_limit < 0 ? unlimitedCount : qMin(m_limit, unlimitedCount);
++}
++
++int LimitProxyModel::unlimitedRowCount(const QModelIndex &parent) const
++{
++    if (parent.isValid()) // We are not a tree
++        return 0;
++
++    return QIdentityProxyModel::rowCount(parent);
++}
++
++int LimitProxyModel::limit() const
++{
++    return m_limit;
++}
++
++void LimitProxyModel::setLimit(int limit)
++{
++    if (limit != m_limit) {
++        bool inserting = false;
++        bool removing = false;
++        const int oldCount = rowCount();
++        const int unlimitedCount = QIdentityProxyModel::rowCount();
++        if (m_limit < 0) {
++            if (limit < oldCount) {
++                removing = true;
++                beginRemoveRows(QModelIndex(), limit, oldCount - 1);
++            }
++        } else if (limit < 0) {
++            if (m_limit < unlimitedCount) {
++                inserting = true;
++                beginInsertRows(QModelIndex(), m_limit, unlimitedCount - 1);
++            }
++        } else {
++            if (limit > m_limit && unlimitedCount > m_limit) {
++                inserting = true;
++                beginInsertRows(QModelIndex(), m_limit, qMin(limit, unlimitedCount) - 1);
++            } else if (limit < m_limit && limit < oldCount) {
++                removing = true;
++                beginRemoveRows(QModelIndex(), limit, oldCount - 1);
++            }
++        }
++
++        m_limit = limit;
++
++        if (inserting) {
++            endInsertRows();
++        } else if (removing) {
++            endRemoveRows();
++        }
++
++        Q_EMIT limitChanged();
++    }
++}
++
++void LimitProxyModel::sourceRowsAboutToBeInserted(const QModelIndex &parent, int start, int end)
++{
++    if (m_limit < 0) {
++        beginInsertRows(mapFromSource(parent), start, end);
++        m_sourceInserting = true;
++    } else if (start < m_limit) {
++        const int nSourceAddedItems = end - start + 1;
++        const int currentCount = QIdentityProxyModel::rowCount();
++        if (currentCount + nSourceAddedItems <= m_limit) {
++            // After Inserting items we will be under the limit
++            // so just proceed with the insertion normally
++            beginInsertRows(mapFromSource(parent), start, end);
++            m_sourceInserting = true;
++        } else if (currentCount >= m_limit) {
++            // We are already over the limit so to our users we are not adding items, just
++            // changing it's data, i.e we had something like
++            // A B C D E
++            // with a limit of 5
++            // after inserting (let's say three 'F' at position 1) we will have
++            // A F F F B
++            // so we just need to signal a dataChanged from 1 to 4
++            m_dataChangedBegin = start;
++            m_dataChangedEnd = m_limit - 1;
++        } else { // currentCount < m_limit && currentCount + nSourceAddedItems > m_limit
++            // We have less items than the limit but after adding them we will be over
++            // To our users this means we need to insert some items and change the
++            // data of some others, i.e we had something like
++            // A B C
++            // with a limit of 5
++            // after inserting (let's say three 'F' at position 1) we will have
++            // A F F F B
++            // so we need to signal an insetion from position 1 to 2, instead of from
++            // position 1 to 3 and a after that a data changed from 3 to 4
++            const int nItemsToInsert = m_limit - currentCount;
++            beginInsertRows(mapFromSource(parent), start, start + nItemsToInsert - 1);
++            m_sourceInserting = true;
++            m_dataChangedBegin = start + nItemsToInsert;
++            m_dataChangedEnd = m_limit - 1;
++            if (m_dataChangedBegin > m_dataChangedEnd) {
++                // Just in case we were empty and insert 6 items with a limit of 5
++                // We don't want to signal a dataChanged from 5 to 4
++                m_dataChangedBegin = -1;
++                m_dataChangedEnd = -1;
++            }
++        }
++    }
++}
++
++void LimitProxyModel::sourceRowsAboutToBeRemoved(const QModelIndex &parent, int start, int end)
++{
++    if (m_limit < 0) {
++        beginRemoveRows(mapFromSource(parent), start, end);
++        m_sourceRemoving = true;
++    } else if (start < m_limit) {
++        const int nSourceRemovedItems = end - start + 1;
++        const int currentCount = QIdentityProxyModel::rowCount();
++        if (currentCount <= m_limit) {
++            // We are already under the limit so
++            // so just proceed with the removal normally
++            beginRemoveRows(mapFromSource(parent), start, end);
++            m_sourceRemoving = true;
++        } else if (currentCount - nSourceRemovedItems >= m_limit) {
++            // Even after removing items we will be at or over the limit
++            // So to our users we are not removing anything, just changing the data
++            // i.e. we had a internal model with
++            // A B C D E F G H
++            // and a limit of 5, our users just see
++            // A B C D E
++            // so if we remove 3 items starting at 1 we have to expose
++            // A E F G H
++            // that is, a dataChanged from 1 to 4
++            m_dataChangedBegin = start;
++            m_dataChangedEnd = m_limit - 1;
++        } else { // currentCount > m_limit && currentCount - nSourceRemovedItems < m_limit
++            // We have more items than the limit but after removing we will be below it
++            // So to our users we both removing and changing the data
++            // i.e. we had a internal model with
++            // A B C D E F G
++            // and a limit of 5, our users just see
++            // A B C D E
++            // so if we remove items from 1 to 3 we have to expose
++            // A E F G
++            // that is, a remove from 4 to 4 and a dataChanged from 1 to 3
++            const int nItemsToRemove = m_limit - (currentCount - nSourceRemovedItems);
++            beginRemoveRows(mapFromSource(parent), m_limit - nItemsToRemove, m_limit - 1);
++            m_sourceRemoving = true;
++            m_dataChangedBegin = start;
++            m_dataChangedEnd = m_limit - nItemsToRemove - 1;
++            if (m_dataChangedBegin > m_dataChangedEnd) {
++                m_dataChangedBegin = -1;
++                m_dataChangedEnd = -1;
++            }
++        }
++    }
++}
++
++void LimitProxyModel::sourceRowsInserted(const QModelIndex & /*parent*/, int /*start*/, int /*end*/)
++{
++    if (m_sourceInserting) {
++        endInsertRows();
++        m_sourceInserting = false;
++    }
++    if (m_dataChangedBegin != -1 && m_dataChangedEnd != -1) {
++        dataChanged(index(m_dataChangedBegin, 0), index(m_dataChangedEnd, 0));
++        m_dataChangedBegin = -1;
++        m_dataChangedEnd = -1;
++    }
++}
++
++void LimitProxyModel::sourceRowsRemoved(const QModelIndex & /*parent*/, int /*start*/, int /*end*/)
++{
++    if (m_sourceRemoving) {
++        endRemoveRows();
++        m_sourceRemoving = false;
++    }
++    if (m_dataChangedBegin != -1 && m_dataChangedEnd != -1) {
++        dataChanged(index(m_dataChangedBegin, 0), index(m_dataChangedEnd, 0));
++        m_dataChangedBegin = -1;
++        m_dataChangedEnd = -1;
++    }
++}
 === added file 'src/app/webbrowser/limit-proxy-model.h'
 --- src/app/webbrowser/limit-proxy-model.h	1970-01-01 00:00:00 +0000
 +++ src/app/webbrowser/limit-proxy-model.h	2014-07-04 14:19:02 +0000
@@ -0,0 +1,70 @@
++/*
++ * Copyright 2014 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++#ifndef __LIMIT_PROXY_MODEL_H__
++#define __LIMIT_PROXY_MODEL_H__
++
++// Qt
++#include <QtCore/QIdentityProxyModel>
++
++class QSortFilterProxyModel;
++
++class LimitProxyModel : public QIdentityProxyModel
++{
++    Q_OBJECT
++
++    Q_PROPERTY(QObject* sourceModel READ sourceModel WRITE setSourceModel NOTIFY sourceModelChanged)
++    Q_PROPERTY(int limit READ limit WRITE setLimit NOTIFY limitChanged)
++    Q_PROPERTY(int count READ rowCount NOTIFY countChanged)
++    Q_PROPERTY(int unlimitedCount READ unlimitedRowCount NOTIFY unlimitedCountChanged)
++
++public:
++    LimitProxyModel(QObject* parent=0);
++
++    QAbstractItemModel* sourceModel() const;
++    void setSourceModel(QObject* sourceModel);
++
++    int limit() const;
++    void setLimit(int limit);
++
++    int rowCount(const QModelIndex &parent = QModelIndex()) const;
++    int unlimitedRowCount(const QModelIndex &parent = QModelIndex()) const;
++
++Q_SIGNALS:
++    void sourceModelChanged() const;
++    void limitChanged() const;
++    void unlimitedCountChanged();
++    void countChanged();
++
++private Q_SLOTS:
++    void sourceRowsAboutToBeInserted(const QModelIndex &parent, int start, int
++end);
++    void sourceRowsAboutToBeRemoved(const QModelIndex &parent, int start, int
++end);
++    void sourceRowsInserted(const QModelIndex &parent, int start, int end);
++    void sourceRowsRemoved(const QModelIndex &parent, int start, int end);
++
++private:
++    int m_limit;
++    bool m_sourceInserting;
++    bool m_sourceRemoving;
++    int m_dataChangedBegin;
++    int m_dataChangedEnd;
++};
++
++#endif // __LIMIT_PROXY_MODEL_H__
 === modified file 'src/app/webbrowser/webbrowser-app.cpp'
 --- src/app/webbrowser/webbrowser-app.cpp	2014-06-30 10:40:59 +0000
 +++ src/app/webbrowser/webbrowser-app.cpp	2014-07-04 14:19:02 +0000
@@ -21,8 +21,10 @@
  #include "history-model.h"
  #include "history-matches-model.h"
  #include "history-timeframe-model.h"
++#include "history-byvisits-model.h"
  #include "history-domainlist-model.h"
  #include "history-domainlist-chronological-model.h"
++#include "limit-proxy-model.h"
  #include "searchengine.h"
  #include "settings.h"
  #include "tabs-model.h"
@@ -75,8 +77,10 @@
      qmlRegisterType<HistoryModel>(uri, 0, 1, "HistoryModel");
      qmlRegisterType<HistoryMatchesModel>(uri, 0, 1, "HistoryMatchesModel");
      qmlRegisterType<HistoryTimeframeModel>(uri, 0, 1, "HistoryTimeframeModel");
++    qmlRegisterType<HistoryByVisitsModel>(uri, 0 , 1, "HistoryByVisitsModel");
      qmlRegisterType<HistoryDomainListModel>(uri, 0, 1, "HistoryDomainListModel");
      qmlRegisterType<HistoryDomainListChronologicalModel>(uri, 0, 1, "HistoryDomainListChronologicalModel");
++    qmlRegisterType<LimitProxyModel>(uri, 0 , 1, "LimitProxyModel");
      qmlRegisterType<TabsModel>(uri, 0, 1, "TabsModel");
      qmlRegisterType<BookmarksModel>(uri, 0, 1, "BookmarksModel");
 === modified file 'tests/autopilot/webbrowser_app/emulators/browser.py'
 --- tests/autopilot/webbrowser_app/emulators/browser.py	2014-06-25 20:38:15 +0000
 +++ tests/autopilot/webbrowser_app/emulators/browser.py	2014-07-04 14:19:02 +0000
@@ -116,6 +116,12 @@
      def get_geolocation_dialog(self):
          return self.wait_select_single("GeolocationPermissionRequest")
++    def get_many_new_tab_view(self):
++        return self.select_many("NewTabView")
++
++    def get_new_tab_view(self):
++        return self.wait_select_single("NewTabView")
++
      def get_selection(self):
          return self.wait_select_single(Selection)
 === modified file 'tests/autopilot/webbrowser_app/tests/__init__.py'
 --- tests/autopilot/webbrowser_app/tests/__init__.py	2014-06-11 11:12:14 +0000
 +++ tests/autopilot/webbrowser_app/tests/__init__.py	2014-07-04 14:19:02 +0000
@@ -156,8 +156,6 @@
      def assert_activity_view_eventually_hidden(self):
          self.assertThat(self.main_window.get_many_activity_view,
                          Eventually(Equals([])))
--        self.assertThat(self.main_window.get_current_webview().visible,
--                        Eventually(Equals(True)))
      def ensure_activity_view_visible(self):
          self.ensure_chrome_is_hidden()
@@ -169,6 +167,14 @@
          ping = urllib.request.urlopen(self.base_url + "/ping")
          self.assertThat(ping.read(), Equals(b"pong"))
++    def assert_new_tab_view_eventually_visible(self):
++        new_tab_view = self.main_window.get_new_tab_view()
++        self.assertThat(new_tab_view.visible, Eventually(Equals(True)))
++
++    def assert_new_tab_view_eventually_hidden(self):
++        self.assertThat(self.main_window.get_many_new_tab_view,
++                        Eventually(Equals([])))
++
  class StartOpenRemotePageTestCaseBase(BrowserTestCaseBase):
 === modified file 'tests/autopilot/webbrowser_app/tests/test_tabs.py'
 --- tests/autopilot/webbrowser_app/tests/test_tabs.py	2014-06-27 11:51:42 +0000
 +++ tests/autopilot/webbrowser_app/tests/test_tabs.py	2014-07-04 14:19:02 +0000
@@ -77,6 +77,7 @@
          self.assertThat(self.main_window.currentIndex, Equals(0))
          self.open_new_tab()
          self.assertThat(self.main_window.currentIndex, Eventually(Equals(1)))
++        self.assert_new_tab_view_eventually_visible()
      def test_switch_tabs(self):
          self.ensure_activity_view_visible()
@@ -167,3 +168,12 @@
          tabs = self.main_window.get_tabslist_view_delegates()
          self.pointing_device.click_object(tabs[0])
          self.assertThat(error.visible, Eventually(Equals(False)))
++
++    def test_new_tab_view(self):
++        self.ensure_activity_view_visible()
++        self.open_new_tab()
++        self.assert_new_tab_view_eventually_visible()
++        url = self.base_url + "/aleaiactaest"
++        self.type_in_address_bar(url)
++        self.keyboard.press_and_release("Enter")
++        self.assert_new_tab_view_eventually_hidden()
 === modified file 'tests/unittests/CMakeLists.txt'
 --- tests/unittests/CMakeLists.txt	2014-05-22 15:27:38 +0000
 +++ tests/unittests/CMakeLists.txt	2014-07-04 14:19:02 +0000
@@ -6,8 +6,10 @@
  add_subdirectory(history-domain-model)
  add_subdirectory(history-domainlist-model)
  add_subdirectory(history-domainlist-chronological-model)
++add_subdirectory(history-byvisits-model)
  add_subdirectory(session-utils)
  add_subdirectory(tabs-model)
  add_subdirectory(bookmarks-model)
++add_subdirectory(limit-proxy-model)
  add_subdirectory(container-url-patterns)
  add_subdirectory(cookie-store)
 === added directory 'tests/unittests/history-byvisits-model'
 === added file 'tests/unittests/history-byvisits-model/CMakeLists.txt'
 --- tests/unittests/history-byvisits-model/CMakeLists.txt	1970-01-01 00:00:00 +0000
 +++ tests/unittests/history-byvisits-model/CMakeLists.txt	2014-07-04 14:19:02 +0000
@@ -0,0 +1,6 @@
++set(TEST tst_HistoryByVisitsModelTests)
++add_executable(${TEST} tst_HistoryByVisitsModelTests.cpp)
++include_directories(${webbrowser-app_SOURCE_DIR})
++qt5_use_modules(${TEST} Test)
++target_link_libraries(${TEST} webbrowser-app-models)
++add_test(${TEST} ${CMAKE_CURRENT_BINARY_DIR}/${TEST} -xunitxml -o ${TEST}.xml)
 === added file 'tests/unittests/history-byvisits-model/tst_HistoryByVisitsModelTests.cpp'
 --- tests/unittests/history-byvisits-model/tst_HistoryByVisitsModelTests.cpp	1970-01-01 00:00:00 +0000
 +++ tests/unittests/history-byvisits-model/tst_HistoryByVisitsModelTests.cpp	2014-07-04 14:19:02 +0000
@@ -0,0 +1,89 @@
++/*
++ * Copyright 2014 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++// Qt
++#include <QtTest/QSignalSpy>
++#include <QtTest/QtTest>
++
++// local
++#include "domain-utils.h"
++#include "history-model.h"
++#include "history-timeframe-model.h"
++#include "history-byvisits-model.h"
++
++class HistoryByVisitsModelTests : public QObject
++{
++    Q_OBJECT
++
++private:
++    HistoryModel* history;
++    HistoryTimeframeModel* timeframe;
++    HistoryByVisitsModel* model;
++
++private Q_SLOTS:
++    void init()
++    {
++        history = new HistoryModel;
++        history->setDatabasePath(":memory:");
++        timeframe = new HistoryTimeframeModel;
++        timeframe->setSourceModel(history);
++        model = new HistoryByVisitsModel;
++        model->setSourceModel(timeframe);
++    }
++
++    void cleanup()
++    {
++        delete model;
++        delete timeframe;
++        delete history;
++    }
++
++    void shouldBeInitiallyEmpty()
++    {
++        QCOMPARE(model->rowCount(), 0);
++    }
++
++    void shouldNotifyWhenChangingSourceModel()
++    {
++        QSignalSpy spy(model, SIGNAL(sourceModelChanged()));
++        model->setSourceModel(timeframe);
++        QVERIFY(spy.isEmpty());
++        HistoryTimeframeModel* timeframe2 = new HistoryTimeframeModel;
++        model->setSourceModel(timeframe2);
++        QCOMPARE(spy.count(), 1);
++        QCOMPARE(model->sourceModel(), timeframe2);
++        model->setSourceModel(0);
++        QCOMPARE(spy.count(), 2);
++        QCOMPARE(model->sourceModel(), (HistoryTimeframeModel*) 0);
++        delete timeframe2;
++    }
++
++    void shouldBeSortedByVisits()
++    {
++        history->add(QUrl("http://example.org/"), "Example Domain", QUrl());
++        QTest::qWait(1001);
++        history->add(QUrl("http://ubuntu.com/"), "Ubuntu", QUrl());
++        QTest::qWait(1001);
++        history->add(QUrl("http://ubuntu.com/"), "Ubuntu", QUrl());
++        QCOMPARE(model->data(model->index(0, 0), HistoryModel::Domain).toString(), QString("ubuntu.com"));
++        QCOMPARE(model->data(model->index(1, 0), HistoryModel::Domain).toString(), QString("example.org"));
++    }
++};
++
++QTEST_MAIN(HistoryByVisitsModelTests)
++#include "tst_HistoryByVisitsModelTests.moc"
 === added directory 'tests/unittests/limit-proxy-model'
 === added file 'tests/unittests/limit-proxy-model/CMakeLists.txt'
 --- tests/unittests/limit-proxy-model/CMakeLists.txt	1970-01-01 00:00:00 +0000
 +++ tests/unittests/limit-proxy-model/CMakeLists.txt	2014-07-04 14:19:02 +0000
@@ -0,0 +1,6 @@
++set(TEST tst_LimitProxyModelTests)
++add_executable(${TEST} tst_LimitProxyModelTests.cpp)
++include_directories(${webbrowser-app_SOURCE_DIR})
++qt5_use_modules(${TEST} Test)
++target_link_libraries(${TEST} webbrowser-app-models)
++add_test(${TEST} ${CMAKE_CURRENT_BINARY_DIR}/${TEST} -xunitxml -o ${TEST}.xml)
 === added file 'tests/unittests/limit-proxy-model/tst_LimitProxyModelTests.cpp'
 --- tests/unittests/limit-proxy-model/tst_LimitProxyModelTests.cpp	1970-01-01 00:00:00 +0000
 +++ tests/unittests/limit-proxy-model/tst_LimitProxyModelTests.cpp	2014-07-04 14:19:02 +0000
@@ -0,0 +1,144 @@
++/*
++ * Copyright 2014 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++// Qt
++#include <QtTest/QSignalSpy>
++#include <QtTest/QtTest>
++
++// local
++#include "domain-utils.h"
++#include "history-model.h"
++#include "history-timeframe-model.h"
++#include "history-byvisits-model.h"
++#include "limit-proxy-model.h"
++
++class LimitProxyModelTests : public QObject
++{
++    Q_OBJECT
++
++private:
++    HistoryModel* history;
++    HistoryTimeframeModel* timeframe;
++    HistoryByVisitsModel* byvisits;
++    LimitProxyModel* model;
++
++private Q_SLOTS:
++    void init()
++    {
++        history = new HistoryModel;
++        history->setDatabasePath(":memory:");
++        timeframe = new HistoryTimeframeModel;
++        timeframe->setSourceModel(history);
++        byvisits = new HistoryByVisitsModel;
++        byvisits->setSourceModel(timeframe);
++        model = new LimitProxyModel;
++        model->setSourceModel(byvisits);
++    }
++
++    void cleanup()
++    {
++        delete model;
++        delete byvisits;
++        delete timeframe;
++        delete history;
++    }
++
++    void shouldBeInitiallyEmpty()
++    {
++        QCOMPARE(model->rowCount(), 0);
++    }
++
++    void shouldLimitBeInitiallyMinusOne()
++    {
++        QCOMPARE(model->limit(), -1);
++    }
++
++    void shouldNotifyWhenChangingSourceModel()
++    {
++        QSignalSpy spy(model, SIGNAL(sourceModelChanged()));
++        model->setSourceModel(byvisits);
++        QVERIFY(spy.isEmpty());
++        HistoryByVisitsModel* byvisits2 = new HistoryByVisitsModel;
++        model->setSourceModel(byvisits2);
++        QCOMPARE(spy.count(), 1);
++        QCOMPARE(model->sourceModel(), byvisits2);
++        model->setSourceModel(0);
++        QCOMPARE(spy.count(), 2);
++        QCOMPARE(model->sourceModel(), (HistoryByVisitsModel*) 0);
++        delete byvisits2;
++    }
++
++    void shouldLimitEntriesWithLimitSetBeforePopulating()
++    {
++        model->setLimit(2);
++
++        history->add(QUrl("http://example1.org/"), "Example 1 Domain", QUrl());
++        QTest::qWait(1001);
++        history->add(QUrl("http://example2.org/"), "Example 2 Domain", QUrl());
++        QTest::qWait(1001);
++        history->add(QUrl("http://example3.org/"), "Example 3 Domain", QUrl());
++
++        QCOMPARE(model->rowCount(), 2);
++        QCOMPARE(model->unlimitedRowCount(), 3);
++    }
++
++    void shouldLimitEntriesWithLimitSetAfterPopulating()
++    {
++        history->add(QUrl("http://example1.org/"), "Example 1 Domain", QUrl());
++        QTest::qWait(1001);
++        history->add(QUrl("http://example2.org/"), "Example 2 Domain", QUrl());
++        QTest::qWait(1001);
++        history->add(QUrl("http://example3.org/"), "Example 3 Domain", QUrl());
++
++        model->setLimit(2);
++
++        QCOMPARE(model->rowCount(), 2);
++        QCOMPARE(model->unlimitedRowCount(), 3);
++    }
++
++    void shouldNotLimitEntriesIfLimitIsMinusOne()
++    {
++        model->setLimit(-1);
++
++        history->add(QUrl("http://example1.org/"), "Example 1 Domain", QUrl());
++        QTest::qWait(1001);
++        history->add(QUrl("http://example2.org/"), "Example 2 Domain", QUrl());
++        QTest::qWait(1001);
++        history->add(QUrl("http://example3.org/"), "Example 3 Domain", QUrl());
++
++        QCOMPARE(model->unlimitedRowCount(), 3);
++        QCOMPARE(model->rowCount(), model->unlimitedRowCount());
++    }
++
++    void shouldNotLimitEntriesIfLimitIsGreaterThanRowCount()
++    {
++        model->setLimit(4);
++
++        history->add(QUrl("http://example1.org/"), "Example 1 Domain", QUrl());
++        QTest::qWait(1001);
++        history->add(QUrl("http://example2.org/"), "Example 2 Domain", QUrl());
++        QTest::qWait(1001);
++        history->add(QUrl("http://example3.org/"), "Example 3 Domain", QUrl());
++
++        QCOMPARE(model->unlimitedRowCount(), 3);
++        QCOMPARE(model->rowCount(), model->unlimitedRowCount());
++    }
++};
++
++QTEST_MAIN(LimitProxyModelTests)
++#include "tst_LimitProxyModelTests.moc"