Bug 134340 – curl: Improve errors by including the domain

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[cand]

The domain field in ResourceError was empty before this. Fill it properly.

[cand]

Created attachment 233890 [details]
Improve curl resourceerrors

[Build Bot]

Comment on attachment 233890 [details]
Improve curl resourceerrors

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

New failing tests:
media/W3C/video/src/src_reflects_attribute_not_source_elements.html

[Build Bot]

Created attachment 233894 [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

[Peter Gal]

Comment on attachment 233890 [details]
Improve curl resourceerrors

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

This patch doesn't have a changelog. Also please mark patches with r? to be reviewed.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:703
> +            URL tmpurl(URL(), url);

The tmpurl name is not really descriptive, maybe currentUrl?

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:928
> +        URL tmpurl(URL(), handle->m_url);

ditto.

[Basuke Suzuki]

Created attachment 302606 [details]
Patch

Added error domain as "CurlErrorDomain" for ResourceErrors created while curl handling. With the conversation with Alex on the #webkit, it should be the string indicating the source of the error, not the domain name of the connection. Url information is passed explicitly in anyway.

[Build Bot]

Comment on attachment 302606 [details]
Patch

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

New failing tests:
media/modern-media-controls/volume-down-support/volume-down-support.html

[Build Bot]

Created attachment 302614 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Alex Christensen]

Comment on attachment 302606 [details]
Patch

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

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:723
> +                ResourceError resourceError(AtomicString(errorDomainCurl), msg->data.result, url, String(curl_easy_strerror(msg->data.result)));

ASCIILiteral

> Source/WebCore/platform/network/curl/ResourceHandleManager.h:105
> +extern const char* const errorDomainCurl;

This shouldn't be necessary.

[Basuke Suzuki]

Created attachment 302678 [details]
Patch

Fixed based on Alex review.

[WebKit Commit Bot]

Attachment 302678 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Basuke Suzuki]

Created attachment 302691 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 302691 [details]
Patch

Clearing flags on attachment: 302691

Committed r212981: <http://trac.webkit.org/changeset/212981>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.