[Merge] lp:~osomon/webbrowser-app/remove-formFactor into lp:webbrowser-app

[Alexandre Abreu]

See replies

Diff comments:

> 
> === modified file 'src/app/webbrowser/Browser.qml'
> --- src/app/webbrowser/Browser.qml	2016-01-28 22:12:27 +0000
> +++ src/app/webbrowser/Browser.qml	2016-02-10 14:13:13 +0000
> @@ -460,8 +462,9 @@
>          webview: browser.currentWebview
>          forceHide: browser.fullscreen
>          forceShow: recentView.visible
> -        defaultMode: (formFactor == "desktop") ? Oxide.LocationBarController.ModeShown
> -                                               : Oxide.LocationBarController.ModeAuto
> +        defaultMode: (internal.hasMouse && !internal.hasTouchScreen)

Yes I agree that it is not a blocker at all but I am still unsure about the predicate there ... it seems awkward even more with how HW is now evolving with a bit of a mix of everything,

> +                         ? Oxide.LocationBarController.ModeShown
> +                         : Oxide.LocationBarController.ModeAuto
>      }
>  
>      Chrome {
> @@ -1432,6 +1434,10 @@
>          }
>  
>          readonly property bool hasMouse: (miceModel.count + touchPadModel.count) > 0
> +        readonly property bool hasTouchScreen: touchScreenModel.count > 0
> +
> +        readonly property real freeMemRatio: (MemInfo.total > 0) ? (MemInfo.free / MemInfo.total) : 1.0
> +        readonly property bool lowOnMemory: freeMemRatio < 0.2

Would you mind making the value a bit more obvious for future potential updates then? like make it a readonly var (or even potentially better something that we can "play" on easily) ... not a blocker though,

>  
>          function getOpenPages() {
>              var urls = []
> @@ -1817,15 +1824,41 @@
>      }
>  
>      Connections {
> -        // On mobile, ensure that at most n webviews are instantiated at all
> -        // times, to reduce memory consumption (see http://pad.lv/1376418).
> -        // Note: this works only in narrow mode, where the list of tabs is a
> -        // stack. Switching from wide mode to narrow mode will result in
> -        // undefined behaviour (tabs previously loaded won’t be unloaded).
> -        target: ((formFactor == "mobile") && !browser.wide) ? tabsModel : null
> -        onCurrentTabChanged: {
> -            if (tabsModel.count > browser.maxLiveWebviews) {
> -                tabsModel.get(browser.maxLiveWebviews).unload()
> +        target: internal
> +        onFreeMemRatioChanged: {
> +            if (internal.lowOnMemory) {
> +                // Unload an inactive tab to (hopefully) free up some memory
> +                function getCandidate(model) {
> +                    // Naive implementation that only takes into account the
> +                    // last time a tab was current. In the future we might
> +                    // want to take into account other parameters such as
> +                    // whether the tab is currently playing audio/video.
> +                    var candidate = null
> +                    for (var i = 0; i < model.count; ++i) {
> +                        var tab = model.get(i)
> +                        if (tab.current || !tab.webview) {

you might want to check a tab close event plays well with a tab having been "unloaded" before ...

> +                            continue
> +                        }
> +                        if (!candidate || (candidate.lastCurrent > tab.lastCurrent)) {
> +                            candidate = tab
> +                        }
> +                    }
> +                    return candidate
> +                }
> +                var candidate = getCandidate(publicTabsModel)
> +                if (candidate) {
> +                    console.warn("Unloading background tab (%1) to free up some memory".arg(candidate.url))
> +                    candidate.unload()
> +                    return
> +                } else if (browser.incognito) {
> +                    candidate = getCandidate(privateTabsModelLoader.item)
> +                    if (candidate) {
> +                        console.warn("Unloading a background incognito tab to free up some memory")
> +                        candidate.unload()
> +                        return
> +                    }
> +                }
> +                console.warn("System low on memory, but unable to pick a tab to unload")
>              }
>          }
>      }


-- 
https://code.launchpad.net/~osomon/webbrowser-app/remove-formFactor/+merge/285539
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~osomon/webbrowser-app/remove-formFactor into lp:webbrowser-app.