Closed Bug 1307491 Opened 3 years ago Closed 3 years ago

Remove IOService.SetAppOffline

Categories

(Core :: Networking, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: jduell.mcbugs, Assigned: valentin)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-active])

Attachments

(5 files, 1 obsolete file)

AFAICT this was only used by B2G.  So we should be able to rip out everything we added in bug 786419
Whiteboard: [necko-next]
* * *
Bug 1307491 - Remove support for per-app-offline [nukeb2g] r=bagder
Attachment #8800245 - Flags: review?(daniel)
Attachment #8800245 - Attachment is obsolete: true
Attachment #8800245 - Flags: review?(daniel)
Whiteboard: [necko-next] → [necko-active]
Comment on attachment 8800249 [details]
Bug 1307491 - (Part 2) Remove support for per-app-offline in dom/ [nukeb2g]

https://reviewboard.mozilla.org/r/85228/#review83792

::: dom/base/nsGlobalWindow.cpp:10786
(Diff revision 1)
>    // Don't fire an event if the status hasn't changed
>    if (mWasOffline == isOffline) {
>      return;
>    }
>  
>    mWasOffline = isOffline;

This is totally correct. But what about:

if (mWasOffline == NS_IsOffline()) {
  return;
}

mWasOffline = !mWasOffline;

::: dom/workers/RuntimeService.cpp
(Diff revision 1)
>    }
>    if (!strcmp(aTopic, NS_IOSERVICE_OFFLINE_STATUS_TOPIC)) {
>      SendOfflineStatusChangeEventToAllWorkers(NS_IsOffline());
>      return NS_OK;
>    }
> -  if (!strcmp(aTopic, NS_IOSERVICE_APP_OFFLINE_STATUS_TOPIC)) {

Funny, we don't have any AddObserver/RemoveObserver about this...
Attachment #8800249 - Flags: review?(amarchesini) → review+
Comment on attachment 8800250 [details]
Bug 1307491 - (Part 3) Remove support for per-app-offline in don/network [nukeb2g]

https://reviewboard.mozilla.org/r/85230/#review83796
Attachment #8800250 - Flags: review?(amarchesini) → review+
Comment on attachment 8800252 [details]
Bug 1307491 - (Part 5) Remove support for per-app-offline in dom/ipc [nukeb2g]

https://reviewboard.mozilla.org/r/85234/#review83798
Attachment #8800252 - Flags: review?(amarchesini) → review+
Comment on attachment 8800251 [details]
Bug 1307491 - (Part 4) Remove support for per-app-offline in PeerConnection.js [nukeb2g]

https://reviewboard.mozilla.org/r/85232/#review83924
Attachment #8800251 - Flags: review?(rjesup) → review+
Comment on attachment 8800248 [details]
Bug 1307491 - (Part 1) Remove support for per-app-offline in netwerk/ [nukeb2g]

https://reviewboard.mozilla.org/r/85226/#review84976

Wow, seeing this much code get removed in one blow makes me all warm and fuzzy inside! :-)
Attachment #8800248 - Flags: review?(daniel) → review+
Comment on attachment 8800249 [details]
Bug 1307491 - (Part 2) Remove support for per-app-offline in dom/ [nukeb2g]

https://reviewboard.mozilla.org/r/85228/#review83792

> Funny, we don't have any AddObserver/RemoveObserver about this...

I think it was removed in another recent bug.
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5c0780a3a6cb
(Part 1) Remove support for per-app-offline in netwerk/ [nukeb2g] r=bagder
https://hg.mozilla.org/integration/autoland/rev/b0c3f48a9c5d
(Part 2) Remove support for per-app-offline in dom/ [nukeb2g] r=baku
https://hg.mozilla.org/integration/autoland/rev/fe032494b410
(Part 3) Remove support for per-app-offline in don/network [nukeb2g] r=baku
https://hg.mozilla.org/integration/autoland/rev/76085b0fc074
(Part 4) Remove support for per-app-offline in PeerConnection.js [nukeb2g] r=jesup
https://hg.mozilla.org/integration/autoland/rev/8168208334e5
(Part 5) Remove support for per-app-offline in dom/ipc [nukeb2g] r=baku
Valentin, it seems like two declarations of NS_IsAppOffline in nsNetUtil.h remains after these patches.  :-)
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d868cd247b5
follow-up: Remove two remaining declarations of NS_IsAppOffline
(In reply to Pulsebot from comment #21)
> Pushed by eakhgari@mozilla.com:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/7d868cd247b5
> follow-up: Remove two remaining declarations of NS_IsAppOffline

Thanks for pushing that for me :)
No longer blocks: 1369194
You need to log in before you can comment on or make changes to this bug.