webbrowser-app

Merge lp:~osomon/webbrowser-app/save-restore-state into lp:webbrowser-app

save-restore-state
Merge into trunk

Proposed by Olivier Tilloy on 2014-11-24

Status:

Merged

Approved by:

Olivier Tilloy on 2015-02-03

Approved revision:

834

Merged at revision:

886

Proposed branch:

lp:~osomon/webbrowser-app/save-restore-state

Merge into:

lp:webbrowser-app

Diff against target:

242 lines (+45/-70)

5 files modified

debian/control (+2/-2)
src/app/webbrowser/Browser.qml (+27/-16)
src/app/webbrowser/BrowserTab.qml (+13/-1)
tests/autopilot/webbrowser_app/tests/test_session_save_restore.py (+0/-51)
tests/unittests/qml/tst_BrowserTab.qml (+3/-0)

To merge this branch:

bzr merge lp:~osomon/webbrowser-app/save-restore-state

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Michael Vogt (community)		2014-11-24	Approve on 2015-02-03
PS Jenkins bot	continuous-integration		Needs Fixing on 2015-02-03
Review via email: mp+242678@code.launchpad.net

Commit message

Use the new restoreState API in oxide 1.4 to properly save and restore navigation state across sessions.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-24:

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

review: Needs Fixing (continuous-integration)

lp:~osomon/webbrowser-app/save-restore-state updated on 2014-12-05

826. By Olivier Tilloy on 2014-12-05: Merge the latest changes from trunk,
and resolve a conflict.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-12-11:

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

review: Needs Fixing (continuous-integration)

lp:~osomon/webbrowser-app/save-restore-state updated on 2014-12-16

827. By Olivier Tilloy on 2014-12-16: Merge the latest changes from trunk,
and resolve a conflict.
828. By Olivier Tilloy on 2014-12-16: Remove a not very useful comment.
829. By Olivier Tilloy on 2014-12-16: Rename the 'blob' property to 'savedState', which is much more explicit.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-12-16:

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

review: Needs Fixing (continuous-integration)

lp:~osomon/webbrowser-app/save-restore-state updated on 2014-12-17

830. By Olivier Tilloy on 2014-12-17: Merge the latest changes from trunk,
and resolve a couple of conflicts.
831. By Olivier Tilloy on 2014-12-17: Fix BrowserTab unit tests.
832. By Olivier Tilloy on 2014-12-17: Revert accidental change introduced when merging latest trunk.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-12-17:

FAILED: Continuous integration, rev:832
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1352/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/606/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/339/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/110/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/110/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/110/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/604/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/345/console

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

review: Needs Fixing (continuous-integration)

lp:~osomon/webbrowser-app/save-restore-state updated on 2014-12-18

833. By Olivier Tilloy on 2014-12-18: Merge the latest changes from trunk, and resolve a conflict.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-12-18:

FAILED: Continuous integration, rev:833
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1359/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/627/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/357/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/117/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/117/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/117/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/625/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/363/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-28:

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

review: Needs Fixing (continuous-integration)

lp:~osomon/webbrowser-app/save-restore-state updated on 2015-01-28

834. By Olivier Tilloy on 2015-01-28: Bump the required oxide version in the build dependencies as well, since unit tests require oxide 1.4.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-28:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-02-03:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Michael Vogt (mvo) wrote on 2015-02-03:

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 'debian/control'
 --- debian/control	2015-01-19 10:45:25 +0000
 +++ debian/control	2015-01-28 14:25:23 +0000
@@ -6,7 +6,7 @@
                 debhelper (>= 9),
                 dh-translations,
                 hardening-wrapper,
--               liboxideqt-qmlplugin (>= 1.3),
++               liboxideqt-qmlplugin (>= 1.4),
                 libqt5sql5-sqlite,
                 python-flake8,
                 python3-all,
@@ -34,7 +34,7 @@
  Depends: ${misc:Depends},
           ${shlibs:Depends},
           fonts-liberation,
