Bug 71081 - Move type-specific exception descriptions into the implementation files for each type
Summary: Move type-specific exception descriptions into the implementation files for e...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks: 70890
  Show dependency treegraph
 
Reported: 2011-10-27 16:38 PDT by Adam Barth
Modified: 2011-10-27 21:44 PDT (History)
1 user (show)

See Also:


Attachments
Patch (73.83 KB, patch)
2011-10-27 16:39 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (92.91 KB, patch)
2011-10-27 17:58 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (91.71 KB, patch)
2011-10-27 18:04 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (91.85 KB, patch)
2011-10-27 18:22 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (91.92 KB, patch)
2011-10-27 18:24 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2011-10-27 16:38:52 PDT
Move type-specific exception descriptions into the implementation files for each type
Comment 1 Adam Barth 2011-10-27 16:39:48 PDT
Created attachment 112782 [details]
Patch
Comment 2 Eric Seidel (no email) 2011-10-27 16:42:15 PDT
Comment on attachment 112782 [details]
Patch

crazy.  Looks fine if EWS says it's fine.
Comment 3 Adam Barth 2011-10-27 17:58:55 PDT
Created attachment 112794 [details]
Patch
Comment 4 Eric Seidel (no email) 2011-10-27 18:03:17 PDT
Comment on attachment 112794 [details]
Patch

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

LGTM.

> Source/WebCore/dom/EventException.cpp:54
> +    description->typeName = "DOM Events";
> +    description->code = ec - EventExceptionOffset;
> +    description->type = EventExceptionType;

So strange that we're passed in a Description, instead of returning one.  Then it's constructor could do all this jazz.  I guess we could have a fill() function until then.
Comment 5 Adam Barth 2011-10-27 18:04:13 PDT
Created attachment 112796 [details]
Patch
Comment 6 Gyuyoung Kim 2011-10-27 18:15:24 PDT
Comment on attachment 112796 [details]
Patch

Attachment 112796 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10223979
Comment 7 Adam Barth 2011-10-27 18:15:58 PDT
It's to match the API of http://trac.webkit.org/browser/trunk/Source/WebCore/dom/ExceptionCode.h#L97
Comment 8 Adam Barth 2011-10-27 18:22:16 PDT
Created attachment 112800 [details]
Patch
Comment 9 Adam Barth 2011-10-27 18:24:01 PDT
Created attachment 112803 [details]
Patch
Comment 10 Eric Seidel (no email) 2011-10-27 19:49:39 PDT
Comment on attachment 112803 [details]
Patch

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

Our build system(s) setup is epicly bad.  We really need to end this madness.

> Source/WebCore/dom/DOMCoreException.cpp:34
> +// FIXME: This should be an array of structs to pair the names and descriptions.

Thank you for adding this.

> Source/WebCore/dom/DOMCoreException.cpp:104
> +    description->typeName = "DOM";
> +    description->code = ec;
> +    description->type = DOMExceptionType;
> +
> +    size_t tableSize = WTF_ARRAY_LENGTH(coreExceptionNames);
> +    size_t tableIndex = ec - INDEX_SIZE_ERR;
> +
> +    description->name = tableIndex < tableSize ? coreExceptionNames[tableIndex] : 0;
> +    description->description = tableIndex < tableSize ? coreExceptionDescriptions[tableIndex] : 0;

This should be some sort of constructor or "fill" method in a later iteration of this cleanup.
Comment 11 Adam Barth 2011-10-27 21:44:42 PDT
Committed r98686: <http://trac.webkit.org/changeset/98686>