We need a better application development environment for Presentation API. This can help promoting the new API to community.

Hi schien I intend to working on this bug. So I investigated some bugs.(bug 1184036 / bug 1184073..etc) Please let me ask some questions about Presentation API. Question: 1. Do you have a plan to implement the other DeviceProvider? (e.g. HDMI / Miracast / MulticastDNS) 2. Could you tell me the supported platform of Presentation API? 3. What is the reason of not supporting Presentation API in Desktop Firefox?

(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #1) > Hi schien > > I intend to working on this bug. So I investigated some bugs.(bug 1184036 / > bug 1184073..etc) Thanks a lot for taking over this task! Please see my answer below. > Please let me ask some questions about Presentation API. > > Question: > 1. Do you have a plan to implement the other DeviceProvider? (e.g. HDMI / > Miracast / MulticastDNS) MulticastDNS is our current implementation HDMI support in bug 1208417 Miracast support is blocked by bug 925615 DIAL support is under investigation > 2. Could you tell me the supported platform of Presentation API? Right now only on Firefox OS (Gonk and Mac), need implement mDNS on Linux and Windows for simulator. Fennec implementation is ongoing, will support controller side. > 3. What is the reason of not supporting Presentation API in Desktop Firefox? 1. API spec is still changing rapidly at current stage 2. need mDNS support on all desktop OS, only Mac is implemented now.

Thanks schien. (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #2) > (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #1) > > 2. Could you tell me the supported platform of Presentation API? > Right now only on Firefox OS (Gonk and Mac), need implement mDNS on Linux > and Windows for simulator. OK. I know your plan and necessary implementation. According to bug1115480, current mdnsresponder is not support windows and linux. right? So I think that problem is not linked mdnsresponder. (linux : bug 1184073 / windows : ?)

(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #3) > Thanks schien. > > (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment > #2) > > (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #1) > > > 2. Could you tell me the supported platform of Presentation API? > > Right now only on Firefox OS (Gonk and Mac), need implement mDNS on Linux > > and Windows for simulator. > OK. I know your plan and necessary implementation. > > According to bug1115480, current mdnsresponder is not support windows and > linux. right? > So I think that problem is not linked mdnsresponder. (linux : bug 1184073 / > windows : ?) linux: bug 1225736 windows: bug 1239909 Using Presentation API doesn't require to use mDNS if we only allow running sender and receiver on the same computer. It'll depends on the design of the debugging environment.

Hi schien. (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #4) > (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #3) > > Thanks schien. > > > > (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment > > #2) > > > (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #1) > > > > 2. Could you tell me the supported platform of Presentation API? > > > Right now only on Firefox OS (Gonk and Mac), need implement mDNS on Linux > > > and Windows for simulator. > > OK. I know your plan and necessary implementation. > > > > According to bug1115480, current mdnsresponder is not support windows and > > linux. right? > > So I think that problem is not linked mdnsresponder. (linux : bug 1184073 / > > windows : ?) > > linux: bug 1225736 > windows: bug 1239909 > > Using Presentation API doesn't require to use mDNS if we only allow running > sender and receiver on the same computer. It'll depends on the design of the > debugging environment. Thanks. I think that Web Developer will create |Controlling Page| and |Presentation Page| using by Presentation API. They will verify their page on simulator of tv or connected device(or smartphone). So We should provide Simulator Device Provider in order to confirm Web Content which using PresentationAPI is correct. And Simulator Device Provider will provide basic functions: - Can start PresentationRequest. - Can display PresentationURL in not Simulator window but new popup document.(Simulating presentation display) - Can communicate with |Controlling page| and |Presentation page|. - Can join with id.

Hi schien. I created the demonstration video. There are two demonstration videos. 1) Simulate presentation display as new window. Simulation of Presentation will create new chrome window(simulated Presentation Display), and communicate with |Controller Page(App)| and |Presentation Page(new window)|. 2) Simulation presentation display as inner document. Simulation of Presentation API will create new document in the |Controller Page(App)|, and communicate with |Controller Page(App)| and |Presentation Page(inner document)|. I would like to get your feedback about follow points. - I considered that 1-UA only. Is it OK? - I would like to recommend Chrome window. I think that the Web Developer who will use this API want to verify the |Controller Page(App)|.

(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #5) > And Simulator Device Provider will provide basic functions: > - Can start PresentationRequest. > - Can display PresentationURL in not Simulator window but new popup > document.(Simulating presentation display) > - Can communicate with |Controlling page| and |Presentation page|. > - Can join with id. Adding to above function, I think that Simulator should have function as follow: - The Simulator can support multiple display. - The Simulator can change state of PresentationConnection.

Yes, the Chrome window one is definitely works for developer working on controller page. This can be the first step and we can think more about how to let developer debugging the receiver page on TV side.

Interface of XPCOM only.

Hi schien. I have a question about Receiver. Presentation receiver created by |Presentation::Init|[1]. Is this receiver never change? In the case of 1-UA, I think that the receiver will change after calling |Presentation::Init|. [1] https://dxr.mozilla.org/mozilla-central/source/dom/presentation/Presentation.cpp#65

(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #13) > Hi schien. > > I have a question about Receiver. > Presentation receiver created by |Presentation::Init|[1]. > Is this receiver never change? > In the case of 1-UA, I think that the receiver will change after calling > |Presentation::Init|. > > [1] > https://dxr.mozilla.org/mozilla-central/source/dom/presentation/Presentation. > cpp#65 Did you encounter bug 1234128 while testing your patches? If so, I'll raise the priority of that bug and try fix it as soon as I can.

Thanks! (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #14) > Did you encounter bug 1234128 while testing your patches? If so, I'll raise > the priority of that bug and try fix it as soon as I can. Yes. My comment #13 is same as bug 1234128. Now I don't necessary immediately. However, I would like to know that how you resolve that bug. (e.g. when change the |mReceiver|? What is the trigger to change receiver?) Thanks.

The Presentation receiver object should be the same but need extra synchronization handling between chrome process and content process. Bug 1234128 is more like a race condition of webapi implementation to me.

I'll separate this bug to several patches. 1. Add empty XPCOM(Device provider). 2. Add controlling channel code(include launching the receiver). 3. Add TCP Connecting code(Communicate with receiver). 4. tests and tests. As first step, I add the SimulatorProvider to the presentation DOM. This is providing the dummy PresentationDevice and PresentationControlChannel. This provider will launch the receiver and communicate with sender and receiver. https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c114035ec3a

Comment on attachment 8720141 [details] [diff] [review] Add SimulatorProvider to Presentation DOM. Hi schein. I added the XPCOM which stimulate for the Presentation Device template file. I'll check-in after all patches reviewed. So could you please review patch one by one?

Sorry I forgot writing the hg log.

Comment on attachment 8721194 [details] [diff] [review] Part1.Add SimulatorProvider to Presentation DOM. Review of attachment 8721194 [details] [diff] [review]: ----------------------------------------------------------------- need to correct the control channel / transport channel establishment for session.send() / session.onmessage. ::: dom/presentation/provider/SimulatorProvider.js @@ +51,5 @@ > +function SimulatorDevice() { > + // Bug 1217713 Currently, device simulator is fixed value. > + this.mId = "0"; > + this.mName = "simulator"; > + this.mType = "type0"; id = "simulator-0" name = "Simulator Device" type = "simulator" @@ +76,5 @@ > +function SimulatorPresentationControlChannel() { > + this._listener = undefined; > +} > + > +SimulatorPresentationControlChannel.prototype = { You can leverage dom/presentation/provider/TCPPresentationServer.js for the control channel / transport channel establishment, otherwise you'll be blocked by bug 1195221. You'll also need to add the "GetAddress" implementation in https://dxr.mozilla.org/mozilla-central/source/dom/presentation/PresentationSessionInfo.cpp#533 for simulator, "127.0.0.1" should be enough. ::: dom/presentation/provider/moz.build @@ +13,5 @@ > + EXTRA_COMPONENTS += [ > + 'SimulatorProvider.js', > + ] > + > + nit: remove one extra new line.

Thanks schien. (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #21) > Comment on attachment 8721194 [details] [diff] [review] > Part1.Add SimulatorProvider to Presentation DOM. > You can leverage dom/presentation/provider/TCPPresentationServer.js for the > control channel / transport channel establishment, otherwise you'll be > blocked by bug 1195221. OK. I'll use TCPPresentationServer.

We should modify gaia parts in order to notify launched remote. (like multi_screen_contoroller.js)

Hi schien. I have a question about current Presentation API implementation. - In PresentationSessionInfo.cpp[1], The address and port of receiver is not used. Are you going to be using this? - In PresentationSessioninfo.cpp, Perhaps, the receiver will connect to server(sender)[2]. However There aren't the way to know sender address and port in receiver with TCPPresentationServer. [1] https://dxr.mozilla.org/mozilla-central/rev/be593a64d7c6a826260514fe758ef32a6ee580f7/dom/presentation/PresentationSessionInfo.cpp#579 [2] https://dxr.mozilla.org/mozilla-central/rev/be593a64d7c6a826260514fe758ef32a6ee580f7/dom/presentation/PresentationSessionInfo.cpp#448 Thanks.

(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #24) > Hi schien. > > I have a question about current Presentation API implementation. > - In PresentationSessionInfo.cpp[1], The address and port of receiver is > not used. Are you going to be using this? No, we don't need to use these information for TCP-type connection. > - In PresentationSessioninfo.cpp, Perhaps, the receiver will connect to > server(sender)[2]. However There aren't the way to know sender address and > port in receiver with TCPPresentationServer. I've put a note on this at comment #21. You'll also need to add the "GetAddress" implementation in https://dxr.mozilla.org/mozilla-central/source/dom/presentation/PresentationSessionInfo.cpp#533 for simulator, "127.0.0.1" should be enough. > > [1] > https://dxr.mozilla.org/mozilla-central/rev/ > be593a64d7c6a826260514fe758ef32a6ee580f7/dom/presentation/ > PresentationSessionInfo.cpp#579 > [2] > https://dxr.mozilla.org/mozilla-central/rev/ > be593a64d7c6a826260514fe758ef32a6ee580f7/dom/presentation/ > PresentationSessionInfo.cpp#448 > > Thanks.

Many thanks, schine. After IRC chat, I investigated the packet of handshake, and I understood handshake communication of control channel. In my understand, handshaking is as follow: ----------------------------------------------- Sender Receiver | ------------ Request:Init ------------> | | ------------ Request:Offer -----------> | | <----------- Connect:DataTransfer ----- | | <----------- Request:Answer ----------- | [Called PresentationConnection.send() from Sender] | -- Send Data on transfer connection --> | [Called PresentationConnection.send() from Receiver] | <-- Send data on transfer connection -- | ----------------------------------------------- Is my understanding incorrect? In my case, I misunderstood that sender didn't send the message data via PresentationConnection.send(). In fact, I confirm the packet of those sending data. [1] https://dxr.mozilla.org/mozilla-central/rev/05c087337043dd8e71cc27bdb5b9d55fd00aaa26/dom/presentation/PresentationSessionTransport.cpp#117 Thanks.

Yes that's correct. The full path of sending a message is following: PresentationConnection.send() => PresentationService.sendSessionMessage() => PresentationSessionInfo::Send() => PresentationSessionTransport::Send() => sending data over TCP socket. The full path of receiving a message is following: PresentationSessionTransport::OnDataAvailable => PresentationSessionInfo::NotifyData() => PresentationConnection::NotifyMessage() => PresentationConnection.onmessage event handler in web page.

Many thanks shcien! I modify the SimulatorProvider using TCPPresentationServer, and I'll add the launching the new window code and gaia side. - create new window in SimulatorProvider.js - modify gaia system(multi_window.js) when simulator. Could you confirm this patch?

Comment on attachment 8729396 [details] [diff] [review] Part1.Add SimulatorProvider to Presentation API. Review of attachment 8729396 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/presentation/PresentationSessionInfo.cpp @@ +535,5 @@ > + NS_DispatchToMainThread( > + NS_NewRunnableMethodWithArg<nsCString>( > + this, > + &PresentationControllingInfo::OnGetAddress, > + "127.0.0.1")); NS_LITERAL_CSTRING("127.0.0.1") ::: dom/presentation/provider/BuiltinProviders.manifest @@ +2,5 @@ > contract @mozilla.org/presentation-device/tcp-presentation-server;1 {f4079b8b-ede5-4b90-a112-5b415a931deb} > + > +#ifdef MOZ_MULET > +component {1a9820e5-dcd0-4079-a4d6-f788136274bd} SimulatorProvider.js > +contract @mozilla.org/presentation-device/internal-simulator-provider;1 {1a9820e5-dcd0-4079-a4d6-f788136274bd} simulator-provider is enough. ::: dom/presentation/provider/SimulatorProvider.js @@ +19,5 @@ > +function SimulatorProvider() { > + this._device = undefined; > + this._deviceListener = undefined; > + this._tcpServer = undefined; > + this._tcpServerPort = undefined; Don't need to declare attributes that is undefined. @@ +34,5 @@ > + forceDiscovery : function() { > + if (!this._tcpServer) { > + debug("create server"); > + this.createTCPServer(); > + } Is this case really happened? tcpServer is created at object initialization phase and is never removed. @@ +38,5 @@ > + } > + }, > + > + set listener(aListener) { > + this._device = new SimulatorDevice(this); You can also use plain JS object here to save some code. @@ +86,5 @@ > + _requestSession: function sp_requestSession(aDevice, aUrl, aPresentationId) { > + // The sender should connect the receiver TCPPresentationServer > + let deviceInfo = new SimulatorDeviceInfo(aDevice.id, > + "127.0.0.1", > + this._tcpServerPort); use a plain JS object is enough, see https://dxr.mozilla.org/mozilla-central/source/dom/presentation/tests/xpcshell/test_tcp_control_channel.js#131 @@ +89,5 @@ > + "127.0.0.1", > + this._tcpServerPort); > + > + return this._tcpServer.requestSession(deviceInfo, aUrl, aPresentationId); > + nit: remove trailing space. @@ +103,5 @@ > + */ > +function SimulatorDevice(aProvider) { > + this.mId = "simuator-0"; > + this.mName = "Simulator Device"; > + this.mType = "simulator"; You can return the string directly at getter function and remove these members. @@ +116,5 @@ > + establishControlChannel(url, presentationId) { > + return this._provider._requestSession(this, url, presentationId); > + }, > + > + classID: Components.ID("{31a2db09-d0c4-409b-9e77-a7602f323676}"), You don't need classID for SimulatorDevice. @@ +142,5 @@ > + > +this.NSGetFactory = XPCOMUtils.generateNSGetFactory([ > + SimulatorProvider, > + SimulatorDevice, > + SimulatorDeviceInfo]); You only need factory for SimulatorProvider.

schien, thanks. I addressed your comment.

schien, thanks. I would like to get your feedback second part patch. Especially, the implementation method of simulator window. The second patch is gecko/b2g components(PresentationRequestUIGlue). I'm going to use the remote window(system/js/remote)[1]. So I fixed that |PresentationRequestUIGlue| notified simulator flag to |multi_screen_controller.js|.[2] I'll modify multi_screen_controller.js to notify |launch-presentation-ap| event to remote window[3] in order to communicate with remote window and presentation internal implementation. [1] http://mxr.mozilla.org/gaia/source/apps/system/js/remote/ [2] http://mxr.mozilla.org/gaia/source/apps/system/js/multi_screen_controller.js

I'm going to implement gaia part as follow. https://github.com/mantaroh/gaia/commit/cfb5932f7c4d5c82af75bf8128b4bf06eb9ceb68

Comment on attachment 8731108 [details] [diff] [review] Part2. Add b2g components of PresentationSimulator. Review of attachment 8731108 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/components/PresentationRequestUIGlue.js @@ +73,5 @@ > SystemAppProxy._sendCustomEvent("mozPresentationChromeEvent", > { type: "presentation-launch-receiver", > url: aUrl, > + id: aSessionId, > + simulator: aIsSimulator }); hmm...do we really need this attribute? I thought we only need a simulator-specific event handler. Need to see the corresponding UI code to understand the usage.

Comment on attachment 8731076 [details] [diff] [review] Part1.Add SimulatorProvider to Presentation API. Review of attachment 8731076 [details] [diff] [review]: ----------------------------------------------------------------- What do you think about using plain JS object without function/prototype declaration for SimulatorDevice and SimulatorDeviceInfo, like in my previous comment #29?

Add ni? for comment #34.

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #34) > Comment on attachment 8731076 [details] [diff] [review] > Part1.Add SimulatorProvider to Presentation API. > > Review of attachment 8731076 [details] [diff] [review]: > ----------------------------------------------------------------- > > What do you think about using plain JS object without function/prototype > declaration for SimulatorDevice and SimulatorDeviceInfo, like in my previous > comment #29? Oh sorry, I misunderstood your comment. I fixed SimulatorDeviceInfo to plain JS object.

Comment on attachment 8731522 [details] Part3. Modify gaia part in order to launch the presentation app. Thanks SChien! (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #33) > hmm...do we really need this attribute? I thought we only need a > simulator-specific event handler. Need to see the corresponding UI code to > understand the usage. Could you please confirm Gaia part patch? This patch is work in progress, however It can run as expected. So I'm going to change the event flow is as follow, and change the gaia part like this patch. Current implementation don't have event handler which corresponding |presentation-launch-receiver|. So I would like to add the this event handler to |multi_screen_controller|. >--------------------------------------------------------------------------------------------------------------------- >(Gecko Layer) | (Gaia layer) >--------------------------------------------------------------------------------------------------------------------- >[PresentatoinRequestUIGlue] |[multi_screen_controller] [remote/message_controller] [remote/app_window_manager] > | -- presentation-launch-receiver --> | | | > | | |(if simulator) | | > | | | --launch-presentation-app-->| | > | | | | --lanchPresentationApp()--> |(Note.1) > | | | | | > | | | <-------------- presentation-receier-launched ------------|(Note.2) > | <--presentation-receiver-launched--| | | > | | | | | >--------------------------------------------------------------------------------------------------------------------- Note.1: Launch presentatoin application on remote window. Note.2: Notify that application has launched, when received mozbrowserloadend event.

Comment on attachment 8731514 [details] [diff] [review] Part1.Add SimulatorProvider to Presentation API. Review of attachment 8731514 [details] [diff] [review]: ----------------------------------------------------------------- The top-level window management should be able to shared with Tommy's work on HDMI provider. Tommy can you take a look at this part? r+ for rest of the work. ::: dom/presentation/provider/SimulatorProvider.js @@ +1,1 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public Add mode line at the begin, see https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Mode_Line @@ +2,5 @@ > + * License, v. 2.0. If a copy of the MPL was not distributed with this > + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > +"use strict"; > + > +const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components; Cr is not used. @@ +8,5 @@ > +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > +Cu.import("resource://gre/modules/Services.jsm"); > + > +function debug(aMsg) { > + dump("[SimulatorProvider]" + aMsg + "\n"); remove the debug function if no code is using it, or comment out the log before check-in. @@ +22,5 @@ > + > +SimulatorProvider.prototype = { > + _init: function() { > + this._createTCPServer(); > + }, This function is only one line and used in one place. You can simply put "this._createTCPServer()" into constructor. @@ +83,5 @@ > + this._tcpServerPort = this._tcpServer.port; > + }, > + > + _requestSession: function sp_requestSession(aDevice, aUrl, aPresentationId) { > + // When lanching simulator, we use the MultiScreen implementation in gaia. s/lanching/launching @@ +95,5 @@ > + "width=" + maxWidth + ",height=" + maxHeight; > + let remoteShellURL = Services.prefs.getCharPref("b2g.multiscreen.chrome_remote_url") + > + "#PresentationSimulator-receiver"; > + this._receiverWin = Services.ww.openWindow(null, > + remoteShellURL, nit: trailing space. @@ +111,5 @@ > + }; > + > + // The sender should connect the receiver TCPPresentationServer > + return this._tcpServer.requestSession(simulatorDeviceInfo, aUrl, aPresentationId); > + nit: remove extra new line.

launch-presentation-app is handled in apps/system/js/remote/message_controller.js. Tommy can you explain the 1-UA flow to @mantaroh so that you can leverage each others work? Thanks!

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #40) > launch-presentation-app is handled in > apps/system/js/remote/message_controller.js. Tommy can you explain the 1-UA > flow to @mantaroh so that you can leverage each others work? > Thanks! Hi mantaroh, there is some architecture changed in Gaia side when Bug 1235123 fixed. I think your Gaia patch should be waited for Bug 1235123 fixed. And you don't need to create a broadcast channel in remote_app_window_manager.js by yourself. It is already created in message_controller.js. .

Hi Tommy! (In reply to Tommy Kuo [:KuoE0] from comment #41) > (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment > #40) > Hi mantaroh, there is some architecture changed in Gaia side when Bug > 1235123 fixed. I think your Gaia patch should be waited for Bug 1235123 > fixed. This change is changing each window messaging. Perhaps those change will affect for this bugs. I'm going to wait for your change. Thanks.

Comment on attachment 8731514 [details] [diff] [review] Part1.Add SimulatorProvider to Presentation API. Review of attachment 8731514 [details] [diff] [review]: ----------------------------------------------------------------- Almost looks good to me. Just some suggestions following. ::: dom/presentation/provider/SimulatorProvider.js @@ +31,5 @@ > + > + set listener(aListener) { > + // nsIPresentationDevice > + this._device = { > + id: "simulator-0", Is it possible to have more then one simulators? If no, I think the ID can be only "simulator" like ID of HDMI display is only "hdmi". @@ +100,5 @@ > + "PresentationSimulatorDisplay", > + flags, > + null); > + } > + I prefer to put the code (the entire `if (!this._receiverWin)` block) to open window into the implementation of the simulator device. I think it is a functionality of the device. And call the function here. You can refer to the patch (part 2) in Bug 1208417.

Thanks schien,tommy. (In reply to Tommy Kuo [:KuoE0] from comment #43) > Comment on attachment 8731514 [details] [diff] [review] > Part1.Add SimulatorProvider to Presentation API. > > Review of attachment 8731514 [details] [diff] [review]: > ----------------------------------------------------------------- > > Almost looks good to me. Just some suggestions following. I modified you guys comments. This patch is carrying forward r+ from schien.(comment #39) Thanks.

This patch is effective after landed bug 1208417. When runtime is simulator, multi_screen send the |launch-presentation-app| event in order to open presentation url.