Closed Bug 1276170 Opened 4 years ago Closed 9 months ago
Need a way to stop creating thumbnails for each site while running on our automation tests
58 bytes, text/x-review-board-request
Some of our animation's tests check restyling count or actual styles caused by restyling. While running those tests, creating a thumbnail flushes style and paint, it will result intermittent failures. I guess bug 1245738 is the one of those intermittent failures. And now I am convinced that bug 1275718 is also caused by creating thumbnails. To avoid those kind of failures we'd like to stop creating thumbnails while running on tests. I think adding a preference to control thumbnail creation is sufficient for us. Margaret, what do you think of?
Adding a pref to control thumbnailing sounds fine to me. However, can we just set it for specific test suites? In general it's best to be testing the application as close to what ships as possible, so I feel like we should leave thumbnailing enabled for at least some integration tests.
Thank you, Margaret. (In reply to :Margaret Leibovic from comment #1) > Adding a pref to control thumbnailing sounds fine to me. However, can we > just set it for specific test suites? Sure. The pref should be enabled in those specific tests which check styles or paints.
I tried initially to add the pref observer in Tab.java but it failed. https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a2959fc6c7d8464605d27c336c51560c28f592b https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a2959fc6c7d&selectedJob=21696465 I guess it's because the pref was set *after* a thumbnail job was post. Clearing the job when setting the pref might solve this failure but I've decided to add the observer to ThumbnailHelper.java.
Thumbnail creation flushes styles or paints. In some layout tests especially for animations, we check restyle count or computed styles. If the thumbnail creation happens in those tests, it results unexpected failures. To prevent such failures we need a pref to disable thumbnail creation for those kind of tests. Review commit: https://reviewboard.mozilla.org/r/56746/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56746/
Comment on attachment 8758488 [details] MozReview Request: Bug 1276170 - Add a pref to disable thumbnail creation. r?Margaret I think kats might be a better reviewer for this (or at least a good additional set of eyes).
I ran into a similar problem in bug 1264592. In that case the "element was painted" flag was getting set because of Fennec thumbnailing, and we decided to only set the flag if the IsPaintingToWindow() condition returned true. (In reply to Hiroyuki Ikezoe (:hiro) from comment #0) > Some of our animation's tests check restyling count or actual styles caused > by restyling. I'm not sure what APIs those tests are using to check restyling count, but can a similar approach be applied to that? I don't have a strong opinion either way; CC'ing Markus as well since he suggested using the IsPaintingToWindow() check in bug 1264592 comment 10.
See Also: → 1264592
Comment on attachment 8758488 [details] MozReview Request: Bug 1276170 - Add a pref to disable thumbnail creation. r?Margaret Thank you, kats for suggesting. I did not know that IsPaintingToWindow() and NS_FRAME_PAINTED_THEBES and nsIDOMWindowUtils.checkAndClearPaintedState! I will figure out it and whether it can be used for animation tests.
In case of bug 1275718, the real problem was not "paint" itself, the problem is busyness of the main-thread caused by creating thumbnail process. I'd take an workaround for the busyness in bug 1275718. In case of bug 1245738, the real problem is not yet unknown, but it has not been for a while. So It might be unrelated to this bug. I'd like to leave this bug open for someone who will be faced with similar problems, but please feel free to close this if you are uncomfortable.
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195 Needinfo :susheel if you think this bug should be re-triaged.
Priority: P3 → P5
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.