Bug 148128

Summary: Decode data URLs in web process
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, andersca, ap, barraclough, beidson, buildbot, cdumez, commit-queue, darin, dbates, japhet, kling, rniwa, sabouhallawa
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=148533
https://bugs.webkit.org/show_bug.cgi?id=175568
Bug Depends on: 150900    
Bug Blocks:    
Attachments:
Description Flags
patch
buildbot: commit-queue-
Archive of layout-test-results from ews103 for mac-mavericks
none
Archive of layout-test-results from ews105 for mac-mavericks-wk2
none
patch
buildbot: commit-queue-
Archive of layout-test-results from ews100 for mac-mavericks
none
Archive of layout-test-results from ews101 for mac-mavericks
none
Archive of layout-test-results from ews103 for mac-mavericks
none
Archive of layout-test-results from ews102 for mac-mavericks
none
patch
none
patch
darin: review+
patch
buildbot: commit-queue-
Archive of layout-test-results from ews103 for mac-mavericks
none
Archive of layout-test-results from ews104 for mac-mavericks-wk2
none
patch none

Description Antti Koivisto 2015-08-18 08:33:25 PDT
Currently we send data URLs to networking layer for decoding. This involves long roundtrip through various processes and API layers.
Comment 1 Brady Eidson 2015-08-18 09:17:43 PDT
rdar://problem/12858179
Comment 2 Antti Koivisto 2015-08-18 11:29:23 PDT
Created attachment 259282 [details]
patch
Comment 3 WebKit Commit Bot 2015-08-18 11:31:02 PDT
Attachment 259282 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/DataURLDecoder.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/network/DataURLDecoder.cpp:131:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/network/DataURLDecoder.h:46:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 3 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Chris Dumez 2015-08-18 11:54:59 PDT
Comment on attachment 259282 [details]
patch

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

> Source/WebCore/platform/network/DataURLDecoder.cpp:61
> +    ASSERT(urlString.startsWith(dataString));

Shouldn't this be case insensitive?

> Source/WebCore/platform/network/DataURLDecoder.cpp:63
> +    size_t mediaTypeEnd = urlString.find(base64String);

ditto
Comment 5 Build Bot 2015-08-18 12:04:39 PDT
Comment on attachment 259282 [details]
patch

Attachment 259282 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/72522

New failing tests:
contentfiltering/block-after-response-then-deny-unblock.html
svg/text/text-default-font-size.html
contentfiltering/block-after-response.html
contentfiltering/block-after-response-then-allow-unblock.html
editing/selection/find-yensign-and-backslash.html
imported/mozilla/svg/filters/feSpecularLighting-1.svg
Comment 6 Build Bot 2015-08-18 12:04:45 PDT
Created attachment 259286 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 7 Build Bot 2015-08-18 12:05:14 PDT
Comment on attachment 259282 [details]
patch

Attachment 259282 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/72521

New failing tests:
editing/selection/find-yensign-and-backslash.html
imported/mozilla/svg/filters/feSpecularLighting-1.svg
svg/text/text-default-font-size.html
Comment 8 Build Bot 2015-08-18 12:05:25 PDT
Created attachment 259287 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 9 Said Abou-Hallawa 2015-08-18 12:24:03 PDT
Comment on attachment 259282 [details]
patch

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

> Source/WebCore/loader/ResourceLoader.cpp:228
> +        return;

Will that make loading data uri resources synchronous? If yes then I think this is going to be problematic. Please see comments in https://bugs.webkit.org/show_bug.cgi?id=99677. Also if you stick with this approach, you may want to roll out http://trac.webkit.org/changeset/179626 since it will not be needed anymore.
Comment 10 Chris Dumez 2015-08-18 12:26:27 PDT
Comment on attachment 259282 [details]
patch

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

>> Source/WebCore/loader/ResourceLoader.cpp:228
>> +        return;
> 
> Will that make loading data uri resources synchronous? If yes then I think this is going to be problematic. Please see comments in https://bugs.webkit.org/show_bug.cgi?id=99677. Also if you stick with this approach, you may want to roll out http://trac.webkit.org/changeset/179626 since it will not be needed anymore.

DataURLDecoder::decode() does the decoding in another thread so it will no be synchronous.
Comment 11 Said Abou-Hallawa 2015-08-18 12:39:17 PDT
Comment on attachment 259282 [details]
patch

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

