Merge lp:~abreu-alexandre/webbrowser-app/better-control-webengine-lib-loaded into lp:webbrowser-app

[Alexandre Abreu]

Make sure that we dont load unecessary libs (oxide or qtwebkit) correspnding to the webengine that we dont use.

Make sure that we dont load unecessary libs (oxide or qtwebkit) correspnding to the webengine that we dont use.

[Alberto Mardegan (mardy)]

I would rather call setSource() on Component.onCompleted, in order to set all the properties at construction time.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:490
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/739/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4769
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4153/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/241
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/241
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/241/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/241
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4119
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4898
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4898/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4351
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4351/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6386/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5925

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

review: Needs Fixing (continuous-integration)

[Alexandre Abreu (abreu-alexandre)]

> I would rather call setSource() on Component.onCompleted, in order to set all
> the properties at construction time.

100% agree, but it probably wont be able to make it for 14.04 then, since for a consulted reason
(see below) it requires a fix in unity-webapps-qml.

Reason:

in src/app/webcontainer/WebappContainerWebview.qm, the currentWebView property is bounded
to webappContainerWebViewLoader.item (which makes sense). This property is then bounded to
& used by the UnityWebapps qml element to inject the proper API & search for the proper
webapp locally installed (if any) w/ the proper name to know its homepage, etc.

The thing is that the Webapps qml element does not atm property listen to its bounded
webview property changes. So it considers that at Component.onCompleted time it should be
done.

Delaying a bit the creation of the webview backend will hand off the value null (since
it has not been created at qml property eval time) to the Webapps element and prevent
the webapp from doing anything (even navigating).

[Olivier Tilloy (osomon)]

> Delaying a bit the creation of the webview backend will hand off the value
> null (since
> it has not been created at qml property eval time) to the Webapps element and
> prevent
> the webapp from doing anything (even navigating).

How about, in src/app/webcontainer/WebApp.qml, lazy loading the UnityWebApps.UnityWebApps component when webview.currentWebview becomes not null? Then you could do what Alberto suggests.

Other than that the change looks sound, although I haven’t functionally tested it.

One minor suggestion: in the onLoaded slot, you don’t need to prefix 'item' with 'webappContainerWebViewLoader.', that would make the code easier to read.

[Alexandre Abreu (abreu-alexandre)]

> > Delaying a bit the creation of the webview backend will hand off the value
> > null (since
> > it has not been created at qml property eval time) to the Webapps element
> and
> > prevent
> > the webapp from doing anything (even navigating).
>
> How about, in src/app/webcontainer/WebApp.qml, lazy loading the
> UnityWebApps.UnityWebApps component when webview.currentWebview becomes not
> null? Then you could do what Alberto suggests.

totally, I can go into all sorts of ways to make it work, but
I am a bit reluctant to go there & make it more convoluted, instead of fixing it the right way ..

[Olivier Tilloy (osomon)]

> totally, I can go into all sorts of ways to make it work, but
> I am a bit reluctant to go there & make it more convoluted, instead of fixing
> it the right way ..

Well, it’s not really about whether it’s convoluted or not. Alberto’s suggestion has the advantage of having the properties being set at construction time, which definitely avoids potential issues. Additionally, lazy loading UnityWebApps would mean a (potentially) reduced startup time (I haven’t measured anything though).

[Oliver Grawert (ogra)]

i just tested this together with the other merge i mention on bug 1303676 and it seems to at least help with this release blocking bug, it would be nice to get this landed in one way or the other so we can have a releasable image.

=== modified file 'src/app/webcontainer/WebappContainerWebview.qml'
--- src/app/webcontainer/WebappContainerWebview.qml	2014-04-09 13:34:46 +0000
+++ src/app/webcontainer/WebappContainerWebview.qml	2014-04-10 18:47:57 +0000
@@ -37,30 +37,13 @@
     Loader {
         id: webappContainerWebViewLoader
         anchors.fill: parent
-        sourceComponent: withOxide ? webappContainerWebViewOxide : webappContainerWebViewWebkit
-    }
-
-    Component {
-        id: webappContainerWebViewWebkit
-
-        WebViewImplWebkit {
-            toolbar: containerWebview.toolbar
-            url: containerWebview.url
-            webappName: containerWebview.webappName
-            webappUrlPatterns: containerWebview.webappUrlPatterns
-            developerExtrasEnabled: containerWebview.developerExtrasEnabled
-        }
-    }
-
-    Component {
-        id: webappContainerWebViewOxide
-
-        WebViewImplOxide {
-            toolbar: containerWebview.toolbar
-            url: containerWebview.url
-            webappName: containerWebview.webappName
-            webappUrlPatterns: containerWebview.webappUrlPatterns
-            developerExtrasEnabled: containerWebview.developerExtrasEnabled
+        source: withOxide ? Qt.resolvedUrl("WebViewImplOxide.qml") : Qt.resolvedUrl("WebViewImplWebkit.qml")
+        onLoaded: {
+            webappContainerWebViewLoader.item.toolbar = containerWebview.toolbar
+            webappContainerWebViewLoader.item.url = containerWebview.url
+            webappContainerWebViewLoader.item.webappName = containerWebview.webappName
+            webappContainerWebViewLoader.item.webappUrlPatterns = containerWebview.webappUrlPatterns
+            webappContainerWebViewLoader.item.developerExtrasEnabled = containerWebview.developerExtrasEnabled
         }
     }
 }