1215659 - newtab tile's page thumbnail is not scaled correctly (HiDPI problem?)

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: newtab-screenshot.png

I am using Nightly 44 with e10s on a Retina MacBook Pro. I have seen similar content scaling bugs when dragging e10s browser windows between my MacBook Pro's builtin Retina display and an external non-HiDPI display, e.g. bug 1125325 and bug 978913.

See the attached screenshot.

Comment 1

[Jim Mathies [:jimm]]

Might be a regression from bug 1197361.

Comment 2

[Mason Chang [Inactive] [:mchang]]

Yup, probably a regression from 1197361.

Comment 3

[Mason Chang [Inactive] [:mchang]]

Attachment: Use destination canvas as thumbnail size

There was a problem, where sometimes we would be provided with a destination canvas already, but we'd go down a path to try to take the snapshot at a size smaller than the destination canvas. In those cases, just try to snapshot at the destination canvas size.

Also, sometimes, we'd get a black canvas because nsIProgressWatcher would actually give us an error, but we'd continue to snapshot the page. The web content wasn't actually ready, so we'd snapshot nothing.

Comment 4

[Drew Willcoxon :adw]

Comment on attachment 8677208 [details] [diff] [review]
Use destination canvas as thumbnail size

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

Thanks for fixing that second problem too, that's great.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f6b31718d94d

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f6b31718d94d

Comment 7

[Mason Chang [Inactive] [:mchang]]

Comment on attachment 8677208 [details] [diff] [review]
Use destination canvas as thumbnail size

Approval Request Comment
[Feature/regressing bug #]: Regression from bug 1197361.
[User impact if declined]: Can get black screenshots shown during newtab images.
[Describe test coverage new/current, TreeHerder]: Mochitests in tree herder, manual tests.
[Risks and why]: Low, this change makes sure we correctly size the thumbnails.
[String/UUID change made/needed]: None

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/f6b31718d94d

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8677208 [details] [diff] [review]
Use destination canvas as thumbnail size

Given that this was manually tested, has automated mochitest coverage and has been in Nightly for a few days, let's uplift to Aurora44.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7b8547621a2e

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/7b8547621a2e