Bug 164173 - Convert more of DOM from ExceptionCode to Exception
Summary: Convert more of DOM from ExceptionCode to Exception
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-28 21:42 PDT by Darin Adler
Modified: 2016-10-29 19:59 PDT (History)
7 users (show)

See Also:


Attachments
Patch (119.05 KB, patch)
2016-10-28 21:59 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (799.37 KB, application/zip)
2016-10-28 23:10 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (1.09 MB, application/zip)
2016-10-28 23:15 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-yosemite (1.62 MB, application/zip)
2016-10-28 23:19 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (deleted)
2016-10-28 23:30 PDT, Build Bot
no flags Details
Patch (119.02 KB, patch)
2016-10-29 08:37 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (120.20 KB, patch)
2016-10-29 09:28 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (120.32 KB, patch)
2016-10-29 09:30 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (1.16 MB, application/zip)
2016-10-29 10:49 PDT, Build Bot
no flags Details
Patch (123.00 KB, patch)
2016-10-29 13:48 PDT, Darin Adler
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2016-10-28 21:42:52 PDT
Convert more of DOM from ExceptionCode to Exception
Comment 1 Darin Adler 2016-10-28 21:59:38 PDT
Created attachment 293274 [details]
Patch
Comment 2 Darin Adler 2016-10-28 22:01:18 PDT
After this patch, the only 8 remaining IDL files using LegacyException are:

ChildNode.idl
Document.idl
Element.idl
EventTarget.idl
NamedNodeMap.idl
Node.idl
ParentNode.idl
Range.idl
Comment 3 Build Bot 2016-10-28 23:10:05 PDT
Comment on attachment 293274 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html
Comment 4 Build Bot 2016-10-28 23:10:08 PDT
Created attachment 293280 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 5 Build Bot 2016-10-28 23:15:41 PDT
Comment on attachment 293274 [details]
Patch

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

New failing tests:
fast/mediastream/RTCRtpSender-replaceTrack.html
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html
fast/mediastream/RTCPeerConnection.html
Comment 6 Build Bot 2016-10-28 23:15:45 PDT
Created attachment 293281 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 7 Build Bot 2016-10-28 23:19:26 PDT
Comment on attachment 293274 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html
Comment 8 Build Bot 2016-10-28 23:19:29 PDT
Created attachment 293282 [details]
Archive of layout-test-results from ews117 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 9 Build Bot 2016-10-28 23:30:37 PDT
Comment on attachment 293274 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html
Comment 10 Build Bot 2016-10-28 23:30:41 PDT
Created attachment 293284 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 11 youenn fablet 2016-10-29 06:21:12 PDT
Comment on attachment 293274 [details]
Patch

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

> Source/WebCore/Modules/mediastream/RTCConfiguration.h:45
>  class RTCConfiguration : public RefCounted<RTCConfiguration> {

A bunch of WebRTC classes have been made ref-counted without actually needed it.
This one should be made a struct, as well as RTCOffer/Answer as you noted.

> Source/WebCore/Modules/mediastream/RTCConfiguration.idl:33
>  ] interface RTCConfiguration {

RTCConfiguration should be a dictionary as per the spec.
Comment 12 Darin Adler 2016-10-29 08:37:25 PDT
Created attachment 293294 [details]
Patch
Comment 13 Darin Adler 2016-10-29 09:28:42 PDT
Created attachment 293295 [details]
Patch
Comment 14 Darin Adler 2016-10-29 09:30:33 PDT
Created attachment 293296 [details]
Patch
Comment 15 Build Bot 2016-10-29 10:49:14 PDT
Comment on attachment 293296 [details]
Patch

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

New failing tests:
fast/mediastream/RTCRtpSender-replaceTrack.html
Comment 16 Build Bot 2016-10-29 10:49:18 PDT
Created attachment 293297 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 17 Darin Adler 2016-10-29 13:48:00 PDT
Created attachment 293310 [details]
Patch
Comment 18 Ryosuke Niwa 2016-10-29 15:14:56 PDT
Comment on attachment 293310 [details]
Patch

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

> Source/WebCore/Modules/mediastream/RTCOfferAnswerOptions.h:53
> +// FIXME: Why is this reference counted?
> +class RTCOfferOptions : public RefCounted<RTCOfferOptions>, public RTCOfferAnswerOptions {

It seems that we can keep RTCOfferAnswerOptions RefCounted and add a FIXME there instead.
I don't think it's really cleaner to use RefCounted<> in two subclasses of RTCOfferAnswerOptions.

> Source/WebCore/editing/InsertIntoTextNodeCommand.cpp:72
> +// FIXME: Why would reapply be iOS-specific?
>  void InsertIntoTextNodeCommand::doReapply()

I guess the only difference would be that we don't momentarilyRevealLastTypedCharacter on iOS upon redo.
Comment 19 Darin Adler 2016-10-29 19:33:48 PDT
Comment on attachment 293310 [details]
Patch

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

>> Source/WebCore/Modules/mediastream/RTCOfferAnswerOptions.h:53
>> +class RTCOfferOptions : public RefCounted<RTCOfferOptions>, public RTCOfferAnswerOptions {
> 
> It seems that we can keep RTCOfferAnswerOptions RefCounted and add a FIXME there instead.
> I don't think it's really cleaner to use RefCounted<> in two subclasses of RTCOfferAnswerOptions.

But it's more efficient. No need for virtual destructor.
Comment 20 WebKit Commit Bot 2016-10-29 19:59:34 PDT
Comment on attachment 293310 [details]
Patch

Clearing flags on attachment: 293310

Committed r208118: <http://trac.webkit.org/changeset/208118>
Comment 21 WebKit Commit Bot 2016-10-29 19:59:40 PDT
All reviewed patches have been landed.  Closing bug.