WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
109267
Migrate ExceptionCode ASSERTs in SVG to ASSERT_NO_EXCEPTION.
https://bugs.webkit.org/show_bug.cgi?id=109267
Summary
Migrate ExceptionCode ASSERTs in SVG to ASSERT_NO_EXCEPTION.
Mike West
Reported
2013-02-08 00:22:58 PST
I tried to land changes in SVG code as part of
https://bugs.webkit.org/show_bug.cgi?id=109044
; they caused test failures, and I'm not sure why. I'll find out and fix the problems in a smaller, more focused patch.
Attachments
Patch
(11.37 KB, patch)
2013-02-08 03:20 PST
,
Mike West
no flags
Details
Formatted Diff
Diff
Patch for landing
(11.17 KB, patch)
2013-02-08 04:12 PST
,
Mike West
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Mike West
Comment 1
2013-02-08 01:33:14 PST
It looks like ExceptionCodePlaceholder doesn't do the right thing with the negation operator, which is upsetting to SVGLength::setValue, which has different behavior based on 'ec's value (
http://trac.webkit.org/browser/trunk/Source/WebCore/svg/SVGLength.cpp?rev=142247#L213
).
Mike West
Comment 2
2013-02-08 02:38:42 PST
Ugh. Now I understand what ASSERT_NO_EXCEPTION is doing. It burns. Off the top of my head, I have no idea how to make it behave in a way that would make `if (!ec)` work in debug. :( I will ponder.
Mike West
Comment 3
2013-02-08 03:09:06 PST
So, as it turns out, this behavior is intentional: ASSERT_NO_EXCEPTION attempts to ensure that methods which care about the value of an ExceptionCode must initialize that ExceptionCode to 0 (see
https://bugs.webkit.org/show_bug.cgi?id=78194
). SVGLength::setValue does not.
Mike West
Comment 4
2013-02-08 03:20:30 PST
Created
attachment 187278
[details]
Patch
Mike West
Comment 5
2013-02-08 03:21:27 PST
Hola, Jochen. Interested in reviewing this patch? :)
jochen
Comment 6
2013-02-08 03:39:40 PST
Comment on
attachment 187278
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=187278&action=review
ok
> Source/WebCore/ChangeLog:19 > + again with the latter. It does the same for 'ASSERT(ec == 0);' (and, as
I don't see wuch a change in the code?
Mike West
Comment 7
2013-02-08 04:11:00 PST
(In reply to
comment #6
)
> (From update of
attachment 187278
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=187278&action=review
> > ok > > > Source/WebCore/ChangeLog:19 > > + again with the latter. It does the same for 'ASSERT(ec == 0);' (and, as > > I don't see wuch a change in the code?
The joys of copy/paste. I'll fix that. :)
Mike West
Comment 8
2013-02-08 04:12:27 PST
Created
attachment 187288
[details]
Patch for landing
WebKit Review Bot
Comment 9
2013-02-08 04:56:31 PST
Comment on
attachment 187288
[details]
Patch for landing Clearing flags on attachment: 187288 Committed
r142261
: <
http://trac.webkit.org/changeset/142261
>
WebKit Review Bot
Comment 10
2013-02-08 04:56:36 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug