834193 - Implement CFStateChangeEvent, DataErrorEvent, USSDReceivedEvent using codegenerator

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

Attachment: v1

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 705914 [details] [diff] [review]
v1

>diff --git a/dom/base/nsDOMClassInfo.cpp b/dom/base/nsDOMClassInfo.cpp
...
> #ifdef MOZ_B2G_RIL
> #include "nsIDOMMobileConnection.h"
>+#include "nsIDOMUSSDReceivedEvent.h"
>+#include "nsIDOMDataErrorEvent.h"
Actually, you shouldn't need these #includes.
nsDOMClassInfo.cpp defines MOZ_GENERATED_EVENTS_INCLUDES and then #include "GeneratedEvents.h"
so the interfaces for the generated events are included automatically.
(you can see GeneratedEvents.h in <objdir>/js/xpconnect/src/GeneratedEvents.h)

Comment 4

[José Antonio Olivera Ortega [:jaoo]]

Blocking this per comment #32 in bug 827280.

Comment 5

[Vicamo Yang [:vicamo][:vyang]]

Rewrite CFStateChangeEvent as well after bug 827280.

Comment 6

[Vicamo Yang [:vicamo][:vyang]]

Attachment: v2

1) Address review comment in comment #3.
2) Rewrite CFStateChangeEvent as well

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 706849 [details] [diff] [review]
v2

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

::: js/xpconnect/src/event_impl_gen.conf.in
@@ +32,5 @@
>      'MozWifiStatusChangeEvent',
>      'MozWifiConnectionInfoEvent',
>      'MozCellBroadcastEvent',
> +    'USSDReceivedEvent',
> +    'DataErrorEvent',

Should these two actually be in a MOZ_B2G_RIL ifdef?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 706849 [details] [diff] [review]
v2

Yes, USSDReceivedEvent and DataErrorEvent should e b2g only stuff, but 
very unfortunately they are already exposed in desktop (release) builds.
Please file a followup to fix that and make it block bug Bug 734145.
This bug could be just about using codegenerator for various events.

So, update uuid of nsIDOMCFStateChangeEvent and keep the old behavior
of USSDReceivedEvent and DataErrorEvent where they are exposed to desktop.

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Olli Pettay [:smaug] from comment #8)
> Yes, USSDReceivedEvent and DataErrorEvent should e b2g only
> stuff, but very unfortunately they are already exposed in
> desktop (release) builds. Please file a followup to fix that
> and make it block bug Bug 734145. This bug could be just about
> using codegenerator for various events.

Hei, I've already moved all the three events in MOZ_B2G_RIL in this patch. Is there still anything other than uuid update of nsIDOMCFStateChangeEvent to be done?

> So, update uuid of nsIDOMCFStateChangeEvent and keep the old
> behavior of USSDReceivedEvent and DataErrorEvent where they
> are exposed to desktop.

Same above. Is it still necessary to keep old behaviour?

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Well, it is a change to behavior in desktop build.
Event if those events aren't really usable in desktop build, one can detect them.
"DataErrorEvent" in window is true.

At least get a review from someone who reviewed Bug 734145.

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

Attachment: v3

Update uuid of nsIDOMCFStateChangeEvent only. As review comment #10 said, maybe Bent can help give some feedbacks on removing USSDReceivedEvent and DataErrorEvent from destkop build?

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 706867 [details] [diff] [review]
v3

Mounir, do you mind have a review on these changes?

Comment 13

[Mounir Lamouri (:mounir)]

Comment on attachment 706867 [details] [diff] [review]
v3

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

Olli should review that. Sorry for the delay Vicamo.

Comment 14

[Mounir Lamouri (:mounir)]

Comment on attachment 706867 [details] [diff] [review]
v3

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

Oh... He already reviewed it. Just land that then. His review his enough ;)

Comment 15

[Mounir Lamouri (:mounir)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #11)
> Created attachment 706867 [details] [diff] [review]
> v3
> 
> Update uuid of nsIDOMCFStateChangeEvent only. As review comment #10 said,
> maybe Bent can help give some feedbacks on removing USSDReceivedEvent and
> DataErrorEvent from destkop build?

I do not think there would be any problem with that. r=me if you need.

Comment 16

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Mounir Lamouri (:mounir) from comment #13)
> Sorry for the delay Vicamo.

You don't have to. I know you probably get a ton of review requests :)

Comment 17

[Vicamo Yang [:vicamo][:vyang]]

Attachment: v4

Rebase only. Try https://tbpl.mozilla.org/?tree=Try&rev=acb0585c406c

Comment 18

[Vicamo Yang [:vicamo][:vyang]]

Attachment: v5

Rebase after bug 835148

Comment 19

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/26f2a61829ac

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/26f2a61829ac