Closed Bug 938189 Opened 11 years ago Closed 11 years ago

Avoid duplicate snapshot taking and compression during a history swipe animation

Categories

(Firefox :: General, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: spohl, Assigned: spohl)

References

Details

Attachments

(2 files, 4 obsolete files)

We mistakenly used to take a snapshot and compress it when we started a swipe animation and again when we were just about to navigate away from the current page. This separates the two steps of snapshotting and compression and only does them once per animation.
Attached patch Patch (obsolete) (deleted) — Splinter Review
Attachment #831585 - Flags: review?(felipc)
Attachment #831585 - Flags: review?(felipc) → review+
Attached patch Telemetry (obsolete) (deleted) — Splinter Review
Attachment #831635 - Flags: review?(felipc)
Attached patch Patch (obsolete) (deleted) — Splinter Review
Updated to apply over new patches in bug 836430. Carrying over r+.
Attachment #831585 - Attachment is obsolete: true
Attachment #831981 - Flags: review+
Attached patch Telemetry (obsolete) (deleted) — Splinter Review
Updated to apply over new patches in bug 836430. Carrying over r?.
Attachment #831635 - Attachment is obsolete: true
Attachment #831635 - Flags: review?(felipc)
Attachment #831982 - Flags: review?
Attachment #831982 - Flags: review?
Attached patch Patch (deleted) — Splinter Review
b-c tests failed in the try run. :mikedeboer was kind enough to point out to me that I forgot about the early return at the beginning of 'HSA__takeSnapshot'. When a tab first opens, the history index is -1 which would lead to an early return and leave gBrowser.selectedBrowser.snapshots uninitialized. However, we would then try to retrieve it in 'HSA__compressSnapshotAtCurrentIndex', which leads to the errors in b-c. This patch here is green on try: https://tbpl.mozilla.org/?tree=Try&rev=a8d1dcbc78f0
Attachment #831981 - Attachment is obsolete: true
Attachment #832359 - Flags: review?(felipc)
Attached patch Telemetry (deleted) — Splinter Review
Updated for new patch. Setting back to r?.
Attachment #831982 - Attachment is obsolete: true
Attachment #832360 - Flags: review?(felipc)
Attachment #832359 - Flags: review?(felipc) → review+
Attachment #832360 - Flags: review?(felipc) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Blocks: 1382560
No longer blocks: 1382560
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: