Closed Bug 1253181 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: mantaroh, Assigned: mantaroh)

Details

(Whiteboard: btpp-active)

Attachments

(1 file, 1 obsolete file)

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)
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+
Thanks!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9e2e521cbd13
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.