1343174 - selectOrAddTab/getTabWithURL doesn't correctly handle zombie tabs

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Jan Henning [:JanH]]

We just query browser.currentURI, which unless I'm mistaken will be "about:blank" when dealing with zombie tabs.

We need to fall back to the session store data in that case to handle this correctly, although depending on how it pans out, bug 1343090 might also present an alternative approach.

Comment 1

[Jan Henning [:JanH]]

Attachment: Bug 1343174 - Part 1 - When looking for tabs with the same URL, fall back to the session store data for zombie tabs.

Delay-loaded tabs all have browsers pointing to about:blank, so there's not much sense in querying browser.currentURI for them. Instead we read their session data to determine the correct URL, which allows selectOrAddTab to successfully detect duplicate tabs even when they are zombified.

Review commit: https://reviewboard.mozilla.org/r/116066/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116066/

Comment 2

[Jan Henning [:JanH]]

Attachment: Bug 1343174 - Part 2 - Rewrite test_selectoraddtab to track tabs instead of browsers.

It's easy to get a tab's browser, but going the other way round requires always calling getTabForBrowser(), which then additionally has to search each open tab in turn to find the tab with the matching browser.
Also, the browser won't survive tab zombification, so for Part 3 we'll have to start keeping a reference to the tab object anyway.

Review commit: https://reviewboard.mozilla.org/r/116068/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116068/

Comment 3

[Jan Henning [:JanH]]

Attachment: Bug 1343174 - Part 3 - Test that selectOrAddTab() finds zombified tabs as well.

Review commit: https://reviewboard.mozilla.org/r/116070/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116070/

Comment 4

[Andrzej Hunt :ahunt]

Comment on attachment 8842141 [details]
Bug 1343174 - Part 1 - When looking for tabs with the same URL, fall back to the session store data for zombie tabs.

https://reviewboard.mozilla.org/r/116066/#review119336

Comment 5

[Andrzej Hunt :ahunt]

Comment on attachment 8842142 [details]
Bug 1343174 - Part 2 - Rewrite test_selectoraddtab to track tabs instead of browsers.

https://reviewboard.mozilla.org/r/116068/#review119340

Comment 6

[Andrzej Hunt :ahunt]

Comment on attachment 8842143 [details]
Bug 1343174 - Part 3 - Test that selectOrAddTab() finds zombified tabs as well.

https://reviewboard.mozilla.org/r/116070/#review119342

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adc6bebaff1d
Part 1 - When looking for tabs with the same URL, fall back to the session store data for zombie tabs. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/8c914488df8c
Part 2 - Rewrite test_selectoraddtab to track tabs instead of browsers. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/6dee9c5fcc9f
Part 3 - Test that selectOrAddTab() finds zombified tabs as well. r=ahunt

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/adc6bebaff1d
https://hg.mozilla.org/mozilla-central/rev/8c914488df8c
https://hg.mozilla.org/mozilla-central/rev/6dee9c5fcc9f

Comment 9

[Julien Cristau [:jcristau]]

I'm assuming this doesn't need to go in 54, feel free to reset the flag and request uplift if I'm mistaken.