>>> Source/WebCore/loader/ResourceLoader.cpp:228
>>> +        return;
>> 
>> Will that make loading data uri resources synchronous? If yes then I think this is going to be problematic. Please see comments in https://bugs.webkit.org/show_bug.cgi?id=99677. Also if you stick with this approach, you may want to roll out http://trac.webkit.org/changeset/179626 since it will not be needed anymore.
> 
> DataURLDecoder::decode() does the decoding in another thread so it will no be synchronous.

So who is firing the onload event for data uri resources in this approach?
Comment 12 Andy Estes 2015-08-18 19:39:18 PDT
Comment on attachment 259282 [details]
patch

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

> Source/WebCore/platform/network/DataURLDecoder.cpp:153
> +        RunLoop::main().dispatch([decodeTaskPtr, success] {

Does this work correctly when the WebThread is in use? As far as I can tell, RunLoop::initializeMainRunLoop() is never called in that case.
Comment 13 Antti Koivisto 2015-08-19 03:37:43 PDT
Created attachment 259369 [details]
patch
Comment 14 WebKit Commit Bot 2015-08-19 03:39:00 PDT
Attachment 259369 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/DataURLDecoder.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/network/DataURLDecoder.cpp:129:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/network/DataURLDecoder.h:46:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 3 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Antti Koivisto 2015-08-19 04:50:24 PDT
> Does this work correctly when the WebThread is in use? As far as I can tell,
> RunLoop::initializeMainRunLoop() is never called in that case.

No it won't. Maybe we should initialize RunLoop::main() to be the web thread runloop on iOS WK1? Or maybe add a WebCore level global function somewhere for getting the web RunLoop/WorkQueue?
Comment 16 Antti Koivisto 2015-08-19 05:12:43 PDT
> So who is firing the onload event for data uri resources in this approach?

They are fired by whoever fires them currently.
Comment 17 Darin Adler 2015-08-19 09:52:32 PDT
Comment on attachment 259369 [details]
patch

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

> Source/WebCore/platform/network/DataURLDecoder.h:31
> +#include <wtf/text/WTFString.h>

Could use <wtf/Forward.h> instead.

> Source/WebCore/platform/network/DataURLDecoder.h:46
> +void decode(const URL&, std::function<void (const Result*)>);

Why a pointer rather than a reference?

> Source/WebCore/platform/text/DecodeEscapeSequences.h:118
> -        return (encoding.isValid() ? encoding : UTF8Encoding()).decode(buffer.data(), p - buffer.data());
> +        if (encoding.isValid())
> +            return encoding.decode(buffer.data(), p - buffer.data());
> +        return String(buffer.data(), p - buffer.data());

What makes this behavior change OK? Maybe I am wrong, but I don’t think it’s OK! I have no objections to making a fast path which treats sequences as Latin-1 since that’s what we store in 8-bit WTF::String, but that’s not the same thing as treating sequences as UTF-8. The old contract was that we would treat the sequences as UTF-8 when no encoding was passed in.
Comment 18 Antti Koivisto 2015-08-19 10:48:31 PDT
> What makes this behavior change OK? Maybe I am wrong, but I don’t think it’s
> OK! I have no objections to making a fast path which treats sequences as
> Latin-1 since that’s what we store in 8-bit WTF::String, but that’s not the
> same thing as treating sequences as UTF-8. The old contract was that we
> would treat the sequences as UTF-8 when no encoding was passed in.

I haven't fully confirmed but the idea is that it is not a behavior change because no one else is calling this with invalid encoder.
Comment 19 Antti Koivisto 2015-08-19 10:51:26 PDT
> Why a pointer rather than a reference?

Null here indicates decode failure. Could use some other mechanism for that of course.
Comment 20 Antti Koivisto 2015-08-19 10:58:35 PDT
> I haven't fully confirmed but the idea is that it is not a behavior change
> because no one else is calling this with invalid encoder.

We need to do something along these lines because data urls expect bytes to get through unmodified even when not using base64. This kind of stuff is supposed to work:

data:image/png,%89PNG%0D%0A%1A%0A%00%00%00%0D...
Comment 21 Darin Adler 2015-08-19 11:14:21 PDT
(In reply to comment #20)
> We need to do something along these lines because data urls expect bytes to
> get through unmodified even when not using base64. This kind of stuff is
> supposed to work:
> 
> data:image/png,%89PNG%0D%0A%1A%0A%00%00%00%0D...

Yes, there’s no question that for data URLs we want to decode into bytes, not characters. I think it's a bit peculiar that we decode into a String rather than a SharedBuffer or something like that.

My point was about the other callers of this API, not a doubt that this was correct for data URL handling.
Comment 22 Build Bot 2015-08-19 11:43:46 PDT
Comment on attachment 259369 [details]
patch

Attachment 259369 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/75783

New failing tests:
contentfiltering/block-after-response-then-deny-unblock.html
contentfiltering/block-after-response.html
contentfiltering/block-after-response-then-allow-unblock.html
Comment 23 Build Bot 2015-08-19 11:43:52 PDT
Created attachment 259384 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 24 Antti Koivisto 2015-08-19 11:51:59 PDT
> Yes, there’s no question that for data URLs we want to decode into bytes,
> not characters. I think it's a bit peculiar that we decode into a String
> rather than a SharedBuffer or something like that.

Perhaps I should add a separate decodeDataURLEscapeSequences() that produces bytes directly. It will be very similar to the existing DecodeEscapeSequences stuff but maybe there is a way to minimize duplication.
Comment 25 Build Bot 2015-08-19 12:05:41 PDT
Comment on attachment 259369 [details]
patch

Attachment 259369 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/75713

New failing tests:
contentfiltering/block-after-response-then-deny-unblock.html
contentfiltering/block-after-response.html
contentfiltering/block-after-response-then-allow-unblock.html
Comment 26 Build Bot 2015-08-19 12:05:48 PDT
Created attachment 259389 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 27 Build Bot 2015-08-19 13:43:31 PDT
Comment on attachment 259369 [details]
patch

Attachment 259369 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/76039

New failing tests:
contentfiltering/block-after-response-then-deny-unblock.html
contentfiltering/block-after-response.html
contentfiltering/block-after-response-then-allow-unblock.html
Comment 28 Build Bot 2015-08-19 13:43:36 PDT
Created attachment 259399 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 29 Build Bot 2015-08-19 15:26:23 PDT
Comment on attachment 259369 [details]
patch

Attachment 259369 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/76277

New failing tests:
contentfiltering/block-after-response-then-deny-unblock.html
contentfiltering/block-after-response.html
contentfiltering/block-after-response-then-allow-unblock.html
Comment 30 Build Bot 2015-08-19 15:26:30 PDT
Created attachment 259413 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 31 Antti Koivisto 2015-08-21 08:10:19 PDT
Created attachment 259613 [details]
patch
Comment 32 WebKit Commit Bot 2015-08-21 08:12:14 PDT
Attachment 259613 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/DataURLDecoder.h:47:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 33 Antti Koivisto 2015-08-21 09:09:15 PDT
Created attachment 259618 [details]
patch
Comment 34 WebKit Commit Bot 2015-08-21 09:11:03 PDT
Attachment 259618 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/DataURLDecoder.cpp:84:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WebCore/platform/network/DataURLDecoder.h:47:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 35 Darin Adler 2015-08-21 09:42:27 PDT
Comment on attachment 259618 [details]
patch

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

Looks good to me.

> Source/WebCore/loader/ResourceLoader.cpp:210
> +    if (url.protocolIsData()) {

I think the code inside this if statement is long enough that it could be clearer if factored into a separate private named member function, perhaps named loadDataURL or startLoadingDataURL or something.

> Source/WebCore/loader/ResourceLoader.cpp:222
> +            ResourceResponse dataResponse { url, result.mimeType, dataSize, result.charset };

If we had the right kind of constructor for ResourceResponse we could use WTF::move on all the arguments here to avoid a little bit of reference count churn.

> Source/WebCore/platform/network/DataURLDecoder.cpp:53
> +    DecodeTask(const String& urlString, StringView encodedData, bool isBase64, DecodeCompletionHandler completionHandler)
> +        : urlString(urlString)
> +        , encodedData(encodedData)
> +        , isBase64(isBase64)
> +        , completionHandler(completionHandler)
> +    { }

I don’t think we need this constructor. We should just be able to write:

    std::make_unique<DecodeTask>({ ... });

below without using a constructor.

> Source/WebCore/platform/network/DataURLDecoder.cpp:68
> +    ASSERT(urlString.startsWith(dataString));

This check needs to ignore ASCII case.

> Source/WebCore/platform/network/DataURLDecoder.cpp:72
> +    if (headerEnd == notFound)
> +        return nullptr;

Do we have a test case that covers this?

> Source/WebCore/platform/network/DataURLDecoder.cpp:87
> +        mimeType = "text/plain";

I think ASCIILiteral("text/plain") will be slightly more efficient.

> Source/WebCore/platform/network/DataURLDecoder.cpp:89
> +            charset = "US-ASCII";

I think ASCIILiteral("US-ASCII") will be slightly more efficient.

> Source/WebCore/platform/network/DataURLDecoder.cpp:92
> +    decodeTask->result.mimeType = mimeType;

Please consider using WTF::move here to get rid of a tiny bit of reference count churn.

> Source/WebCore/platform/network/DataURLDecoder.cpp:93
> +    decodeTask->result.charset = charset;

Please consider using WTF::move here to get rid of a tiny bit of reference count churn.

> Source/WebCore/platform/network/DataURLDecoder.cpp:104
> +    if (!base64URLDecode(task.encodedData.toStringWithoutCopying(), buffer)) {
> +        // Didn't work, try unescaping and decoding as base64.
> +        auto unescapedString = decodeURLEscapeSequences(task.encodedData.toStringWithoutCopying());

This toStringWithoutCopying thing seems a bit ugly, and could be avoided if the functions took StringView instead of const String& arguments.

> Source/WebCore/platform/network/DataURLDecoder.cpp:116
> +    auto buffer = decodeURLEscapeSequencesAsData(task.encodedData.toStringWithoutCopying(), encoding);

Same thought about toStringWithoutCopying.

> Source/WebCore/platform/network/DataURLDecoder.cpp:117
> +    task.result.data =  SharedBuffer::create(buffer.data(), buffer.size());

Why not use SharedBuffer::adoptVector here?

Stray space here, after the "=".

> Source/WebCore/platform/network/DataURLDecoder.cpp:147
> +            decodeTask->completionHandler(decodeTask->result);

Please consider using WTF::move here to get rid of a tiny bit of reference count churn.

> Source/WebCore/platform/network/DataURLDecoder.h:49
> +void decode(const URL&, DecodeCompletionHandler);

Don’t functions sometimes have state and are thus expensive to copy? If so, it might be nicer to take ownership of the completion handler rather than passing it by value.

> Source/WebCore/platform/text/DecodeEscapeSequences.h:158
> +inline Vector<char> decodeURLEscapeSequencesAsData(const String& string, const TextEncoding& encoding)

I think it would be better to have this take a StringView instead of a String. Should be a simple matter of changing the URLEscapeSequence functions to work on StringView.

> Source/WebCore/platform/text/DecodeEscapeSequences.h:170
> +            encodedRunPosition = length;

I’m not sure this line of code is needed. We intentionally made encodedRunPosition a very large value so it doesn’t need to be explicitly checked for. Using StringView::substring with it should work because it clamps its arguments down to the length of the string. It’s even possible, depending on how URLEscapeSequence::findEndOfRun is written, that we could remove the if statement entirely.

Maybe you don’t want to write code that relies on that. Maybe some day we’ll change the return values to be Optional<size_t> instead to make things like that more clear.

> Source/WebCore/platform/text/DecodeEscapeSequences.h:181
> +        auto stringFragment = StringView(string).substring(decodedPosition, encodedRunPosition - decodedPosition);
> +        auto encodedStringFragment = encoding.encode(StringView(string).substring(decodedPosition, encodedRunPosition - decodedPosition), URLEncodedEntitiesForUnencodables);

Looks like when refactoring this you forgot to use the stringFragment local. I suggest either getting rid of it or using it.

> Source/WebCore/platform/text/DecodeEscapeSequences.h:184
> +        if (encodedRunPosition == length)

If you make the change I suggested above this would be encodedRunPosition == notFound.

> Source/WebCore/platform/text/DecodeEscapeSequences.h:185
> +            return result;

Should this be doing some kind of “shrink to fit” thing?

> Source/WebCore/platform/text/DecodeEscapeSequences.h:195
> +    ASSERT_NOT_REACHED();
> +    return { };

I’m surprised this is needed. Don’t our compilers recognize the infinite loop?

> Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp:155
> +        resourceLoader->start();
>          m_webResourceLoaders.set(identifier, WebResourceLoader::create(resourceLoader));

I know it’s only one or two lines of code, but I think a helper function might be nice for the m_webResourceLoaders thing that’s now repeated four times.
Comment 36 Antti Koivisto 2015-08-21 10:33:36 PDT
> I don’t think we need this constructor. We should just be able to write:
> 
>     std::make_unique<DecodeTask>({ ... });

That wouldn't compile. More awkward version does:
    
    std::make_unique<DecodeTask>(DecodeTask { ... });


> > Source/WebCore/platform/network/DataURLDecoder.cpp:68
> > +    ASSERT(urlString.startsWith(dataString));
> 
> This check needs to ignore ASCII case.

I believe protocol part of URL is always converted to lowercase

> > Source/WebCore/platform/network/DataURLDecoder.cpp:72
> > +    if (headerEnd == notFound)
> > +        return nullptr;
> 
> Do we have a test case that covers this?

Probably not. We have some explicit data url test coverage and a ton of incidental coverage. It would be good to add a bunch of more systematic cases covering all the edges.

> > Source/WebCore/platform/network/DataURLDecoder.h:49
> > +void decode(const URL&, DecodeCompletionHandler);
> 
> Don’t functions sometimes have state and are thus expensive to copy? If so,
> it might be nicer to take ownership of the completion handler rather than
> passing it by value.

std::function has efficient move constructor. I think we just need to use WTF::move in a few places? (instead of say DecodeCompletionHandler&&)

> > Source/WebCore/platform/text/DecodeEscapeSequences.h:170
> > +            encodedRunPosition = length;
> 
> I’m not sure this line of code is needed. We intentionally made
> encodedRunPosition a very large value so it doesn’t need to be explicitly
> checked for. Using StringView::substring with it should work because it
> clamps its arguments down to the length of the string. It’s even possible,
> depending on how URLEscapeSequence::findEndOfRun is written, that we could
> remove the if statement entirely.

Ok. I didn't know that notFound value was intentionally chosen for this purpose.
Comment 37 Antti Koivisto 2015-08-21 14:30:46 PDT
Created attachment 259659 [details]
patch
Comment 38 WebKit Commit Bot 2015-08-21 14:32:05 PDT
Attachment 259659 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/DataURLDecoder.h:47:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 39 Build Bot 2015-08-21 14:50:10 PDT
Comment on attachment 259659 [details]
patch

Attachment 259659 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/87137

New failing tests:
editing/execCommand/apply-style-command-crash.html
fast/hidpi/image-srcset-data-srcset-invalid-inputs.html
editing/style/apply-style-crash.html
fast/css/url-with-multi-byte-unicode-escape.html
Comment 40 Build Bot 2015-08-21 14:50:14 PDT
Created attachment 259663 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 41 Build Bot 2015-08-21 15:11:41 PDT
Comment on attachment 259659 [details]
patch

Attachment 259659 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/87192

New failing tests:
editing/execCommand/apply-style-command-crash.html
fast/hidpi/image-srcset-data-srcset-invalid-inputs.html
editing/style/apply-style-crash.html
fast/css/url-with-multi-byte-unicode-escape.html
Comment 42 Build Bot 2015-08-21 15:11:47 PDT
Created attachment 259668 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 43 Antti Koivisto 2015-08-22 01:37:29 PDT
Created attachment 259715 [details]
patch
Comment 44 WebKit Commit Bot 2015-08-22 01:38:31 PDT
Attachment 259715 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/DataURLDecoder.h:47:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 45 WebKit Commit Bot 2015-08-22 02:57:35 PDT
Comment on attachment 259715 [details]
patch

Clearing flags on attachment: 259715

Committed r188820: <http://trac.webkit.org/changeset/188820>
Comment 46 WebKit Commit Bot 2015-08-22 02:57:43 PDT
All reviewed patches have been landed.  Closing bug.
Comment 47 Alexey Proskuryakov 2015-08-27 11:18:17 PDT
This made at least one test flaky, please see bug 148533.