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-
fix issues (4.85 KB, patch)
2010-03-25 03:42 PDT, Jeremy Orlow
no flags
Jeremy Orlow
Comment 1 2010-03-24 10:45:34 PDT
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.