Bug 140199

Summary: Two tests, which include data uri images, need to be changed and rebaselined since the expected results are incorrect
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: ImagesAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, buildbot, commit-queue, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews105 for mac-mountainlion-wk2
none
Archive of layout-test-results from ews103 for mac-mountainlion
none
Patch
none
Patch
none
Patch
none
Patch
none
update the test and results commit-queue: commit-queue-

Description Said Abou-Hallawa 2015-01-07 13:29:05 PST
fast/forms/basic-buttons.html
compositing/tiling/huge-layer-img.html

These tests have data uri images and both assume that the data of the images are loaded in memory immediately after setting the source of the <img> tag.

Both tests need to be changed such that the results can be dumped only after the image onload() event is fired. And of course the expected results need to be changed since they are incorrect.
Comment 1 Said Abou-Hallawa 2015-01-07 13:36:46 PST
Created attachment 244186 [details]
Patch
Comment 2 Said Abou-Hallawa 2015-01-07 13:47:36 PST
Created attachment 244189 [details]
Patch
Comment 3 Build Bot 2015-01-07 14:04:41 PST
Comment on attachment 244189 [details]
Patch

Attachment 244189 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6083102665342976

New failing tests:
fast/forms/basic-buttons.html
Comment 4 Build Bot 2015-01-07 14:04:44 PST
Created attachment 244197 [details]
Archive of layout-test-results from ews105 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 5 Build Bot 2015-01-07 14:10:18 PST
Comment on attachment 244189 [details]
Patch

Attachment 244189 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6701012496678912

New failing tests:
fast/forms/basic-buttons.html
Comment 6 Build Bot 2015-01-07 14:10:21 PST
Created attachment 244199 [details]
Archive of layout-test-results from ews103 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 7 Said Abou-Hallawa 2015-01-07 14:33:39 PST
Created attachment 244204 [details]
Patch
Comment 8 Simon Fraser (smfr) 2015-01-07 14:47:44 PST
Comment on attachment 244204 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244204&action=review

> LayoutTests/ChangeLog:12
> +        set it as the source of an <img> tag. We need to rebase-line the expected results since

We normally write "rebaseline"
Comment 9 Said Abou-Hallawa 2015-01-07 14:54:49 PST
Created attachment 244207 [details]
Patch
Comment 10 WebKit Commit Bot 2015-01-07 14:55:40 PST
Comment on attachment 244207 [details]
Patch

Rejecting attachment 244207 [details] from commit-queue.

said@apple.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 11 Said Abou-Hallawa 2015-01-07 15:31:18 PST
Comment on attachment 244207 [details]
Patch

I changed my email on webkit.org before changing it in contributors.json. I changed my email back on webkit.org and I logged in with it to be able to cq+ this bug.
Comment 12 WebKit Commit Bot 2015-01-07 15:33:32 PST
Comment on attachment 244207 [details]
Patch

Rejecting attachment 244207 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 244207, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/6527590307201024
Comment 13 Said Abou-Hallawa 2015-01-07 15:39:06 PST
Created attachment 244214 [details]
Patch
Comment 14 Simon Fraser (smfr) 2015-01-07 16:01:20 PST
Comment on attachment 244214 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244214&action=review

> LayoutTests/fast/forms/basic-buttons.html:66
> +function onImageLoad()
> +{
> +    if (typeof counter == 'undefined')
> +        counter = 0;
> +    
> +    if (++counter == 7) {
> +        printSize('button');
> +        printSize('input');
> +        if (window.testRunner)
> +            testRunner.notifyDone();
> +    }
> +}

This change is actually misleading. Since the images are created at document load time, the image load events should fire before the document load event.

So it would be better to change the test to just wait for the window load event, rather than counting images.
Comment 15 Said Abou-Hallawa 2015-01-07 16:27:41 PST
Created attachment 244219 [details]
Patch
Comment 16 WebKit Commit Bot 2015-01-07 18:17:53 PST
Comment on attachment 244219 [details]
Patch

Clearing flags on attachment: 244219

Committed r178078: <http://trac.webkit.org/changeset/178078>
Comment 17 WebKit Commit Bot 2015-01-07 18:18:00 PST
All reviewed patches have been landed.  Closing bug.
Comment 18 Alexey Proskuryakov 2015-01-08 00:39:48 PST
Comment on attachment 244219 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244219&action=review

> LayoutTests/ChangeLog:22
> +        * fast/forms/basic-buttons.html:
> +        * platform/mac-mountainlion/fast/forms/basic-buttons-expected.txt:
> +        * platform/mac/fast/forms/basic-buttons-expected.png:
> +        * platform/mac/fast/forms/basic-buttons-expected.txt:

This test also has results in platform/mac-mavericks, so Mavericks bots started to fail after landing.

> LayoutTests/fast/forms/basic-buttons.html:14
> +if (window.testRunner)
> +    testRunner.waitUntilDone();

This isn't needed. Test results are always dumped after the load event fires, so there is no need for waitUntilDone/notifyDone here.
Comment 19 Alexey Proskuryakov 2015-01-08 00:42:37 PST
Re-opening to fix the above. I'm going to tackle this now, as Mavericks bots are red.
Comment 20 Alexey Proskuryakov 2015-01-08 00:43:28 PST
Created attachment 244246 [details]
update the test and results
Comment 21 WebKit Commit Bot 2015-01-08 00:45:19 PST
Comment on attachment 244246 [details]
update the test and results

Rejecting attachment 244246 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 244246, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.appspot.com/results/6531610396590080
Comment 22 Alexey Proskuryakov 2015-01-08 09:11:54 PST
Argh. Landed the follow-up manually in r178117.