[Presentation API] fix the code of calling not existing listener call.

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
3 years ago
2 months ago

People

(Reporter: mantaroh, Assigned: mantaroh)

Tracking

unspecified
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

(Whiteboard: btpp-active)

Attachments

(1 attachment, 1 obsolete attachment)

TCPPresentationServer is using the not exist listener call as follow.

|listener._onOffer| [1] 
|listener._onAnswer| [2]

Perhaps, I think we should remove '_listener' object.
Is it right?

[1] https://dxr.mozilla.org/mozilla-central/rev/c59c022943f6a7e79f6002e11fc9f56cb836f5dd/dom/presentation/provider/TCPPresentationServer.js#583
[2] https://dxr.mozilla.org/mozilla-central/rev/c59c022943f6a7e79f6002e11fc9f56cb836f5dd/dom/presentation/provider/TCPPresentationServer.js#591

And perhaps type in onSessionRequest[3]. I think it should require '_' prefix before 'listener'.
[3] https://dxr.mozilla.org/mozilla-central/rev/c59c022943f6a7e79f6002e11fc9f56cb836f5dd/dom/presentation/provider/TCPPresentationServer.js#152
Attachment #8726109 - Flags: feedback?(schien)
Comment on attachment 8726109 [details] [diff] [review]
Fix the variable name in TCPPresentationServer.js

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

::: dom/presentation/provider/TCPPresentationServer.js
@@ +148,5 @@
>    // Triggered by TCPControlChannel
>    onSessionRequest: function(aDeviceInfo, aUrl, aPresentationId, aControlChannel) {
>      DEBUG && log("TCPPresentationServer - onSessionRequest: "
>                   + aDeviceInfo.address + ":" + aDeviceInfo.port);
> +    this._listener.onSessionRequest(aDeviceInfo,

the original code is correct because we have a "get listener" function.

@@ +579,5 @@
>      if (this._pendingOffer) {
>        let offer = this._pendingOffer;
>        DEBUG && log("TCPControlChannel - notify pending offer: "
>                     + JSON.stringify(offer));
> +      this._onOffer(new ChannelDescription(offer));

correct fix should be this._listener.onOffer(new ChannelDescription(offer));

@@ +587,5 @@
>      if (this._pendingAnswer) {
>        let answer = this._pendingAnswer;
>        DEBUG && log("TCPControlChannel - notify pending answer: "
>                     + JSON.stringify(answer));
> +      this._onAnswer(new ChannelDescription(answer));

this._listener.onAnswer(new ChannelDescription(answer));
Attachment #8726109 - Flags: feedback?(schien)
Assignee

Comment 3

3 years ago
Thanks schien.

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #1)
> the original code is correct because we have a "get listener" function.
Oh, I misunderstood those implementation.

> @@ +579,5 @@
> correct fix should be this._listener.onOffer(new ChannelDescription(offer));
OK. I fixed. and I'll create xpsshell test of TCPPresentationServer at other new bug.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ea4c89d09c9
Assignee: nobody → mantaroh
Attachment #8726109 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8726230 - Flags: review?(schien)
Whiteboard: btpp-active
Comment on attachment 8726230 [details] [diff] [review]
Fix the variable name in TCPPresentationServer.js

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

lgtm!
Attachment #8726230 - Flags: review?(schien) → review+
Assignee

Comment 5

3 years ago
Thanks!
Keywords: checkin-needed

Comment 7

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9e2e521cbd13
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.