[webkit-reviews] review granted: [Bug 54331] Finish up implementing the new event model in IndexedDB : [Attachment 82374] another

[bugzilla-daemon at webkit.org]

Nate Chapin <japhet at chromium.org> has granted Jeremy Orlow
<jorlow at chromium.org>'s request for review:
Bug 54331: Finish up implementing the new event model in IndexedDB
https://bugs.webkit.org/show_bug.cgi?id=54331

Attachment 82374: another
https://bugs.webkit.org/attachment.cgi?id=82374&action=review

------- Additional Comments from Nate Chapin <japhet at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=82374&action=review

SO MUCH RED! :)

Just a couple of nits, and a caveat that I skimmed the ~500KB of test
expectations changes.

> Source/WebCore/storage/IDBEventDispatcher.cpp:83
> +    // FIXME: "...However, we also wanted to integrate the window.onerror
feature in
> +    //	 HTML5. So after we've fired an "error" event, if
.preventDefault() was
> +    //	 never called on the event, we fire an error event on the
window (can't
> +    //	 remember if this happens before or after we abort the
transaction).
> +    //	 This is a separate event, which for example means that even if
you
> +    //	 attach a capturing "error" handler on window, you won't see
any events
> +    //	 unless an error really went unhandled. And you also can't call

> +    //	 .preventDefault on the error event fired on the window in
order to
> +    //	 prevent the transaction from being aborted. It's purely there
for
> +    //	 error reporting and distinctly different from the event
propagating to
> +    //	 the window.
> +    //	 
> +    //	 This is similar to how "error" events are handled in workers.
> +    //	 
> +    //	 (I think that so far webkit hasn't implemented the
window.onerror
> +    //	 feature yet, so you probably don't want to fire the separate
error
> +    //	 event on the window until that has been implemented)."
> +

[citation needed]

(Sorry for not complaining when I approved it for IDBEvent.cpp).

> Source/WebCore/storage/IDBEventDispatcher.h:31
> +#ifndef IDBEventDispatcher_h
> +#define IDBEvent_h
> +

#define mismatch