Maybe khuey can help review these?

Comment on attachment 678126 [details] [diff] [review] Part b: Make nsPaintRequest implement nsWrapperCache; Review of attachment 678126 [details] [diff] [review]: ----------------------------------------------------------------- r=me with these: ::: content/events/src/nsPaintRequest.cpp @@ +24,5 @@ > > +NS_IMPL_CYCLE_COLLECTING_ADDREF(nsPaintRequest) > +NS_IMPL_CYCLE_COLLECTING_RELEASE(nsPaintRequest) > + > +/* virtual */ JSObject* Nit: The virtual comment doesn't really add much here imo. ::: content/events/src/nsPaintRequest.h @@ +18,4 @@ > { > public: > + NS_DECL_CYCLE_COLLECTING_ISUPPORTS > + NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(nsPaintRequestList) "nsPaintRequest" here. Oh, looks like this is corrected in the next patch. @@ +22,2 @@ > NS_DECL_NSIDOMPAINTREQUEST > + Nit: whitespace

Comment on attachment 678127 [details] [diff] [review] Part c: Implement Paris bindings for PaintRequest; Review of attachment 678127 [details] [diff] [review]: ----------------------------------------------------------------- r=me with these: ::: content/events/src/nsPaintRequest.cpp @@ +56,3 @@ > { > + nsString result; > + GetReason(result); You won't need the temporary here after changing the signature. ::: content/events/src/nsPaintRequest.h @@ +10,5 @@ > #include "nsIDOMPaintRequestList.h" > #include "nsPresContext.h" > #include "nsIDOMEvent.h" > #include "mozilla/Attributes.h" > +#include "nsClientRect.h" Nit: You can just forward declare this. @@ +36,5 @@ > + return mParent; > + } > + > + already_AddRefed<nsClientRect> ClientRect(); > + void GetReason(nsString& aResult) const Nit: nsAString ::: dom/interfaces/events/nsIDOMPaintRequest.idl @@ +24,5 @@ > * needed to repaint the rectangle due to scrolling, and "scroll copy", meaning > * that we updated the rectangle due to scrolling but instead of painting > * manually, we were able to do a copy from another area of the screen. > */ > + [binaryname(DOMReason)] Calling this "DOMReason" seems wrong as the actual DOM will not call this. What about XPCOMReason? IDLReason? I don't care too strongly as long as it's not "DOM".

(In reply to ben turner [:bent] from comment #6) > Comment on attachment 678127 [details] [diff] [review] > Part c: Implement Paris bindings for PaintRequest; > > Review of attachment 678127 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: content/events/src/nsPaintRequest.h > @@ +10,5 @@ > > #include "nsIDOMPaintRequestList.h" > > #include "nsPresContext.h" > > #include "nsIDOMEvent.h" > > #include "mozilla/Attributes.h" > > +#include "nsClientRect.h" > > Nit: You can just forward declare this. I tried; the generated code fails to build because it can't assign an nsClientRect* to an nsIDOMClientRect* without this include. I thought it was better to keep returning the concrete type, to avoid having to change this code when moving ClientRect to the new bindings. Will address your other comments.

Ah, ok. Just remove the include from the cpp then.

https://hg.mozilla.org/mozilla-central/rev/c4267242d766 https://hg.mozilla.org/mozilla-central/rev/75157a7f9a06 https://hg.mozilla.org/mozilla-central/rev/8ca4011a1250