flypig.co.uk

Yesterday we looked at creating a DeclarativeTabFilterModel that allowed the hidden windows to be filtered out. The implementation turned out to be really straightforward, although to be fair that's because Qt is doing the majority of the hard work for us.

Today I'm going to try adding it in to the QML code to see what happens. The interesting file here is TabView.qml. This is the component that displays the tabs to the user. The design of the user interface means it has to handle multiple models: a model to allow switching between standard and private tabs called the modeModel which is defined directly in the QML. A model for handling the standard tab list accessed as the webView.persistentTabModel. The use of "persistent" in the name is a reference to the fact that these tabs will survive the browser being shutdown and restarted. Finally there are the private tabs accessed with the webView.privateTabModel model. These lose the "persistent" moniker because the private tabs are all discarded when the user closes the browser.

We're interested in the last of these two models because we want to wrap them in our filtering proxy. The key bit of code for this is the following (found in the same TabView.qml file:

    TabGridView {
        id: _tabView

        portrait: tabView.portrait
        model: tabItem.privateMode ? webView.privateTabModel
                                   : webView.persistentTabModel
        header: Item {
            width: 1
            height: Theme.paddingLarge
        }

        onHide: tabView.hide()
        onEnterNewTabUrl: tabView.enterNewTabUrl()
        onActivateTab: tabView.activateTab(index)
        onCloseTab: tabView.closeTab(index)
        onCloseAll: tabView.closeAll()
        onCloseAllCanceled: tabView.closeAllCanceled()
        onCloseAllPending: tabView.closeAllPending()
    }

As you can see the tabs are set to use a model that depends on whether private mode is active or not. There's s ternary operator there that chooses between the two models:

        model: tabItem.privateMode ? webView.privateTabModel
                                   : webView.persistentTabModel

This line hooks the correct model up to the model to be used by the TabGridView. The code here looks pretty slick and straightforward, but I recall when it was first implemented it caused a lot of trouble. The difficulty is that when the component switches between persistent and private tabs there's a short period during the animation when both models are used simultaneously. Ensuring that they can both exist without the tabs suddenly switching from one model to the other in an instant needed some work.

It wasn't me that had to implement that, but I probably reviewed the code at some stage.

