Bug 139313

Summary: REGRESSION (173394): Support for webcam is broken
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, buildbot, commit-queue, ddkilzer, rniwa
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=13759
Bug Depends on: 139337    
Bug Blocks:    
Attachments:
Description Flags
patch
ap: review+
updated patch
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
none
some changes to try to make the test work in the bots
buildbot: commit-queue-
Archive of layout-test-results from ews100 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
none
another attempt with the test
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
none
another
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion
none
updated test none

Description Antti Koivisto 2014-12-05 13:56:28 PST
multipart/x-mixed-replace for images is broken due to buffering in network process
Comment 1 Antti Koivisto 2014-12-05 14:12:44 PST
Created attachment 242662 [details]
patch
Comment 2 Antti Koivisto 2014-12-05 14:13:24 PST
rdar://problem/18988179
Comment 3 Alexey Proskuryakov 2014-12-05 14:23:28 PST
Comment on attachment 242662 [details]
patch

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

> Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:200
> +    // For multipart/x-mixed-replace didReceiveResponseAsync gets called multiple times and buffering would require special handling.
> +    if (response.isMultipart() && !isSynchronous())
> +        m_bufferedData = nullptr;

I'm not sure if I understand what happens (or what should happen) for sync loads.

Anyway, this would probably read a little better if the new code was inside an else clause below - there is no need to check isSynchronous twice.

> LayoutTests/ChangeLog:13
> +            The last image is repeated due to a WebCore side bug that causes last part to not render.

This may be appropriate to have in an HTML comment inside the test, as that's where one would be looking when they notice this strangeness.

> LayoutTests/http/tests/multipart/multipart-image.html:10
> +    if (window.testRunner)
> +        setTimeout("testRunner.notifyDone()", 1000);

Can we watch image size changes to finish faster and more reliably? One second is a long time to waste, but may be not enough to avoid random failures.
Comment 4 Antti Koivisto 2014-12-05 15:20:09 PST
(In reply to comment #3)
> I'm not sure if I understand what happens (or what should happen) for sync
> loads.

I think it will just smash all data together and deliver it if the stream ever ends. Don't know what should happen.

Implemented the other changes.
Comment 5 Antti Koivisto 2014-12-05 15:22:08 PST
Created attachment 242668 [details]
updated patch
Comment 6 Alexey Proskuryakov 2014-12-05 15:59:36 PST
Comment on attachment 242668 [details]
updated patch

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

> LayoutTests/http/tests/multipart/multipart-image.html:24
> +<!-- The last image is repeated due to a WebCore side bug that causes last part to not render. -->

https://bugs.webkit.org/show_bug.cgi?id=36536 ?
Comment 7 Build Bot 2014-12-05 16:34:09 PST
Comment on attachment 242668 [details]
updated patch

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

New failing tests:
http/tests/multipart/multipart-image.html
Comment 8 Build Bot 2014-12-05 16:34:12 PST
Created attachment 242678 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 9 Build Bot 2014-12-05 16:35:34 PST
Comment on attachment 242668 [details]
updated patch

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

New failing tests:
http/tests/multipart/multipart-image.html
Comment 10 Build Bot 2014-12-05 16:35:37 PST
Created attachment 242679 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 11 Antti Koivisto 2014-12-06 04:05:19 PST
Created attachment 242709 [details]
some changes to try to make the test work in the bots
Comment 12 Build Bot 2014-12-06 04:50:45 PST
Comment on attachment 242709 [details]
some changes to try to make the test work in the bots

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

New failing tests:
http/tests/multipart/multipart-image.html
Comment 13 Build Bot 2014-12-06 04:50:47 PST
Created attachment 242711 [details]
Archive of layout-test-results from ews100 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 14 Build Bot 2014-12-06 05:19:51 PST
Comment on attachment 242709 [details]
some changes to try to make the test work in the bots

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

New failing tests:
http/tests/multipart/multipart-image.html
Comment 15 Build Bot 2014-12-06 05:19:54 PST
Created attachment 242712 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 16 Antti Koivisto 2014-12-08 00:17:18 PST
*** Bug 139378 has been marked as a duplicate of this bug. ***
Comment 17 Antti Koivisto 2014-12-08 00:40:33 PST
Created attachment 242788 [details]
another attempt with the test
Comment 18 Build Bot 2014-12-08 01:51:24 PST
Comment on attachment 242788 [details]
another attempt with the test

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

New failing tests:
http/tests/multipart/multipart-image.html
Comment 19 Build Bot 2014-12-08 01:51:27 PST
Created attachment 242790 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 20 Antti Koivisto 2014-12-08 01:59:53 PST
Created attachment 242791 [details]
another
Comment 21 Build Bot 2014-12-08 03:11:27 PST
Comment on attachment 242791 [details]
another

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

New failing tests:
http/tests/multipart/multipart-image.html
Comment 22 Build Bot 2014-12-08 03:11:31 PST
Created attachment 242795 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 23 Build Bot 2014-12-08 03:38:21 PST
Comment on attachment 242791 [details]
another

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

New failing tests:
http/tests/multipart/multipart-image.html
Comment 24 Build Bot 2014-12-08 03:38:23 PST
Created attachment 242799 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 25 Antti Koivisto 2014-12-08 05:01:54 PST
https://trac.webkit.org/r176947

Landed without the test since I can't figure out how to make it work in bots (any bots, including those not affected by the patch). It is as if the php script that generated the multipart replies doesn't run correctly.
Comment 26 Alexey Proskuryakov 2014-12-08 11:32:52 PST
Re-opening for a test. This is actually a pre-existent problem in multipart.php, which produces invalid multipart responses (which newer CFNetwork now supports).
Comment 27 Alexey Proskuryakov 2014-12-08 11:33:35 PST
Created attachment 242833 [details]
updated test

Let's see what EWS thinks.
Comment 28 WebKit Commit Bot 2014-12-08 11:36:43 PST
Attachment 242833 [details] did not pass style-queue:


ERROR: LayoutTests/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 29 Alexey Proskuryakov 2014-12-08 16:16:21 PST
Committed the test in <http://trac.webkit.org/r176990>.
Comment 30 Antti Koivisto 2014-12-09 00:18:15 PST
Thank you Alexey.