Bug 173367 - WKErrorGetErrorCode should not return the API::Error enum values directly
Summary: WKErrorGetErrorCode should not return the API::Error enum values directly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-14 08:41 PDT by Carlos Garcia Campos
Modified: 2017-06-14 22:39 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.74 KB, patch)
2017-06-14 08:43 PDT, Carlos Garcia Campos
achristensen: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (17.25 MB, application/zip)
2017-06-14 09:46 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (1.23 MB, application/zip)
2017-06-14 09:54 PDT, Build Bot
no flags Details
Patch for landing (2.71 KB, patch)
2017-06-14 09:56 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2017-06-14 08:41:17 PDT
Even if the values are the same, we should use a switch to ensure we return the C API values and tot all API::Error enum values are exposed in the C API
Comment 1 Carlos Garcia Campos 2017-06-14 08:43:31 PDT
Created attachment 312891 [details]
Patch
Comment 2 Alex Christensen 2017-06-14 08:47:19 PDT
Comment on attachment 312891 [details]
Patch

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

> Source/WebKit2/Shared/API/c/WKErrorRef.cpp:79
> +    default:
> +        break;

Do we need this default?  If we are switching on enum values, a default would prevent us from finding the bug of missing one of the enum values.  Let's remove the default if we can.
Comment 3 Carlos Garcia Campos 2017-06-14 08:52:20 PDT
(In reply to Alex Christensen from comment #2)
> Comment on attachment 312891 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=312891&action=review
> 
> > Source/WebKit2/Shared/API/c/WKErrorRef.cpp:79
> > +    default:
> > +        break;
> 
> Do we need this default?  If we are switching on enum values, a default
> would prevent us from finding the bug of missing one of the enum values. 
> Let's remove the default if we can.

Yes, because not all API::Error values are exposed in the C API.
Comment 4 Build Bot 2017-06-14 09:46:31 PDT
Comment on attachment 312891 [details]
Patch

Attachment 312891 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3929918

New failing tests:
http/tests/misc/window-dot-stop.html
security/block-test.html
http/tests/misc/will-send-request-returns-null-on-redirect.html
http/tests/security/XFrameOptions/x-frame-options-parent-same-origin-deny.html
http/tests/cache/cancel-multiple-post-xhrs.html
security/block-test-no-port.html
http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-deny.html
http/tests/xmlhttprequest/abort-should-cancel-load.html
http/tests/security/XFrameOptions/x-frame-options-multiple-headers-conflict.html
http/tests/security/XFrameOptions/x-frame-options-deny.html
Comment 5 Build Bot 2017-06-14 09:46:33 PDT
Created attachment 312895 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 6 Carlos Garcia Campos 2017-06-14 09:53:09 PDT
It seems we should just return the integer value in case of an error without a name in the C API.

--- /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphonesimulator/layout-test-results/http/tests/cache/cancel-multiple-post-xhrs-expected.txt
+++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release-iphonesimulator/layout-test-results/http/tests/cache/cancel-multiple-post-xhrs-actual.txt
@@ -1,7 +1,7 @@
 http://127.0.0.1:8000/cache/resources/empty.txt - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/cache/resources/empty.txt, main document URL http://127.0.0.1:8000/cache/cancel-multiple-post-xhrs.html, http method POST> redirectResponse (null)
 http://127.0.0.1:8000/cache/resources/empty.txt - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/cache/resources/empty.txt, main document URL http://127.0.0.1:8000/cache/cancel-multiple-post-xhrs.html, http method POST> redirectResponse (null)
 http://127.0.0.1:8000/cache/resources/empty.txt - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/cache/resources/empty.txt, main document URL http://127.0.0.1:8000/cache/cancel-multiple-post-xhrs.html, http method POST> redirectResponse (null)
-http://127.0.0.1:8000/cache/resources/empty.txt - didFailLoadingWithError: <NSError domain NSURLErrorDomain, code -999, failing URL "http://127.0.0.1:8000/cache/resources/empty.txt">
-http://127.0.0.1:8000/cache/resources/empty.txt - didFailLoadingWithError: <NSError domain NSURLErrorDomain, code -999, failing URL "http://127.0.0.1:8000/cache/resources/empty.txt">
-http://127.0.0.1:8000/cache/resources/empty.txt - didFailLoadingWithError: <NSError domain NSURLErrorDomain, code -999, failing URL "http://127.0.0.1:8000/cache/resources/empty.txt">
+http://127.0.0.1:8000/cache/resources/empty.txt - didFailLoadingWithError: <NSError domain NSURLErrorDomain, code 300, failing URL "http://127.0.0.1:8000/cache/resources/empty.txt">
+http://127.0.0.1:8000/cache/resources/empty.txt - didFailLoadingWithError: <NSError domain NSURLErrorDomain, code 300, failing URL "http://127.0.0.1:8000/cache/resources/empty.txt">
+http://127.0.0.1:8000/cache/resources/empty.txt - didFailLoadingWithError: <NSError domain NSURLErrorDomain, code 300, failing URL "http://127.0.0.1:8000/cache/resources/empty.txt">
Comment 7 Build Bot 2017-06-14 09:54:02 PDT
Comment on attachment 312891 [details]
Patch

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

New failing tests:
http/tests/misc/window-dot-stop.html
security/block-test.html
http/tests/misc/will-send-request-returns-null-on-redirect.html
http/tests/security/XFrameOptions/x-frame-options-parent-same-origin-deny.html
http/tests/cache/cancel-multiple-post-xhrs.html
security/block-test-no-port.html
http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-deny.html
http/tests/xmlhttprequest/abort-should-cancel-load.html
http/tests/security/XFrameOptions/x-frame-options-multiple-headers-conflict.html
http/tests/security/XFrameOptions/x-frame-options-deny.html
Comment 8 Build Bot 2017-06-14 09:54:03 PDT
Created attachment 312898 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 9 Carlos Garcia Campos 2017-06-14 09:56:53 PDT
Created attachment 312900 [details]
Patch for landing
Comment 10 Carlos Garcia Campos 2017-06-14 22:39:00 PDT
Committed r218315: <http://trac.webkit.org/changeset/218315>