Closed
Bug 1228667
Opened 8 years ago
Closed 8 years ago
Implement FlyWeb service discovery
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
People
(Reporter: djvj, Assigned: djvj)
References
Details
Attachments
(3 files, 25 obsolete files)
56.56 KB,
patch
|
Details | Diff | Splinter Review | |
16.53 KB,
patch
|
Details | Diff | Splinter Review | |
7.98 KB,
patch
|
Details | Diff | Splinter Review |
Build a first-stab implementation of FlyWeb service discovery in Gecko. The initial implementation of this will use mDNS-based service discovery, but should expand to other discovery protocols over time. As such, the interfaces here should be general enough to accommodate extension to separate transport protocols in the future. The discovery model for FlyWeb has the following basic structure: 1. A collection of "known services" which mutates asynchronously as new services are discovered, and known services are lost. 2. Every known service is associated with a name. 3. Known services start out in an 'unconnected' state. They API can be instructed to attempt to connect to one of them, which attempts to establish a connected session between the browser and the service. 4. At the end of a successful connection, the browser assigns a generated, internal name to the service. Most likely a UUID.
Assignee | ||
Comment 1•8 years ago
|
||
Added some IDL and basic stub implementation code. I am new to XPCOM, IDLs, and gecko/necko in general, so learning as I go.
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8693081 -
Attachment is obsolete: true
Assignee | ||
Comment 3•8 years ago
|
||
More baby steps. This should add a service for instantiating nsFlyWebTransportManager instances. The overspecifiation in nsFlyWebTransportManager is just a testing ground to figure out how xpcom IDL semantics work on smaller APIs before moving onto doing nsFlyWebService and all the things that flow from that, which are the bigger tasks ahead. Anyway, this seems to build (and therefore, it must be correct).
Attachment #8693126 -
Attachment is obsolete: true
Assignee | ||
Comment 4•8 years ago
|
||
More fill-in. Pared down the interfaces a bit - the API now is just discovery with the connection methods removed. Implemented ServiceList, ServiceManager, and ServiceInfo.
Attachment #8698236 -
Attachment is obsolete: true
Assignee | ||
Comment 5•8 years ago
|
||
Fixed a couple bugs with refcount handling. Brief testing using Browser Console - the basics seem to work. I can construct an nsIFlyWebTransportManager service instance and get |internetTransport| from it. I can construct an nsIFlyWebServiceManager service instance and get an empty list of nsIFlyWebServiceList from it.
Attachment #8698604 -
Attachment is obsolete: true
Assignee | ||
Comment 6•8 years ago
|
||
Fixed up service discovery interfaces. This should be good enough to move work onto actually implementing the "internet" (mDNS/DNS-SD) case.
Attachment #8699124 -
Attachment is obsolete: true
Assignee | ||
Comment 7•8 years ago
|
||
Initial start to internet discovery implementation.
Assignee | ||
Comment 8•8 years ago
|
||
Updated discovery patch. This hooks into nsDNSServiceDiscovery, and looks for services. Currently the policy is to do internet service discovery on a set interval. Currently hardcoded to every 10 seconds (3 second scan duration, 10 second overall interval), just for debug testing. Also hardcoded to discover '_http._tcp' services, since those exist on the network and I can see them already. Seems to work well enough on desktop osx.
Attachment #8699289 -
Attachment is obsolete: true
Assignee | ||
Comment 9•8 years ago
|
||
Updated internet discovery code. FlyWebInternetServiceDiscoveryHandler now discovers and resolves '_http._tcp' services (only tested on OSX). For this to work, you also need to include two bugfix patches currently open: https://bugzilla.mozilla.org/show_bug.cgi?id=1229726 https://bugzilla.mozilla.org/show_bug.cgi?id=1233813 These fix issues in mDNS code that gets in the way of discovery.
Attachment #8699646 -
Attachment is obsolete: true
Assignee | ||
Comment 10•8 years ago
|
||
Updated internet discovery. Wired up to 'listServices()' call on nsIDNSServiceManager so that internet-based services are listed. Still doing hardcoded '_http._tcp' services though.
Attachment #8700180 -
Attachment is obsolete: true
Assignee | ||
Comment 11•8 years ago
|
||
Discovery UI patch for firefox desktop - mostly written by mconley as a christmas gift (yay mconley!). It adds a toolbar icon to list services. It doesn't do anything with the listed services as of yet because there's an underlying memory error in my discovery implementation. For some reason, when the UI obtains a nsIFlyWebServiceList, an later that list gets collected, it deletes the underlying nsIFlyWebServiceInfo objects, even though they should be held live by nsIFlyWebServiceManager service object. Spent most of the day trying to track down why that was happening, but didn't get anywhere concrete.
Assignee | ||
Comment 12•8 years ago
|
||
Updated internet discovery patch. Mostly some cleanups. It's still buggy with the refcount issue.
Attachment #8701226 -
Attachment is obsolete: true
Assignee | ||
Comment 13•8 years ago
|
||
Not updated discovery UI patch. Just re-sending it to get the patch order correct.
Attachment #8701641 -
Attachment is obsolete: true
Assignee | ||
Comment 14•8 years ago
|
||
New patch name, same patch (basic service discovery registry and xpcom API).
Attachment #8699288 -
Attachment is obsolete: true
Assignee | ||
Comment 15•8 years ago
|
||
Internet discovery code, now with refcount issues fixed.
Attachment #8701872 -
Attachment is obsolete: true
Assignee | ||
Comment 16•8 years ago
|
||
Same patch, new name.
Attachment #8701873 -
Attachment is obsolete: true
Assignee | ||
Comment 17•8 years ago
|
||
Updated implementation based off of 'larch' tree, using webidl to implement flyweb discovery. This is not building yet. Just learning my way around webidl. I'm getting a strange error message with this patch. Not sure why it wants me to implement "GetWrapperPreserveColor", but it seems more like something that should be autogenerated that is not getting autogenned because I forgot something: 2:38.78 In file included from /Users/kannanvijayan/checkouts/larch/BUILD-DEBUG64/dom/bindings/UnifiedBindings6.cpp:50: 2:38.78 ./FlyWebDiscoveryManagerBinding.cpp:142:21: error: no member named 'GetWrapperPreserveColor' in 'mozilla::dom::FlyWebDiscoveredService' 2:38.79 if (self && self->GetWrapperPreserveColor()) { 2:38.79 ~~~~ ^ 2:38.79 ./FlyWebDiscoveryManagerBinding.cpp:334:68: error: no member named 'GetParentObject' in 'mozilla::dom::FlyWebDiscoveredService' 2:38.79 JS::Rooted<JSObject*> parent(aCx, WrapNativeParent(aCx, aObject->GetParentObject())); 2:38.79 ~~~~~~~ ^ 2:38.81 ./FlyWebDiscoveryManagerBinding.cpp:1035:68: error: no member named 'GetParentObject' in 'mozilla::dom::FlyWebDiscoveredServiceList' 2:38.81 JS::Rooted<JSObject*> parent(aCx, WrapNativeParent(aCx, aObject->GetParentObject())); 2:38.81 ~~~~~~~ ^ 2:38.82 ./FlyWebDiscoveryManagerBinding.cpp:1237:21: error: no member named 'GetWrapperPreserveColor' in 'mozilla::dom::FlyWebDiscoveryManager' 2:38.82 if (self && self->GetWrapperPreserveColor()) { 2:38.82 ~~~~ ^ 2:38.82 ./FlyWebDiscoveryManagerBinding.cpp:1426:68: error: no member named 'GetParentObject' in 'mozilla::dom::FlyWebDiscoveryManager' 2:38.82 JS::Rooted<JSObject*> parent(aCx, WrapNativeParent(aCx, aObject->GetParentObject()));
(In reply to Kannan Vijayan [:djvj] from comment #17) > This is not building yet. Just learning my way around webidl. I'm getting > a strange error message with this patch. Not sure why it wants me to > implement "GetWrapperPreserveColor", but it seems more like something that > should be autogenerated that is not getting autogenned because I forgot > something: > Classes that implement a webidl must also inherit from nsWrapperCache and override a few methods. dom/base/PerformanceEntry.* is a good example
Assignee | ||
Comment 19•8 years ago
|
||
Thanks for that. It helped out. Updated patch that builds. Just the interfaces and some basic mock implementation code so far.
Attachment #8706673 -
Attachment is obsolete: true
(In reply to Kannan Vijayan [:djvj] from comment #19) > Updated patch that builds. Just the interfaces and some basic mock > implementation code so far. Also note that webidl definitions usually live under dom/webidl, and implementations somewhere in dom/. They usually call into the required networking things via XPCOM. Not sure if this is a hard rule though. Better ask one of the DOM peers.
Assignee | ||
Comment 21•8 years ago
|
||
More filling in of WebIDL code.
Attachment #8707097 -
Attachment is obsolete: true
Assignee | ||
Comment 22•8 years ago
|
||
Updated WebIDL patch after figuring out more WebIDL stuff. I can instantiate a FlyWebDiscoveryManager from chrome JS now, and have it hand me back an empty list of discovered services. FlyWebDiscoveryManager, FlyWebDiscoveredServiceList, and FlyWebDiscoveredService objects all seem to work. Now need to plumb them to actual discovery logic.
Attachment #8707181 -
Attachment is obsolete: true
Assignee | ||
Comment 23•8 years ago
|
||
Updated base patch for service discovery infrastructure in webidl. Builds and works.
Attachment #8707613 -
Attachment is obsolete: true
Assignee | ||
Comment 24•8 years ago
|
||
New patch to wire in mDNS-based service discovery. Not wired into UI yet but the objects can be instantiated and used from chrome JS, and listServices can be used to get a list of MDNS services.
Assignee | ||
Comment 25•8 years ago
|
||
Comment on attachment 8707880 [details] [diff] [review] 1-service-discovery-webidl.patch Review of attachment 8707880 [details] [diff] [review]: ----------------------------------------------------------------- Checking for initial feedback on discovery patches. Jonas, can you take a quick look at this and let me know what you think?
Attachment #8707880 -
Flags: feedback?(jonas)
Assignee | ||
Comment 26•8 years ago
|
||
Comment on attachment 8707883 [details] [diff] [review] 2-mdns-discovery.patch Review of attachment 8707883 [details] [diff] [review]: ----------------------------------------------------------------- Checking for initial feedback on discovery patches. Jonas, can you take a quick look at this and let me know what you think?
Attachment #8707883 -
Flags: feedback?(jonas)
Assignee | ||
Comment 27•8 years ago
|
||
Added flyweb ui patch.
Attachment #8708055 -
Flags: feedback?(jonas)
Attachment #8708055 -
Flags: feedback?(jdarcangelo)
Assignee | ||
Comment 28•8 years ago
|
||
(In reply to Valentin Gosu [:valentin] from comment #20) > Classes that implement a webidl must also inherit from nsWrapperCache and > override a few methods. > dom/base/PerformanceEntry.* is a good example Thanks! That was very useful.
Assignee | ||
Comment 29•8 years ago
|
||
Update service discovery patch, all rolled into one. Working off of larch tip.
Attachment #8702669 -
Attachment is obsolete: true
Attachment #8702670 -
Attachment is obsolete: true
Attachment #8702671 -
Attachment is obsolete: true
Attachment #8707880 -
Attachment is obsolete: true
Attachment #8707883 -
Attachment is obsolete: true
Attachment #8708055 -
Attachment is obsolete: true
Attachment #8707880 -
Flags: feedback?(jonas)
Attachment #8707883 -
Flags: feedback?(jonas)
Attachment #8708055 -
Flags: feedback?(jonas)
Attachment #8708055 -
Flags: feedback?(jdarcangelo)
Assignee | ||
Comment 30•8 years ago
|
||
Patch that adds 'connectToService' method to the internal FlyWebDiscoveryManager API. Also changes the flyweb UI button to open pages to the connected services when menu items are clicked. API is not promise-based or anything yet, but we now have a full UI for discovering and opening webpages to '_http._tcp' pages.
Assignee | ||
Comment 31•8 years ago
|
||
Attachment #8709196 -
Attachment is obsolete: true
Assignee | ||
Comment 32•8 years ago
|
||
Attachment #8709197 -
Attachment is obsolete: true
Assignee | ||
Comment 33•8 years ago
|
||
Last patch in this series enables discovery of both '_http._tcp' and '_flyweb._tcp' services.
Assignee | ||
Updated•8 years ago
|
Attachment #8709623 -
Flags: review?(jonas)
Assignee | ||
Updated•8 years ago
|
Attachment #8709624 -
Flags: review?(jonas)
Assignee | ||
Updated•8 years ago
|
Attachment #8709625 -
Flags: review?(jonas)
Assignee | ||
Comment 34•8 years ago
|
||
Closing this bug as it has landed in larch. http://hg.mozilla.org/projects/larch/rev/e426071f5f3c http://hg.mozilla.org/projects/larch/rev/40183f878ffb http://hg.mozilla.org/projects/larch/rev/bfa03eb798e9
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Attachment #8709623 -
Flags: review?(jonas)
Attachment #8709624 -
Flags: review?(jonas)
Attachment #8709625 -
Flags: review?(jonas)
You need to log in
before you can comment on or make changes to this bug.
Description
•