WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
36545
[Chromium] Add an ASSERT macro to the Chromium WebKit API
https://bugs.webkit.org/show_bug.cgi?id=36545
Summary
[Chromium] Add an ASSERT macro to the Chromium WebKit API
Jeremy Orlow
Reported
2010-03-24 10:39:36 PDT
[Chromium] Add an ASSERT macro to the Chromium WebKit API
Attachments
Patch
(4.86 KB, patch)
2010-03-24 10:45 PDT
,
Jeremy Orlow
fishd
: review-
Details
Formatted Diff
Diff
fix issues
(4.85 KB, patch)
2010-03-25 03:42 PDT
,
Jeremy Orlow
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Jeremy Orlow
Comment 1
2010-03-24 10:45:34 PDT
Created
attachment 51522
[details]
Patch
Darin Fisher (:fishd, Google)
Comment 2
2010-03-24 21:07:58 PDT
Comment on
attachment 51522
[details]
Patch
> +++ b/WebKit/chromium/public/WebCommon.h
...
> } // namespace WebKit > > +// ----------------------------------------------------------------------------- > +// Assertions > + > +namespace WebKit { > + > +void failedAssertion(const char* file, int line, const char* function, const char* assertion); > + > +} // namespace WebKit
Why close and then re-open the WebKit namespace? Please annotate WebKit API entry points with WEBKIT_API.
> +// Only use inside the public directory but outside of WEBKIT_IMPLEMENTATION blocks. Otherwise use WTF's ASSERT.
There is actually no problem using this outside of WEBKIT_IMPLEMENTATION blocks.
> +#if defined(NDEBUG) > +#define WEBKIT_ASSERT(assertion) ((void)0) > +#else > +#define WEBKIT_ASSERT(assertion) do \ > + if (!(assertion)) { \ > + WebKit::failedAssertion(__FILE__, __LINE__, __FUNCTION__, #assertion); \ > + } \
^^^ nit: webkit style says no brackets around single line statements.
Jeremy Orlow
Comment 3
2010-03-25 03:35:53 PDT
> > +// Only use inside the public directory but outside of WEBKIT_IMPLEMENTATION blocks. Otherwise use WTF's ASSERT. > > There is actually no problem using this outside of WEBKIT_IMPLEMENTATION > blocks.
I know, but the WebCore asserts are more robust and have extra logic for turning them on/off. Just seemed like a good idea to point people in that direction primarily.
> > +#if defined(NDEBUG) > > +#define WEBKIT_ASSERT(assertion) ((void)0) > > +#else > > +#define WEBKIT_ASSERT(assertion) do \ > > + if (!(assertion)) { \ > > + WebKit::failedAssertion(__FILE__, __LINE__, __FUNCTION__, #assertion); \ > > + } \ > > ^^^ nit: webkit style says no brackets around single line statements.
Just copied what was in Assertions (and then removed a line). I suppose brackets should be around the do/while tho.
Jeremy Orlow
Comment 4
2010-03-25 03:42:36 PDT
Created
attachment 51618
[details]
fix issues
WebKit Commit Bot
Comment 5
2010-03-25 16:48:40 PDT
Comment on
attachment 51618
[details]
fix issues Clearing flags on attachment: 51618 Committed
r56579
: <
http://trac.webkit.org/changeset/56579
>
WebKit Commit Bot
Comment 6
2010-03-25 16:48:46 PDT
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