Network CHANGED should not imply polling for UP/DOWN status

RESOLVED FIXED in Firefox 39

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bagder, Assigned: bagder)

Tracking

Trunk
mozilla39
Points:
---

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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.
(Assignee)

Comment 1

4 years ago
Created attachment 8576740 [details] [diff] [review]
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 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.
Attachment #8576740 - Flags: feedback+
(Assignee)

Comment 3

4 years ago
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
Attachment #8576740 - Flags: review?(valentin.gosu)
Comment on attachment 8576740 [details] [diff] [review]
0001-Bug-1142586-avoid-asking-the-link-service-for-online.patch

Looks good.
Attachment #8576740 - Flags: review?(valentin.gosu) → review+
(Assignee)

Comment 5

4 years ago
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9cbd4a5c9a38
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.