So now let's add in our filtering proxy model.

    TabGridView {
        id: _tabView

        portrait: tabView.portrait
        model: TabFilterModel {
        	sourceModel: tabItem.privateMode ? webView.privateTabModel
        	                                 : webView.persistentTabModel
        	showHidden: false
        }
        header: Item {
            width: 1
            height: Theme.paddingLarge
        }
[...]

When I test this out on-device it works surprisingly well. The hidden tab is indeed hidden in the tab view. I need to sort the tab numbering out, but that's expected because I've not started using the filter model for the numbering yet. As soon as I switch to using it for the count as well, it should all fall into line.

However, there is one more serious problem. Switching between persistent and private modes causes the browser to hang. My guess is that this is to do with the code used to switch between the two, but I'm not certain yet.

But that's okay, this is what building this sort of stuff is all about: try something out, find the glitches, fix the glitches. It's not quite as clean and logical as we might always like, but this is software engineering, not computer science.

As we can see from the code shown above, the model is being used in a TabGridView component. When the tab switches it comes up with this error:

[W] unknown:65 - file:///usr/share/sailfish-browser/pages/components/
    TabGridView.qml:65:19: Unable to assign [undefined] to int

When we look inside the code for TabGridView we can see that this relates to this line:

    currentIndex: model.activeTabIndex

This is making use of the activeTabIndex property, which is a member of DeclarativeTabModel (and so therefore also a member of the persistent and private models that inherit from it) but not a member of our filter proxy model. So that would explain why it's so unhappy.

I can pass through the property quite easily. It looks like the count property is also used, but not the loaded property, so probably we just need to pass through those two.

Having added the required properties and tested out the code, it seems the problem still persists: switching from persistent to private tabs causes the browser to hang. I tried out a few changes to my code to see if that would help, including setting the filter flag to false, but that still doesn't fix it.

Thinking that my changes might have corrupted the tab database details stored on disk I also tried removing the profile data stored at ~/.local/share/org.sailfishos/browser. This cleared all of the tabs, but to my surprise the browser now hangs when creating the first persistent tab as well. So the issue isn't necessarily to do with switching between persistent and private views; more likely it's to do with creating the very first tab in each case.

After reverting my changes to the filter proxy it's now clear that it's not the proxy that's causing the issue here at all: it is indeed the creation of the very first tab. With the profile restored there are a bunch of tabs pre-existing in the persistent tab list. But in the private tab list there are none. So switching to this empty list is what's causing the crash.

So it looks like I need to double back and fix this. Here's the backtrace I get from the crash:

Thread 1 "sailfish-browse" received signal SIGSEGV, Segmentation fault.
Tab::tabId (this=0x48) at ../storage/tab.cpp:38
38          return m_tabId;
(gdb) bt
#0  Tab::tabId (this=0x48) at ../storage/tab.cpp:38
#1  0x00000055555cb3d8 in DeclarativeWebPage::tabId (this=<optimized out>)
    at ../qtmozembed/declarativewebpage.cpp:127
#2  0x0000005555591fd0 in DeclarativeWebContainer::onNewTabRequested
    (this=0x555569b370, tab=...) at include/c++/8.3.0/bits/atomic_base.h:390
#3  0x0000007fb7ec4204 in QMetaObject::activate(QObject*, int, int, void**) ()
    from /usr/lib64/libQt5Core.so.5
#4  0x00000055555f8bb0 in DeclarativeTabModel::newTabRequested
    (this=this@entry=0x55559c7e30, _t1=...) at moc_declarativetabmodel.cpp:366
#5  0x00000055555c5148 in DeclarativeTabModel::newTab
    (this=this@entry=0x55559c7e30, url=..., parentId=parentId@entry=0, 
    browsingContext=browsingContext@entry=0, hidden=hidden@entry=false)
    at ../history/declarativetabmodel.cpp:233
#6  0x00000055555c5318 in DeclarativeTabModel::newTab
    (this=this@entry=0x55559c7e30, url=...)
    at ../history/declarativetabmodel.cpp:199
#7  0x00000055555f8f3c in DeclarativeTabModel::qt_static_metacall
    (_o=_o@entry=0x55559c7e30, _c=_c@entry=QMetaObject::InvokeMetaMethod,
    _id=_id@entry=18, _a=_a@entry=0x7fffffbdc8)
    at moc_declarativetabmodel.cpp:182
[...]
#18 0x0000005555c61460 in ?? ()
Backtrace stopped: not enough registers or memory available to unwind further
(gdb) frame 1
#1  0x00000055555cb3d8 in DeclarativeWebPage::tabId (this=<optimized out>)
    at ../qtmozembed/declarativewebpage.cpp:127
127         return m_initialTab.tabId();
(gdb) p m_initialTab
value has been optimized out
(gdb) b DeclarativeWebPage::setInitialState
Breakpoint 1 at 0x55555cb3d8: file ../qtmozembed/declarativewebpage.cpp, line 134.
(gdb) r
[...]

Thread 1 "sailfish-browse" received signal SIGSEGV, Segmentation fault.
Tab::tabId (this=0x48) at ../storage/tab.cpp:38
38          return m_tabId;
(gdb) 

Notice that the issue here is that m_initialTab isn't set correctly. In fact, by putting additional breakpoints on the code I'm able to see that it's never actually getting set at all. It seems that the DeclarativeWebPage::setInitialState() method that sets it is never getting called.

This is supposed to be called in the WebPageFactory::createWebPage() method, which a breakpoint confirms is also not being called. From inspection of the code it seems that this is supposed to be being called from the WebPages::page() method. But that's also not being called.

Finally, this page() call is supposed to be made from the DeclarativeWebContainer::activatePage() method. You might recall this is a method I messed around with earlier.

It looks like the problem line might actually be one of the debug lines I added:

    qDebug() << "PRINT: onNewTabRequested post activeTab: "
        << m_webPage->tabId();

This line works fine if m_webPage::m_initialTab has been set correctly, but if it's the very first tab it won't yet have been set at all. When this happens the code tries to dereference an uninitialised pointer and boom. If this really is the problem then that will be nice: understandable and easy to fix. I've rebuilt sailfish-browser without the debug output line to see what happens.

This has changed things a bit: there's no longer a crash when creating the very first page. But switching to private browsing mode is still causing problems, even without the changes I made today to add the filter proxy model. That might suggest the issue has been there for much longer than I thought: I've just not switched to private browsing recently.

Running the app through the debugger it becomes clear that the app really is hanging rather than crashing. Or, even more specifically, the front-end is hanging. The JavaScript interpreter seems to happily continue running in the background. It's not at all clear why this might be happening and since it's already quite late now, finding the answer is going to have to wait until the morning.

If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.