1034312 - Telephony uses NS_DECL_NSITELEPHONYLISTENER, but does not inherit from nsITelephonyListener

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

We should keep the current setup with the other object that is the "real" nsITelephonyListener that forwards to the Telephony object, but we can inherit directly from nsITelephonyListener too.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 2

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

Comment on attachment 8450578 [details] [diff] [review]
Patch

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

Can you also fix all similar components:

  dom/voicemail/Voicemail.h
  dom/telephony/Telephony.h
  dom/mobileconnection/src/MobileConnection.h
  dom/cellbroadcast/src/CellBroadcast.h

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I bet none of the others are built on desktop.  I haven't tried a b2g build with the patch from bug 1034302 yet.

Comment 4

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

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #3)
> I bet none of the others are built on desktop.  I haven't tried a b2g build
> with the patch from bug 1034302 yet.

Hey, this suddenly reminds me that we have these strange listeners for the reason that xpconnect forbid us exposing non-DOM interfaces on DOM objects, so Telephony just couldn't inherit nsITelephonyListener. Since we have converted these nodes to WebIDL, this problem has gone and we can simply remove those Foo::Listener classes.

Comment 5

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

Attachment: 0001-Bug-1034312-inherit-from-nsIFooListener-directly-for.patch

JavascriptException: JavascriptException: NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "aListener.enumerateCallStateComplete is not a function" {file: "jar:file:///system/b2g/omni.ja!/components/TelephonyService.js" line: 397}]'[JavaScript Error: "aListener.enumerateCallStateComplete is not a function" {file: "jar:file:///system/b2g/omni.ja!/components/TelephonyService.js" line: 397}]' when calling method: [nsITelephonyService::enumerateCalls] at: app://system.gaiamobile.org/js/screen_manager.js line: 403

Comment 6

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

[JavaScript Error: "No handler for notifyRadioStateChanged" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 2140}]
[JavaScript Error: "No handler for notifyVoiceChanged" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 2140}]
[JavaScript Error: "No handler for notifyDataChanged" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 2140}]
[JavaScript Error: "No handler for notifyVoiceChanged" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 2140}]
[JavaScript Error: "No handler for notifyDataChanged" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 2140}]
[JavaScript Error: "No handler for notifyIccChanged" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 2140}]
[JavaScript Error: "No handler for notifyVoiceChanged" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 2140}]
[JavaScript Error: "No handler for notifyDataChanged" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 2140}]
[JavaScript Error: "No handler for notifyStatusChanged" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 2140}]

Looks like it just can't be done in this way?

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

You have to make them [ChromeOnly] methods on the WebIDL object to expose them to JS.

Comment 8

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

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #7)
> You have to make them [ChromeOnly] methods on the WebIDL object to expose
> them to JS.

But nsITelephonyListener is a XPIDL interface? Or you mean I have to duplicate them into Telephony.webidl? But then the C++ function signatures are still different, so I'm supposed to overload these listener methods? That make me feel I shouldn't have this right now. I'd rather take your revision instead.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

The commit message needs to be updated.  Consider that done.

Comment 10

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

Comment on attachment 8468568 [details] [diff] [review]
Patch

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

mobileconnection part may need rebase. Thank you :)

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/integration/b2g-inbound/rev/fe06c48beccd

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/fe06c48beccd