1154554 - Upload tabs record

Status: RESOLVED FIXED

(Firefox for iOS :: Sync, defect)

Description

[Richard Newman [:rnewman]]

Fleshing out Bug 1141847. As part of tab sync we wish to upload our own tabs.

Comment 1

[Wesley Johnston (:wesj)]

FYI: TabManager takes a storage object to store these in a db (which I assume is needed here), but it isn't actually assigned right now. I've been meaning to fix that :)

Comment 2

[Emily Toop (:fluffyemily)]

Attachment: WIP Feedback Request

This all _seems_ to be working - I get a success response from the PUT, but I cannot then see the tabs I should have uploaded in about:sync-tabs when I sync my desktop.

Comment 3

[Emily Toop (:fluffyemily)]

Comment on attachment 8636034 [details]
WIP Feedback Request

https://github.com/mozilla/firefox-ios/pull/773

Comment 4

[Emily Toop (:fluffyemily)]

Attachment: Pull request

This one definitely works.

Comment 5

[Richard Newman [:rnewman]]

Comment on attachment 8636565 [details] [review]
Pull request

Close!

Comment 6

[Emily Toop (:fluffyemily)]

Attachment: Pull request

Comment 7

[Richard Newman [:rnewman]]

Comment on attachment 8637210 [details] [review]
Pull request

Comments on the PR. Nice work!

Comment 8

[Richard Newman [:rnewman]]

This produces dodgy output:

    {
      "title" : "reddit: the front page of the internet",
      "icon" : null,
      "urlHistory" : [
        "http:\/\/www.reddit.com\/",
        "http:\/\/localhost:56412\/about\/home\/#panel=2",
        "http:\/\/www.reddit.com\/"
      ],
      "lastUsed" : "1437702183165"
    },

Comment 9

[Richard Newman [:rnewman]]

And it's in the DB:

["http:\/\/www.reddit.com\/","http:\/\/localhost:56412\/about\/home\/#panel=2","http:\/\/www.reddit.com\/"]

Comment 10

[Richard Newman [:rnewman]]

Here's the error:

                history: [displayURL] + browser.historyList,

    var historyList: [NSURL] {
        func listToUrl(item: WKBackForwardListItem) -> NSURL { return item.URL }
        var tabs = self.backList?.map(listToUrl) ?? [NSURL]()
        tabs.append(self.url!)
        return tabs
    }

so browser.historyList *already* includes every URL, including the current one. We then tag the URL on the front. That's wrong.

Comment 11

[Richard Newman [:rnewman]]

This breaks the remote tabs panel.


2015-07-23 19:54:51.021 [Error] [SQLiteRemoteClientsAndTabs.swift:218] getClientsAndTabs(): Couldn't cast tab Optional(<RemoteTab clientGUID: nil, URL: https://support.mozilla.org/en-US/questions, title: Support Forum | Mozilla Support, lastUsed: 1437706465816>) to RemoteTab.


        var acc = [String: [RemoteTab]]()
        for tab in tabCursor {
            if let tab = tab, guid = tab.clientGUID {                   <<<<
                if acc[guid] == nil {
                    acc[guid] = [tab]
                } else {
                    acc[guid]!.append(tab)
                }
            } else {
                log.error("Couldn't cast tab \(tab) to RemoteTab.")
            }
        }


Of course, it has no GUID, 'cos it's local. We don't filter them out when querying.

Comment 12

[Richard Newman [:rnewman]]

And another issue: on first sync we'll wipe the tabs DB as we apply incoming records. That'll throw away whatever we're about to upload. We should check for `clientGUID IS NULL` when wiping here.


    public func wipeTabs() -> Deferred<Result<()>> {
        return self.doWipe { (conn, inout err: NSError?) -> () in
            self.tabs.delete(conn, item: nil, err: &err)
        }
    }

…


            // If this is a fresh start, do a wipe.
            if self.lastFetched == 0 {
                log.info("Last fetch was 0. Wiping tabs.")
                return localTabs.wipeTabs()
                    >>== afterWipe
            }

            return afterWipe()

Comment 13

[Richard Newman [:rnewman]]

Attachment: Pull request with fixes.