1142586 - Network CHANGED should not imply polling for UP/DOWN status

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Daniel Stenberg [:bagder]]

Bug 1132693 made nsIOService::OnNetworkLinkEvent() poll the link service for UP/DOWN status even for CHANGED events which seems wrong. The check _can_ be fairly expensive.

I cannot see a reason for that as per how the CHANGED event is supposed to work and also the bug 1132693 didn't motivate either. I didn't spot that problem with the patch back then but I think we should fix it now.

Comment 1

[Daniel Stenberg [:bagder]]

Attachment: 0001-Bug-1142586-avoid-asking-the-link-service-for-online.patch

Here's what how I suggest nsIOService::OnNetworkLinkEvent() should deal with CHANGED events.

Comment 2

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

Comment on attachment 8576740 [details] [diff] [review]
0001-Bug-1142586-avoid-asking-the-link-service-for-online.patch

That seems fair. I thought it made sense at the time I made the change. I think Android was the only one affected by the previous regression.

Comment 3

[Daniel Stenberg [:bagder]]

Comment on attachment 8576740 [details] [diff] [review]
0001-Bug-1142586-avoid-asking-the-link-service-for-online.patch

Review handed to :valentin with permission from :jduel

Comment 4

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

Comment on attachment 8576740 [details] [diff] [review]
0001-Bug-1142586-avoid-asking-the-link-service-for-online.patch

Looks good.

Comment 5

[Daniel Stenberg [:bagder]]

Thanks Valentin!

The try-run looks good, ignoring the Mac OS X collapse and a buildbust on b2g I can't see is related.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=41fee025efef

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9cbd4a5c9a38

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9cbd4a5c9a38