Closed
Bug 1387682
Opened 7 years ago
Closed 7 years ago
Screenshots missing when thumbnailer Promise is garbage collected
Categories
(Firefox :: New Tab Page, defect)
Firefox
New Tab Page
Tracking
()
RESOLVED
FIXED
Firefox 57
| Tracking | Status | |
|---|---|---|
| firefox57 | --- | fixed |
People
(Reporter: Mardak, Assigned: Mardak)
References
Details
Attachments
(1 file)
Looks like bug 1353542 made the promise GC-able see https://bugzilla.mozilla.org/show_bug.cgi?id=1353542#c15 try test enabled without fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3028b854e8d7a616708e768ada0b2f4e96aaf055 try test enabled and with fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a53019ce73ea870adfc0accb81aea6879306aabf
|
Assignee | |
Updated•7 years ago
|
Assignee: nobody → edilee
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 2•7 years ago
|
||
Ah these are probably the leaks florian was trying to avoid: 22:33:05 ERROR - TEST-UNEXPECTED-FAIL | browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js | leaked 12 window(s) until shutdown [url = about:newtab] 22:33:05 ERROR - TEST-UNEXPECTED-FAIL | browser/base/content/test/newtab/browser_newtab_undo.js | leaked 2 window(s) until shutdown [url = about:newtab] These shouldn't be activity stream related but I guess the shutdown cleanup of observers isn't enough.
|
|
Assignee | |
Comment 3•7 years ago
|
||
https://treeherder.mozilla.org/logviewer.html#?job_id=121155958&repo=try&lineNumber=3730
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 7•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8894041 [details] Bug 1387682 - Screenshots missing when thumbnailer Promise is garbage collected. https://reviewboard.mozilla.org/r/165136/#review171218 Looks good to me, just one change to the comment. Thanks! ::: toolkit/components/thumbnails/BackgroundPageThumbs.jsm:93 (Diff revision 4) > * Asynchronously captures a thumbnail of the given URL if one does not > * already exist. Otherwise does nothing. > * > * @param url The URL to capture. > * @param options An optional object that configures the capture. See > * capture() for description. Can you add 'unloadingPromise' to the options param comment in capture() like this comment suggests?
Attachment #8894041 -
Flags: review?(usarracini) → review+
|
|
Assignee | |
Comment 8•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8894041 [details] Bug 1387682 - Screenshots missing when thumbnailer Promise is garbage collected. https://reviewboard.mozilla.org/r/165136/#review171218 > Can you add 'unloadingPromise' to the options param comment in capture() like this comment suggests? Added comment to `captureIfMissing` as that's where it's used.
| Comment hidden (mozreview-request) |
|
||
Comment 10•7 years ago
|
||
Pushed by edilee@gmail.com: https://hg.mozilla.org/integration/autoland/rev/02f357c8e980 Screenshots missing when thumbnailer Promise is garbage collected. r=ursula
|
||
Comment 11•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/02f357c8e980
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
|
||
Comment 12•7 years ago
|
||
Commit pushed to master at https://github.com/mozilla/activity-stream https://github.com/mozilla/activity-stream/commit/e8285110be3850d6628f874616d0e865b4612ed0 chore(tests): Re-enable getScreenshots test with bug 1387682 fixed (#3110)
|
||
Updated•5 years ago
|
Component: Activity Streams: Newtab → New Tab Page
You need to log in
before you can comment on or make changes to this bug.
Description
•