Bug 162382 – [Fetch API] Request constructor should provide exception messages

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[youenn fablet]

Request constructor should provide exception messages

[youenn fablet]

Created attachment 289550 [details]
Patch

[Build Bot]

Comment on attachment 289550 [details]
Patch

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

New failing tests:
fetch/fetch-url-serialization.html

[Build Bot]

Created attachment 289553 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 289550 [details]
Patch

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

New failing tests:
fetch/fetch-url-serialization.html

[Build Bot]

Created attachment 289554 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

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

[youenn fablet]

Created attachment 289559 [details]
Rebasing failing test

[Darin Adler]

Comment on attachment 289559 [details]
Rebasing failing test

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

Looks OK. After my next patch, you could have used ExceptionOr<void> instead of Optional<Exception> and we should decide which style we prefer.

> Source/WebCore/dom/Exception.h:38
> +    explicit Exception(ExceptionCode, const String&);

We should consider taking a String&& instead to avoid reference count churn, since we almost always want to store a message in here.

> Source/WebCore/dom/Exception.h:41
> +    const String& message() const { return m_message; }

We should consider adding a releaseMessage function here so we can avoid reference count churn when turning this into a JavaScript exception.

> Source/WebCore/dom/ExceptionOr.h:41
> +    const String& exceptionMessage() const;

We should consider adding a releaseMessage function here so we can avoid reference count churn when turning this into a JavaScript exception.

[youenn fablet]

Thanks for the review.

> Looks OK. After my next patch, you could have used ExceptionOr<void> instead
> of Optional<Exception> and we should decide which style we prefer.

I don't have a strong preference.
Returning Nullopt seems a tad more explicit than returning { }.

> > Source/WebCore/dom/Exception.h:38
> > +    explicit Exception(ExceptionCode, const String&);
> 
> We should consider taking a String&& instead to avoid reference count churn,
> since we almost always want to store a message in here.

Right.
Filed bug 163165 for that purpose.

[youenn fablet]

Created attachment 291013 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 291013 [details]
Patch for landing

Clearing flags on attachment: 291013

Committed r206954: <http://trac.webkit.org/changeset/206954>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.