650573 - Panorama hangs when restoring a session

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Session restore re-uses (overwrites) opened tabs when restoring a session. Panorama closes all groups not present in the to-be-restored session. If those include re-used tabs, these tabs get closed and SR / Panorama wait until these (now closed) tabs get restored.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

Changes:

* bug624727.js: disabling private browsing if it wasn't enabled caused this test to fail
* bug629195.js: the test must call setWindowState() with aOverwrite = true to meet the test's expectations

Passed try:

http://tbpl.mozilla.org/?tree=MozillaTry&pusher=[e-mail]&rev=97fb08e090ab

Comment 2

[Raymond Lee [:raymondlee]]

Comment on attachment 526580 [details] [diff] [review]
patch v1

Looks good

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v2

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 526651 [details] [diff] [review]
patch v2

Passed try:

http://tbpl.mozilla.org/?tree=MozillaTry&pusher=[e-mail]&rev=44ef323d64e9

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v3

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 526749 [details] [diff] [review]
patch v3

Passed try:

http://tbpl.mozilla.org/?tree=MozillaTry&pusher=[e-mail]&rev=c3b827e1e649

Comment 7

[Ian Gilman [:iangilman]]

Comment on attachment 526749 [details] [diff] [review]
patch v3

Paul, can you review this? I'm phasing out my review duties, and would like to spread the Panorama code knowledge around.

Comment 8

[Shawn Wilsher :sdwilsh]

Comment on attachment 526749 [details] [diff] [review]
patch v3

Review of attachment 526749 [details] [diff] [review]:
-----------------------------------------------------------------

r=sdwilsh

::: browser/base/content/test/tabview/browser_tabview_bug624727.js
@@ +63,5 @@
>      let prefix = 'enter';
>      ok(!pb.privateBrowsingEnabled, prefix + ': private browsing is disabled');
> +    registerCleanupFunction(function () {
> +      if (pb.privateBrowsingEnabled)
> +        pb.privateBrowsingEnabled = false

Why do you need the condition here?

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

(In reply to comment #8)
> ::: browser/base/content/test/tabview/browser_tabview_bug624727.js
> @@ +63,5 @@
> >      let prefix = 'enter';
> >      ok(!pb.privateBrowsingEnabled, prefix + ': private browsing is disabled');
> > +    registerCleanupFunction(function () {
> > +      if (pb.privateBrowsingEnabled)
> > +        pb.privateBrowsingEnabled = false
> 
> Why do you need the condition here?

That's just because of bug 651643. Seems we'll not get it before this one because I still experience some strange try failures there.

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch for checkin

Comment 11

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/24438e77a538