Last Comment Bug 761613 - Merge nsIPrivateDOMEvent to nsIDOMEvent
: Merge nsIPrivateDOMEvent to nsIDOMEvent
Status: RESOLVED FIXED
: addon-compat, dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: x86_64 All
: -- normal (vote)
: mozilla16
Assigned To: Olli Pettay [:smaug] (review request backlog because of a work week)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 761621 761729
Blocks: 764479
  Show dependency treegraph
 
Reported: 2012-06-05 08:01 PDT by Olli Pettay [:smaug] (review request backlog because of a work week)
Modified: 2012-12-01 23:55 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP (182.89 KB, patch)
2012-06-05 08:01 PDT, Olli Pettay [:smaug] (review request backlog because of a work week)
no flags Details | Diff | Splinter Review
WIP2 (187.42 KB, patch)
2012-06-05 10:28 PDT, Olli Pettay [:smaug] (review request backlog because of a work week)
no flags Details | Diff | Splinter Review
WIP3 (189.71 KB, patch)
2012-06-06 07:40 PDT, Olli Pettay [:smaug] (review request backlog because of a work week)
no flags Details | Diff | Splinter Review
WIP4 (191.40 KB, patch)
2012-06-06 09:16 PDT, Olli Pettay [:smaug] (review request backlog because of a work week)
no flags Details | Diff | Splinter Review
patch (190.21 KB, patch)
2012-06-06 11:58 PDT, Olli Pettay [:smaug] (review request backlog because of a work week)
jst: review+
Details | Diff | Splinter Review
up-to-date (190.38 KB, patch)
2012-06-10 10:55 PDT, Olli Pettay [:smaug] (review request backlog because of a work week)
no flags Details | Diff | Splinter Review
up-to-date (191.08 KB, patch)
2012-06-10 11:14 PDT, Olli Pettay [:smaug] (review request backlog because of a work week)
no flags Details | Diff | Splinter Review

Description User image Olli Pettay [:smaug] (review request backlog because of a work week) 2012-06-05 08:01:21 PDT
Created attachment 630172 [details] [diff] [review]
WIP

Currently C++ code needs to almost always QI to nsIPrivateDOMEvent when dispatching events,
because SetTrusted etc live in that interface. We should just merge nsIPrivateDOMEvent
to nsIDOMEvent.

The patch ends up removing about 240 loc.

https://tbpl.mozilla.org/?tree=Try&rev=a6abab555883
Comment 1 User image :Ms2ger (⌚ UTC+1/+2) 2012-06-05 08:26:26 PDT
Comment on attachment 630172 [details] [diff] [review]
WIP

Review of attachment 630172 [details] [diff] [review]:
-----------------------------------------------------------------

\o/

::: content/base/src/nsContentUtils.cpp
@@ +3373,1 @@
>    NS_ENSURE_SUCCESS(rv, rv);

Followup to see if we need to return nsresult?

@@ +4638,5 @@
>  //static
>  nsEvent*
>  nsContentUtils::GetNativeEvent(nsIDOMEvent* aDOMEvent)
>  {
> +  return aDOMEvent ? aDOMEvent->GetInternalNSEvent() : nsnull;

Followup to remove this function?

::: content/events/src/nsDOMNotifyPaintEvent.h
@@ +31,5 @@
> +  {
> +    return nsDOMEvent::DuplicatePrivateData();
> +  }
> +  NS_IMETHOD_(void) Serialize(IPC::Message* aMsg, bool aSerializeInterfaceType);
> +  NS_IMETHOD_(bool) Deserialize(const IPC::Message* aMsg, void** aIter);

Can we keep those as they were? Adding [notxpcom] should do that

::: layout/base/nsDocumentViewer.cpp
@@ +1116,5 @@
>    NS_ENSURE_SUCCESS(rv, rv);
>  
>    // XXX Dispatching to |window|, but using |document| as the target.
>    nsCOMPtr<nsIDOMEventTarget> target = do_QueryInterface(mDocument);
> +  event->SetTarget(target);

While you're here, you should be able to drop the QI

::: layout/xul/base/src/nsXULPopupManager.cpp
@@ +436,5 @@
>      // get the event coordinates relative to the root frame of the document
>      // containing the popup.
> +    NS_ASSERTION(aPopup, "Expected a popup node");
> +    nsEvent* event;
> +    event = aEvent->GetInternalNSEvent();

One line, please

::: security/manager/ssl/src/nsSmartCardEvent.cpp
@@ +20,5 @@
>  
>  //NS_DECL_DOM_CLASSINFO(SmartCardEvent)
>  
>  // QueryInterface implementation for nsXMLHttpRequest
>  NS_INTERFACE_MAP_BEGIN(nsSmartCardEvent)

for nsXMLHttpRequest? While you're here.... ;)
Comment 2 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2012-06-05 10:28:35 PDT
Created attachment 630219 [details] [diff] [review]
WIP2

Perhaps this compiles on OSX.

https://tbpl.mozilla.org/?tree=Try&rev=1701fce94997
Comment 3 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2012-06-06 07:40:13 PDT
Created attachment 630566 [details] [diff] [review]
WIP3

Change some silly browser/chrome tests

https://tbpl.mozilla.org/?tree=Try&rev=245a3d278afa
Comment 4 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2012-06-06 09:16:56 PDT
Created attachment 630588 [details] [diff] [review]
WIP4

I'll need to still mark interfaces to builtinclass
Comment 5 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2012-06-06 11:58:09 PDT
Created attachment 630654 [details] [diff] [review]
patch

https://tbpl.mozilla.org/?tree=Try&rev=563255dd88fc

Huge patch but mainly pretty mechanical changes.
Not using nostdcall in the .idl based on the comments from bsmedberg.
Comment 6 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2012-06-10 10:55:26 PDT
Created attachment 631754 [details] [diff] [review]
up-to-date
Comment 7 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2012-06-10 11:14:49 PDT
Created attachment 631756 [details] [diff] [review]
up-to-date
Comment 8 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2012-06-10 12:18:07 PDT
https://hg.mozilla.org/mozilla-central/rev/e011e0440daf
Comment 9 User image Gregor Wagner [:gwagner] 2012-06-13 16:41:24 PDT
smaug, this patch regressed a bunch of eventlisteners implemented in JS for B2G. jst and cjones vote for backing out if there is no easy fix.
Comment 10 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2012-06-14 03:50:44 PDT
Eh. Sure this didn't regress any event listener implementations. All the event listeners in 
web pages are implemented in JS.
Comment 11 User image Florian Quèze [:florian] [:flo] 2012-10-20 09:27:33 PDT
I'm currently updating Instantbird to use Mozilla 16, and the changes in this bug mean I need to update/remove the code using nsIPrivateDOMEvent for a binary component (the binary component using nsIPrivateDOMEvent was original from the MintrayR add-on).

I would have expected this interface removal to be documented on https://developer.mozilla.org/en-US/docs/Firefox_16_for_developers.

Note You need to log in before you can comment on or make changes to this bug.