Last Comment Bug 650573 - Panorama hangs when restoring a session
: Panorama hangs when restoring a session
Status: RESOLVED FIXED
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: -- normal
: Firefox 6
Assigned To: Tim Taubert [:ttaubert]
:
Mentors:
Depends on:
Blocks: 595601 631952 636279 645461 650280 651643 653099
  Show dependency treegraph
 
Reported: 2011-04-16 19:06 PDT by Tim Taubert [:ttaubert]
Modified: 2016-04-12 14:00 PDT (History)
3 users (show)
mounir: in‑testsuite+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch v1 (7.47 KB, patch)
2011-04-17 05:57 PDT, Tim Taubert [:ttaubert]
raymond: feedback+
Details | Diff | Splinter Review
patch v2 (8.49 KB, patch)
2011-04-17 20:14 PDT, Tim Taubert [:ttaubert]
no flags Details | Diff | Splinter Review
patch v3 (8.50 KB, patch)
2011-04-18 09:40 PDT, Tim Taubert [:ttaubert]
sdwilsh: review+
Details | Diff | Splinter Review
patch for checkin (6.83 KB, patch)
2011-05-19 14:50 PDT, Tim Taubert [:ttaubert]
no flags Details | Diff | Splinter Review

Description Tim Taubert [:ttaubert] 2011-04-16 19:06:01 PDT
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] 2011-04-17 05:57:56 PDT
Created attachment 526580 [details] [diff] [review]
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=tim.taubert@gmx.de&rev=97fb08e090ab
Comment 2 Raymond Lee [:raymondlee] 2011-04-17 18:37:23 PDT
Comment on attachment 526580 [details] [diff] [review]
patch v1

Looks good
Comment 3 Tim Taubert [:ttaubert] 2011-04-17 20:14:51 PDT
Created attachment 526651 [details] [diff] [review]
patch v2
Comment 5 Tim Taubert [:ttaubert] 2011-04-18 09:40:59 PDT
Created attachment 526749 [details] [diff] [review]
patch v3
Comment 7 Ian Gilman [:iangilman] 2011-04-22 09:43:49 PDT
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 2011-05-19 13:59:29 PDT
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] 2011-05-19 14:02:16 PDT
(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] 2011-05-19 14:50:16 PDT
Created attachment 533802 [details] [diff] [review]
patch for checkin
Comment 11 Mounir Lamouri (:mounir) 2011-05-20 07:00:15 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/24438e77a538

Note You need to log in before you can comment on or make changes to this bug.