RESOLVED FIXED 172310
[WebIDL] Remove the need for the generator to know about native type mapping
https://bugs.webkit.org/show_bug.cgi?id=172310
Summary [WebIDL] Remove the need for the generator to know about native type mapping
Sam Weinig
Reported 2017-05-18 14:12:14 PDT
[WebIDL] Remove the need for the generator to know about native type mapping
Attachments
Patch (82.19 KB, patch)
2017-05-18 16:20 PDT, Sam Weinig
darin: review+
Sam Weinig
Comment 1 2017-05-18 16:20:08 PDT
Alex Christensen
Comment 2 2017-05-18 16:57:03 PDT
Comment on attachment 310572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=310572&action=review > Source/WebCore/Modules/geolocation/GeoNotifier.cpp:124 > RefPtr<PositionError> error = PositionError::create(PositionError::TIMEOUT, ASCIILiteral("Timeout expired")); Auto. Does this make a ref? > Source/WebCore/bindings/js/JSNodeFilterCustom.cpp:41 > +unsigned short JSNodeFilter::acceptNode(Node& node) Why is unsigned short preferable?
Sam Weinig
Comment 3 2017-05-18 17:16:10 PDT
(In reply to Alex Christensen from comment #2) > Comment on attachment 310572 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=310572&action=review > > > Source/WebCore/Modules/geolocation/GeoNotifier.cpp:124 > > RefPtr<PositionError> error = PositionError::create(PositionError::TIMEOUT, ASCIILiteral("Timeout expired")); > > Auto. Does this make a ref? Yes. Will update. > > > Source/WebCore/bindings/js/JSNodeFilterCustom.cpp:41 > > +unsigned short JSNodeFilter::acceptNode(Node& node) > > Why is unsigned short preferable? It's just more inline with what the IDL says. But it really doesn't matter. I can change it back if you would prefer.
Darin Adler
Comment 4 2017-05-18 19:27:27 PDT
Comment on attachment 310572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=310572&action=review > Source/WebCore/Modules/geolocation/Geolocation.cpp:511 > RefPtr<PositionError> error = PositionError::create(PositionError::PERMISSION_DENIED, ASCIILiteral(permissionDeniedErrorMessage)); auto/Ref > Source/WebCore/Modules/geolocation/Geolocation.cpp:525 > + RefPtr<Geoposition> position = lastPosition(); > + if (position) Would be nice to define inside the if. Does this need to go into a RefPtr rather than a raw pointer? > Source/WebCore/Modules/geolocation/Geolocation.cpp:690 > + RefPtr<Geoposition> position = lastPosition(); > + ASSERT(position); Does this need to go into a RefPtr rather than a raw pointer? > Source/WebCore/Modules/geolocation/Geolocation.cpp:701 > RefPtr<PositionError> positionError = createPositionError(error); auto/Ref > Source/WebCore/Modules/webaudio/AudioBufferCallback.idl:27 > +// DecodeErrorCallback, which takes DOMException. Currently, we pass and AudioBuffer "and" -> "an" > Source/WebKit/mac/DOM/DOM.mm:861 > - return core(self)->acceptNode(core(node)); > + return core(self)->acceptNode(*core(node)); Would be more elegant to throw an exception if someone passes nil rather than crashing.
Sam Weinig
Comment 5 2017-05-19 11:07:52 PDT
Note You need to log in before you can comment on or make changes to this bug.