--         liboxideqt-qmlplugin (>= 1.1),
++         liboxideqt-qmlplugin (>= 1.4),
           libqt5sql5-sqlite,
           qml-module-qtquick2 | qtdeclarative5-qtquick2-plugin,
           qml-module-qtquick-dialogs | qtdeclarative5-dialogs-plugin,
 === modified file 'src/app/webbrowser/Browser.qml'
 --- src/app/webbrowser/Browser.qml	2014-12-17 09:22:40 +0000
 +++ src/app/webbrowser/Browser.qml	2015-01-28 14:25:23 +0000
@@ -17,7 +17,7 @@
   */
  import QtQuick 2.0
--import com.canonical.Oxide 1.0 as Oxide
++import com.canonical.Oxide 1.4 as Oxide
  import Ubuntu.Components 1.1
  import webbrowserapp.private 0.1
  import webbrowsercommon.private 0.1
@@ -415,7 +415,18 @@
                      id: newTabViewLoader
                      anchors.fill: parent
--                    sourceComponent: !parent.url.toString() ? newTabViewComponent : undefined
++                    // Avoid loading the new tab view if the webview is about to load
++                    // content. Since WebView.restoreState is not a notifyable property,
++                    // this can’t be achieved with a simple property binding.
++                    Component.onCompleted: {
++                        if (!parent.url.toString() && !parent.restoreState) {
++                            sourceComponent = newTabViewComponent
++                        }
++                    }
++                    Connections {
++                        target: newTabViewLoader.parent
++                        onUrlChanged: newTabViewLoader.sourceComponent = null
++                    }
                      Component {
                          id: newTabViewComponent
@@ -542,39 +553,39 @@
+         }
          // Those two functions are used to save/restore the current state of a tab.
--        // The current implementation is naive, it only saves/restores the current URL.
--        // In the future, we’ll want to rely on oxide to save and restore a full state
--        // of the corresponding webview as a binary blob, which includes navigation
--        // history, current scroll offset and form data. See http://pad.lv/1353143.
          function serializeTabState(tab) {
              var state = {}
              state.uniqueId = tab.uniqueId
              state.url = tab.url.toString()
              state.title = tab.title
              state.preview = tab.preview.toString()
++            state.savedState = tab.webview ? tab.webview.currentState : tab.restoreState
              return state
+         }
          function createTabFromState(state) {
--            var properties = {"initialUrl": state.url, "initialTitle": state.title}
++            var properties = {'initialUrl': state.url, 'initialTitle': state.title}
              if ('uniqueId' in state) {
                  properties["uniqueId"] = state.uniqueId
+             }
              if ('preview' in state) {
                  properties["preview"] = state.preview
+             }
++            if ('savedState' in state) {
++                properties['restoreState'] = state.savedState
++                properties['restoreType'] = Oxide.WebView.RestoreLastSessionExitedCleanly
++            }
              return tabComponent.createObject(tabContainer, properties)
+         }
+     }
      Connections {
--        target: tabsModel
--        onCurrentTabChanged: session.save()
--        onCountChanged: session.save()
--    }
--    Connections {
--        target: browser.currentWebview
--        onUrlChanged: session.save()
--        onTitleChanged: session.save()
++        target: Qt.application
++        onStateChanged: {
++            if (Qt.application.state != Qt.ApplicationActive) {
++                session.save()
++            }
++        }
++        onAboutToQuit: session.save()
+     }
      // Delay instantiation of the first webview by 1 msec to allow initial
@@ -596,7 +607,7 @@
                  browser.openUrlInNewTab(browser.homepage, true, false)
+             }
              tabsModel.currentTab.load()
--            if (!tabsModel.currentTab.url.toString() && (formFactor == "desktop")) {
++            if (!tabsModel.currentTab.url.toString() && !tabsModel.currentTab.restoreState && (formFactor == "desktop")) {
                  internal.focusAddressBar()
+             }
+         }
 === modified file 'src/app/webbrowser/BrowserTab.qml'
 --- src/app/webbrowser/BrowserTab.qml	2014-12-16 16:25:58 +0000
 +++ src/app/webbrowser/BrowserTab.qml	2015-01-28 14:25:23 +0000
@@ -17,12 +17,15 @@
   */
  import QtQuick 2.0
