Closed Bug 598795 Opened 9 years ago Closed 9 years ago

Clean up reconnect code once we have instant access to sessionstore

Categories

(Firefox Graveyard :: Panorama, defect, P4)

defect

Tracking

(blocking2.0 betaN+)

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: iangilman, Assigned: iangilman)

References

Details

Attachments

(1 file, 1 obsolete file)

Once bug 590268 lands, we can get rid of the "delayed reconnect" code in Panorama, always loading our data immediately, as we'll know the data is going to be there right away.

Note that 590268 by itself will make a dramatic improvement in the panorama UX; this bug is more a code cleanup.
Summary: Take advantage of instant access to sessionstore → Clean up reconnect code once we have instant access to sessionstore
Blocks: 594644
Needs test.
Attachment #484478 - Flags: feedback?(seanedunn)
This blocks a blocker (bug 594644), so requesting blocking.
Assignee: nobody → ian
Status: NEW → ASSIGNED
blocking2.0: --- → ?
Depends on: 598600
Blocks: 597043
No longer blocks: 598154
330	        // the code in the progress listener doesn't fire sometimes because
331	        // tab is being restored so need to catch that.
332	        setTimeout(function() {
333	          if (this && this.isShowingCachedData) {
334	            this.hideCachedData();
335	          }
336	        }, 15000);

I know you didn't write this, but is this something that needs to be improved? Should it be a new bug?

Besides that, it looks straightforward.
Attachment #484478 - Flags: feedback?(seanedunn) → feedback+
(In reply to comment #3)
> 330            // the code in the progress listener doesn't fire sometimes
> because
> 331            // tab is being restored so need to catch that.
> 332            setTimeout(function() {
> 333              if (this && this.isShowingCachedData) {
> 334                this.hideCachedData();
> 335              }
> 336            }, 15000);
> 
> I know you didn't write this, but is this something that needs to be improved?
> Should it be a new bug?
> 
> Besides that, it looks straightforward.

I believe they are just typos because the correct references are used in the original code.
(In reply to comment #3)
> I know you didn't write this, but is this something that needs to be improved?
> Should it be a new bug?

You're referring to the 15 second timeout? That's getting removed in bug 597248.

> Besides that, it looks straightforward.

Thanks!
Comment on attachment 484478 [details] [diff] [review]
patch v1 (requires  590268, 598600)

I've come to the conclusion that this patch doesn't need a test, as it's just a refactoring. Ready for review.
Attachment #484478 - Flags: review?(dolske)
Blocks a blocker.
blocking2.0: ? → betaN+
Comment on attachment 484478 [details] [diff] [review]
patch v1 (requires  590268, 598600)

Load balancing review over to dietrich now that it's blocking.
Attachment #484478 - Flags: review?(dolske) → review?(dietrich)
Comment on attachment 484478 [details] [diff] [review]
patch v1 (requires  590268, 598600)

looks good, r=me
Attachment #484478 - Flags: review?(dietrich) → review+
Landed on panorama-central:

http://hg.mozilla.org/users/ian_iangilman.com/panorama-central/rev/c379ef042d95
Whiteboard: [on-panorama-central]
Blocks: 588597
No longer blocks: 588597
Depends on: 611330
Attached patch patch v2Splinter Review
Updated for bitrot and test failures.
Attachment #484478 - Attachment is obsolete: true
Depends on: 615394
No longer depends on: 615394
Blocks: 598154
Beta8 has 1 bug left on it, moving our blockers to b9
No longer blocks: 597043
http://hg.mozilla.org/mozilla-central/rev/2d849e2d302e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [on-panorama-central]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.