1139453 - Linux nsNotifyAddr should detect "offline" when no network interface is UP

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Daniel Stenberg [:bagder]]

The nsNotifyAddr logic for Firefox on Linux should work similarly to the Windows backend and signal connectivity based on available network interfaces. If there is a network interface available that is UP and isn't lookback, consider it connected.

Outstanding question: should the same logic apply for FxOS ?

Comment 1

[Daniel Stenberg [:bagder]]

Attachment: 0001-bug-1139453-send-NS_NETWORK_LINK_DATA_UP-DOWN-events.patch

Here's a first take to make the Linux version act similar to how we do on Windows.

If there's a network interface UP that isn't a loopback, consider the link up. Otherwise down.

Comment 2

[Daniel Stenberg [:bagder]]

Comment on attachment 8573217 [details] [diff] [review]
0001-bug-1139453-send-NS_NETWORK_LINK_DATA_UP-DOWN-events.patch

Review handed to :valentin with permission from :jduel

Comment 3

[Daniel Stenberg [:bagder]]

Comment on attachment 8573217 [details] [diff] [review]
0001-bug-1139453-send-NS_NETWORK_LINK_DATA_UP-DOWN-events.patch

Ah, scratch that for now. This patch doesn't build/work for b2g since bionic doesn't have getifaddrs(). Need to deal with that somehow first.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Take a look at NetworkManager.js which handles up/down for b2g.
We could even ignore b2g for the moment and just fix it for Desktop Linux.

Comment 5

[Daniel Stenberg [:bagder]]

Ah yes. I'll add some magic to make it desktop-linux only.

Comment 6

[Daniel Stenberg [:bagder]]

Attachment: v2-0001-bug-1139453-send-network-UP-DOWN-events-on-deskto.patch

Yeps, here's a version that explicitly doesn't do the checklink on 2bg but does it on Linux.

Comment 7

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8577238 [details] [diff] [review]
v2-0001-bug-1139453-send-network-UP-DOWN-events-on-deskto.patch

Review of attachment 8577238 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.
r=valentin with the #ifndef change.

::: netwerk/system/linux/nsNotifyAddrListener_Linux.cpp
@@ +108,5 @@
> +{
> +    // b2g instead has NetworkManager.js which handles UP/DOWN
> +}
> +#else
> +void nsNotifyAddrListener::checkLink(void)

The usual pattern I've seen is
void nsNotifyAddrListener::checkLink(void)
{
#ifndef MOZ_WIDGET_GONK
  do stuff
#endif
  return default...
}
 
This way we only have only definition of this method.

Comment 8

[Daniel Stenberg [:bagder]]

Attachment: v3-0001-bug-1139453-send-network-UP-DOWN-events-on-deskto.patch

Modified as per review comment.

Try-run (v2): https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc7bd9be12cb

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a80860d9add6

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a80860d9add6