Make ExceptionCode a proper enumeration instead of a typedef to uint8_t.
Created attachment 316263 [details] Patch
Comment on attachment 316263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316263&action=review > Source/WebCore/Modules/fetch/FetchBody.h:52 > - void formData(FetchBodyOwner&, Ref<DeferredPromise>&& promise) { promise.get().reject(ExceptionCode { 0 }); } > + void formData(FetchBodyOwner&, Ref<DeferredPromise>&& promise) { promise.get().reject(NoException); } How about NOT_SUPPORTED_ERR instead? > Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:3574 > - ExceptionCode ec = 0; > + ExceptionCode ec = NoException; It would be nice to switch these to ExceptionOr<>. > Source/WebKitLegacy/win/DOMCoreClasses.cpp:824 > - WebCore::ExceptionCode ec = 0; > + WebCore::ExceptionCode ec = WebCore::NoException; Is this dead code? > Source/WebKitLegacy/win/WebView.cpp:6045 > - ExceptionCode ec = 0; > + ExceptionCode ec = NoException; Is this dead code?
Created attachment 316292 [details] Patch
Comment on attachment 316292 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316292&action=review > Source/WebCore/dom/Exception.h:29 > +#include "ExceptionCode.h" Because we added this include here we can remove includes of this file from tons of other places, perhaps almost everywhere else it is included. > Source/WebCore/dom/ExceptionCode.h:25 > +enum ExceptionCode { For future clean-up here: I don’t think any of the values in this enumeration need explicitly set values any more--no reason they need to match the legacy codes visible in JavaScript for example--although we do need to keep them in sync with the array inside DOMException.cpp. There’s no need for the 105, 106 thing, and no reason to leave empty slots where no-longer-used values were. I don’t think we need to include all these “introduced in xxx” comments either; not really important when they were introduced. I also think they should be in alphabetical order, not historical order. But we should still distinguish the ones that are web-exposed and the ones that are not. We should consider renaming each of these internal constants to use the style that matches the error names in the specification such as HierarchyRequestError rather than the legacy code name HIERARCHY_REQUEST_ERR. Once everything else is done I think we should consider getting rid of NoException and use optional instead when we need a null value.
Comment on attachment 316292 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316292&action=review >> Source/WebCore/dom/ExceptionCode.h:25 >> +enum ExceptionCode { > > For future clean-up here: > > I don’t think any of the values in this enumeration need explicitly set values any more--no reason they need to match the legacy codes visible in JavaScript for example--although we do need to keep them in sync with the array inside DOMException.cpp. There’s no need for the 105, 106 thing, and no reason to leave empty slots where no-longer-used values were. I don’t think we need to include all these “introduced in xxx” comments either; not really important when they were introduced. I also think they should be in alphabetical order, not historical order. But we should still distinguish the ones that are web-exposed and the ones that are not. > > We should consider renaming each of these internal constants to use the style that matches the error names in the specification such as HierarchyRequestError rather than the legacy code name HIERARCHY_REQUEST_ERR. > > Once everything else is done I think we should consider getting rid of NoException and use optional instead when we need a null value. > We should consider renaming each of these internal constants to use the style that matches the error names in the specification such as HierarchyRequestError rather than the legacy code name HIERARCHY_REQUEST_ERR. I had the same thought and this was my next patch :)
Created attachment 316302 [details] Patch
Comment on attachment 316302 [details] Patch Clearing flags on attachment: 316302 Committed r219831: <http://trac.webkit.org/changeset/219831>
All reviewed patches have been landed. Closing bug.