[Merge] lp:~ahayzen/webbrowser-app/migrate-to-tabs-component into lp:webbrowser-app/staging
Andrew Hayzen
andrew.hayzen at canonical.com
Wed Feb 15 11:27:42 UTC 2017
Resolved further comments:
1) See documentation: https://doc.qt.io/qt-5/qml-qtqml-component.html#incubateObject-method.
There is no guarantee that the incubator will be ready by the time of the return statement. In fact if it works here it’s by accident. It seems to me the only clean solution here is to use faviconFactory.createObject(…).
One side question: is iconSourceFromModelItem() potentially called more than once for every tab? If so, the instantiated FaviconFetcher should be associated to the tab and re-used for subsequent calls. If we are guaranteed that it won’t be reused, then it should be deleted before the return statement.
So what I've done is add a FaviconFetcher to the BrowserTab, then the iconSourceFromModelItem() can simply call return modelData.tab.favicon.localUrl. Then it seems that QML actually creates a binding, as when the value of localUrl changes, it calls the method a second time.
You can prove this by going to a website that loads the icon slowly and add the following traceline:
console.debug(modelData.url, index, modelData.tab.favicon.localUrl);
I then get this output:
qml: http://www.bbc.co.uk/ 3
...
qml: http://www.bbc.co.uk/ 3 file:///home/andrew/.cache/webbrowser-app/favicons/d2a8da919816b78cc42b7a9f87c1d71b.ico
2) Those two imports don't appear to be needed.
Removed
--
https://code.launchpad.net/~ahayzen/webbrowser-app/migrate-to-tabs-component/+merge/312340
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app/staging.
More information about the Ubuntu-reviews
mailing list