++import com.canonical.Oxide 1.4 as Oxide
  import webbrowserapp.private 0.1
  FocusScope {
      property string uniqueId: this.toString() + "-" + Date.now()
      property url initialUrl
      property string initialTitle
++    property string restoreState
++    property int restoreType
      property var request
      property Component webviewComponent
      readonly property var webview: (children.length == 1) ? children[0] : null
@@ -33,7 +36,14 @@
      function load() {
          if (!webview) {
--            webviewComponent.incubateObject(this, {"url": initialUrl})
++            var properties = {}
++            if (restoreState) {
++                properties['restoreState'] = restoreState
++                properties['restoreType'] = restoreType
++            } else {
++                properties['url'] = initialUrl
++            }
++            webviewComponent.incubateObject(this, properties)
+         }
+     }
@@ -41,6 +51,8 @@
          if (webview) {
              initialUrl = webview.url
              initialTitle = webview.title
++            restoreState = webview.currentState
++            restoreType = Oxide.WebView.RestoreCurrentSession
              webview.destroy()
+         }
+     }
 === removed file 'tests/autopilot/webbrowser_app/tests/test_session_save_restore.py'
 --- tests/autopilot/webbrowser_app/tests/test_session_save_restore.py	2014-12-18 18:39:47 +0000
 +++ tests/autopilot/webbrowser_app/tests/test_session_save_restore.py	1970-01-01 00:00:00 +0000
@@ -1,51 +0,0 @@
--# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
--#
--# Copyright 2014 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
--# by the Free Software Foundation.
--#
--# This program 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/>.
--
--from autopilot.matchers import Eventually
--from testtools.matchers import Equals
--
--from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
--
--
--class TestSessionSaveRestore(StartOpenRemotePageTestCaseBase):
--
--    def create_new_tab(self, url):
--        self.open_tabs_view()
--        self.open_new_tab()
--        new_tab_view = self.main_window.get_new_tab_view()
--        self.main_window.go_to_url(url)
--        new_tab_view.wait_until_destroyed()
--
--    def test_session_is_saved_and_restored(self):
--        paths = ["/test1", "/test2", "/wait/0"]
--        for path in paths[1:]:
--            self.create_new_tab(self.base_url + path)
--        self.open_tabs_view()
--        tabs_view = self.main_window.get_tabs_view()
--        self.assertThat(tabs_view.count, Eventually(Equals(len(paths))))
--        process = self.app.process
--        process.kill()
--        process.wait(10)
--        self.ARGS = []
--        self.app = self.launch_app()
--        self.open_tabs_view()
--        tabs_view = self.main_window.get_tabs_view()
--        self.assertThat(tabs_view.count, Eventually(Equals(len(paths))))
--        previews = tabs_view.get_ordered_previews()
--        for i in range(len(paths)):
--            self.assertThat(previews[len(paths) - 1 - i].url,
--                            Eventually(Equals(self.base_url + paths[i])))
--        self.assert_number_webviews_eventually(1)
 === modified file 'tests/unittests/qml/tst_BrowserTab.qml'
 --- tests/unittests/qml/tst_BrowserTab.qml	2014-12-16 17:24:32 +0000
 +++ tests/unittests/qml/tst_BrowserTab.qml	2015-01-28 14:25:23 +0000
@@ -35,6 +35,7 @@
                  property string title
                  property url icon
                  property var request
++                property string currentState
+             }
              readonly property bool webviewPresent: webview
              readonly property bool captureTakerPresent: captureTaker != undefined
@@ -66,10 +67,12 @@
              tab.webview.url = "http://ubuntu.com"
              tab.webview.title = "Ubuntu"
++            tab.webview.currentState = "foobar"
              tab.unload()
              tryCompare(tab, 'webviewPresent', false)
              compare(tab.initialUrl, "http://ubuntu.com")
              compare(tab.initialTitle, "Ubuntu")
++            compare(tab.restoreState, "foobar")
              tab.destroy()
+         }

webbrowser-app

Merge lp:~osomon/webbrowser-app/save-restore-state into lp:webbrowser-app

Commit message

Description of the change

Preview Diff

Subscribers