[FlyWeb] Simplify mDiscoveryManagerTable




3 years ago
3 years ago


(Reporter: sicking, Unassigned)


(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)



(1 attachment)

Created attachment 8753302 [details] [diff] [review]
Patch to fix

The mDiscoveryManagerTable hash table can be simplified.

Right now it's a hash table mapping FlyWebDiscoveryManager to a struct containing a bool. We use this bool to track if the FlyWebDiscoveryManager is interested in being notified about services-list changes or not.

It would be simpler to only register the FlyWebDiscoveryManager when we actually care about getting NotifyDiscoveredServicesChanged() notifications, and then unregister when we no longer care about notifications.

This way we also don't need to track "active" FlyWebDiscoveryManagers in FlyWebService. Any registered manager is an active one and so we check the size of mDiscoveryManagerTable to see if there are active managers.

This way mDiscoveryManagerTable can also be a hash-set rather than a hash-map.

Another simplification we can do is to let the id's returned by StartDiscovery be an integer rather than an integer prefixed by a string.

Finally this patch makes FlyWebDiscoveryManager::mCallbackMap a nsRefPtrHashtable rather than a nsClassHashtable of RefPtrs.
Attachment #8753302 - Flags: review?(kvijayan)
Comment on attachment 8753302 [details] [diff] [review]
Patch to fix

Review of attachment 8753302 [details] [diff] [review]:

Looks good.  Nice simplification.
Attachment #8753302 - Flags: review?(kvijayan) → review+
This landed: http://hg.mozilla.org/integration/mozilla-inbound/rev/576019c74103
As parts of nightly upmerge for bug 1272099, bug 1272101.
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.