Closed Bug 1550605 Opened 10 months ago Closed 7 months ago

network connection status api

Categories

(WebExtensions :: General, enhancement, P2)

enhancement

Tracking

(firefox69 verified, firefox70 verified)

VERIFIED FIXED
mozilla70
Tracking Status
firefox69 --- verified
firefox70 --- verified

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(2 files)

Consider exposing some network connection events.

https://searchfox.org/mozilla-central/source/netwerk/base/nsINetworkLinkService.idl

we could have an api that has:
browser.network.getStatus() (up/down/etc)
browser.network.getLinkType() (wifi/usb/etc)
browser.network.onConnectionChanged

The onConnectionChanged would send an object essentially matching the values in nsINetworkLinkService whenever either "network:link-status-changed" or "network:link-type-changed" notifications are sent.

P3 for more info from dragana

Due to the simplicity of this api and that soft freeze has not happened yet, I wrote up the patch quickly. But IIUC :dragana has some concerns that the notifications could happen too much.

Assignee: nobody → mixedpuppy

This should probably be dup'ed to bug 1514589. I didn't want to do that with the open patch here.
For reference, this could use some explanation about the use case, I could swear we had some discussion about exposing this earlier and decided not to do it, but I can't seem to find it.

(In reply to Andrew Swan [:aswan] from comment #3)

This should probably be dup'ed to bug 1514589.

Maybe, maybe not. That seems to be an external request for a public api, for now at least, I was intending this to be a privileged api to avoid public promises around it.

See Also: → 1552972
Duplicate of this bug: 1552972
Summary: network connection api → network connection status api

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:mixedpuppy, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(mixedpuppy)

This will wait for additional platform work targeting Fx70.

Flags: needinfo?(mixedpuppy)
Priority: P3 → P2
Depends on: 1561005

Since the network id landed, we'll try this for 69, better sooner than later for secure proxy.

Attached file networkStatus.xpi

Simple extension dumps status to console when network status updates.

While this will have a test, it uses a mock network service. The attached xpi will dump the status changes to the console. QE could use this to validate that they can see real network changes on the different platforms. e.g. turn wifi off then on. I see:

{"id":"bweB5kWjxX8iirGKroX5tWq38vg=","status":"down","type":"unknown"}
{"status":"down","type":"unknown"}
{"status":"up","type":"unknown"}

and on reload

{"id":"bweB5kWjxX8iirGKroX5tWq38vg=","status":"up","type":"unknown"}

There may be some timing issue about getting the network id via the onConnectionChanged handler.

Flags: qe-verify?

another QE note: this is a privileged api, the attached extension can only be tested by loading via about:debugging on nightly or dev-ed.

Pushed by scaraveo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/599521424e2a
add networkStatus api r=rpl,mayhemer
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

Can this be uplifted to 69, in case we need it DoH?

Flags: needinfo?(mixedpuppy)

(In reply to Nhi Nguyen (:nhi) from comment #18)

Can this be uplifted to 69, in case we need it DoH?

It's a larger chunk of code than I'd like to uplift, but I also don't feel it's a terribly risky uplift either. I'll put in a request, you may want to touch base with relman or whomever you'll need to convince if necessary.

Flags: needinfo?(mixedpuppy)

Comment on attachment 9063919 [details]
Bug 1550605 add networkStatus api

Beta/Release Uplift Approval Request

  • User impact if declined: requested by :nhi for secure-proxy.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Attached extension will dump the network id to console, see comment 11
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): This is a larger patch than I like uplifting, but there's still lots of time before next uplift, and I don't feel like anything in it is particularly scary. The dependency which contains the lower level platform support, bug 1561005, landed in time for 69.
  • String changes made/needed: none
Attachment #9063919 - Flags: approval-mozilla-beta?
Flags: qe-verify? → qe-verify+
QA Whiteboard: [qa-triaged]
Duplicate of this bug: 1569126

It would be good for QA to look at this first before we commit to uplifting. I'm not crazy about uplifting new APIs to Beta halfway through the cycle.

of note: values in this api are simply reflected from what platform provides us. The test I outlined for QE is providing them with a network id, but no network type. For myself, I currently get neither id or type. The API itself is working fine.

Verified as fixed in FF70 with the results described by Shane in comment #23.

Status: RESOLVED → VERIFIED

Comment on attachment 9063919 [details]
Bug 1550605 add networkStatus api

Self-contained WebExtension API with test coverage. Approved for 69.0b10.

Attachment #9063919 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.