Bug 185035 - Rename NetworkLoadChecker::returnError() to NetworkLoadChecker::accessControlErrorForValidationHandler()
Summary: Rename NetworkLoadChecker::returnError() to NetworkLoadChecker::accessControl...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-04-26 10:53 PDT by Daniel Bates
Modified: 2018-04-26 11:30 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.32 KB, patch)
2018-04-26 10:56 PDT, Daniel Bates
youennf: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2018-04-26 10:53:18 PDT
The name NetworkLoadChecker::returnError() is disingenuous as this function can either return a ResourceRequest object or an Error object. We should come up with more accurate name for this function.
Comment 1 Daniel Bates 2018-04-26 10:56:42 PDT
Created attachment 338893 [details]
Patch
Comment 2 Daniel Bates 2018-04-26 10:58:22 PDT
Comment on attachment 338893 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:193
>          String message = makeString("Unsafe attempt to load URL ", request.url().stringCenterEllipsizedToLength(), " from origin ", m_origin->toString(), ". Domains, protocols and ports must match.\n");

Will rename message to error before landing.
Comment 3 youenn fablet 2018-04-26 11:02:28 PDT
Comment on attachment 338893 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:147
> +NetworkLoadChecker::RequestOrError NetworkLoadChecker::createRequestOrError(String&& error)

createRequestOrError seems a bit odd.
How about createValidationError?
Comment 4 Daniel Bates 2018-04-26 11:12:50 PDT
(In reply to youenn fablet from comment #3)
> Comment on attachment 338893 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=338893&action=review
> 
> > Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:147
> > +NetworkLoadChecker::RequestOrError NetworkLoadChecker::createRequestOrError(String&& error)
> 
> createRequestOrError seems a bit odd.
> How about createValidationError?

From our IRC conversation, will rename to accessControlErrorForValidationHandler.
Comment 5 Daniel Bates 2018-04-26 11:21:16 PDT
(In reply to Daniel Bates from comment #2)
> Comment on attachment 338893 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=338893&action=review
> 
> > Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:193
> >          String message = makeString("Unsafe attempt to load URL ", request.url().stringCenterEllipsizedToLength(), " from origin ", m_origin->toString(), ". Domains, protocols and ports must match.\n");
> 
> Will rename message to error before landing.

Actually will keep as-is and rename argument to accessControlErrorForValidationHandler() to message to describe that its argument represents the error message for the access control error. Will change other locals named "error" to "message" when passed to accessControlErrorForValidationHandler().
Comment 6 Daniel Bates 2018-04-26 11:29:37 PDT
Committed r231059: <https://trac.webkit.org/changeset/231059>
Comment 7 Radar WebKit Bug Importer 2018-04-26 11:30:33 PDT
<rdar://problem/39764569>