Pinned tabs reappear next load after unpinning

[bsclifton]

Test plan

See #8588

Original issue description

• Did you search for similar issues before submitting this one?
  Yes

• Describe the issue you encountered:
  I have 8 pinned tabs. 3 of them are ones which I am not able to get rid of. I can unpin them and for the current session, they disappear, things look good. As soon as I launch Brave again, they show back up again

• Platform (Win7, 8, 10? macOS? Linux distro?):
  macOS

• Brave Version (revision SHA):
  Brave 0.15.0 RC3

• Steps to reproduce:

  1. Pin a bunch of tabs and exit
  2. ??? (maybe try bookmarking them too?)
  3. Relaunch Brave and unpin them
  4. The pinned tab will move over into a tab. Close the tab
  5. Quit Brave and relaunch
  6. Pinned tab shows up?!

• Actual result:
  Pinned tabs are not removable

• Expected result:
  Unpinned tabs stay removed

• Will the steps above reproduce in a fresh profile? If not what other info can be added?

• Is this an issue in the currently released version?
  Unsure- I'd have to check

• Can this issue be consistently reproduced?
  Saying no for now- but I can provide more information

• Extra QA steps:
  1.
  2.
  3.

• Screenshot if needed:

• Any related issues:

[bbondy]

maybe fixed by
0f41941 ?

[bsclifton]

Confirmed it still happens after that. Will continue digging in

[alexwykoff]

I couldn't reproduce on 0.15.0 RC5

[bbondy]

I wonder if you're hitting some state that doesn't save session state in general?

[bbondy]

Need better steps or a session file to reproduce. I'm moving this to 0.15.1 for now.

[bsclifton]

Also getting a report via Twitter that the order of the pinned tabs isn't sticking. After quitting and relaunching, it goes back to the wrong order. Since I have a session where both issues happen, I'll self assign and knock this out 😄

[bsclifton]

I believe I've found the root cause! This was a tricky one

@bbondy if you have a few mins, maybe you can help me think through a fix

I pinned a tab a while back- it's stored in the sites array like so:

"https://mastodon.social/web/getting-started|0|0": {
  "lastAccessedTime": 1493371009561,
  "order": 993,
  "partitionNumber": 0,
  "favicon": "https://mastodon.social/favicon.ico",
  "location": "https://mastodon.social/web/getting-started",
  "tags": [
    "pinned"
  ],
  "themeColor": "#282C37"
},

This is getting loaded when the app starts... pinned tabs are re-created from the sites array. They get created and loaded. In this case, the pinned tab above redirects to https://mastodon.social/about. When you try to unpin, the tab object looks like this:

{
  "audible": false,
  "active": true,
  "guestInstanceId": 8,
  "autoDiscardable": true,
  "partition": "persist:default",
  "windowId": 1,
  "incognito": false,
  "canGoForward": false,
  "url": "https://mastodon.social/about",
  "tabId": 26,
  "index": 7,
  "status": "complete",
  "highlighted": true,
  "partitionNumber": 0,
  "title": "mastodon.social - Mastodon",
  "pinned": false,
  "id": 26,
  "selected": true,
  "discarded": false,
  "canGoBack": false
}

The URL here is never going to match the pinned tab URL. In order to work properly, pinning / unpinning would need to use something other than URL (like a guid or object id). So while unpinning makes it disappear temporarily, it'll pop back up next time the code fires:

1. When you create a new window
2. When you start the browser up again

One quick way to identify potential problems is looking through closed frames. Because I was trying to debug, I was closing the same tab OVER and OVER again. I noticed provisionalLocation and location were always different

"provisionalLocation": "https://mastodon.social/web/getting-started",
"location": "https://mastodon.social/about",

[bsclifton]

I'm still having some other issues (which we can solve after this):

• order of pinned tabs seems to be lost
• one of my pinned tabs shows up twice?! exact same URL

[bsclifton]

Got this issue mostly wrapped up (including tests). I didn't repro anything with order being lost, but I did find another situation where pinning / unpinning can break. I'll try to capture specific steps