1276170 - Need a way to stop creating thumbnails for each site while running on our automation tests

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: Testing, defect, P5)

Description

[Hiroyuki Ikezoe (:hiro)]

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?

Comment 1

[:Margaret Leibovic]

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.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

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.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

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.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: MozReview Request: Bug 1276170 - Add a pref to disable thumbnail creation. r?Margaret

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 5

[:Margaret Leibovic]

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).

Comment 6

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

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.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

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.

Comment 9

[Firefox Bug Husbandry Bot]

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Bug 1532199 will supersede this.