1307491 - Remove IOService.SetAppOffline

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Jason Duell]

AFAICT this was only used by B2G.  So we should be able to rip out everything we added in bug 786419

Comment 1

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

Attachment: (Part 1) Remove support for per-app-offline in netwerk/ [nukeb2g]

* * *
Bug 1307491 - Remove support for per-app-offline [nukeb2g] r=bagder

Comment 2

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

Attachment: Bug 1307491 - (Part 1) Remove support for per-app-offline in netwerk/ [nukeb2g]

* * *
Bug 1307491 - Remove support for per-app-offline [nukeb2g] r=bagder

Review commit: https://reviewboard.mozilla.org/r/85226/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/85226/

Comment 3

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

Attachment: Bug 1307491 - (Part 2) Remove support for per-app-offline in dom/ [nukeb2g]

Review commit: https://reviewboard.mozilla.org/r/85228/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/85228/

Comment 4

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

Attachment: Bug 1307491 - (Part 3) Remove support for per-app-offline in don/network [nukeb2g]

Review commit: https://reviewboard.mozilla.org/r/85230/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/85230/

Comment 5

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

Attachment: Bug 1307491 - (Part 4) Remove support for per-app-offline in PeerConnection.js [nukeb2g]

Review commit: https://reviewboard.mozilla.org/r/85232/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/85232/

Comment 6

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

Attachment: Bug 1307491 - (Part 5) Remove support for per-app-offline in dom/ipc [nukeb2g]

Review commit: https://reviewboard.mozilla.org/r/85234/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/85234/

Comment 7

[Andrea Marchesini [:baku]]

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...

Comment 8

[Andrea Marchesini [:baku]]

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

Comment 9

[Andrea Marchesini [:baku]]

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

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 11

[Daniel Stenberg [:bagder]]

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! :-)

Comment 12

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

Comment on attachment 8800248 [details]
Bug 1307491 - (Part 1) Remove support for per-app-offline in netwerk/ [nukeb2g]

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/85226/diff/1-2/

Comment 13

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

Comment on attachment 8800249 [details]
Bug 1307491 - (Part 2) Remove support for per-app-offline in dom/ [nukeb2g]

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/85228/diff/1-2/

Comment 14

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

Comment on attachment 8800250 [details]
Bug 1307491 - (Part 3) Remove support for per-app-offline in don/network [nukeb2g]

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/85230/diff/1-2/

Comment 15

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

Comment on attachment 8800251 [details]
Bug 1307491 - (Part 4) Remove support for per-app-offline in PeerConnection.js [nukeb2g]

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/85232/diff/1-2/

Comment 16

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

Comment on attachment 8800252 [details]
Bug 1307491 - (Part 5) Remove support for per-app-offline in dom/ipc [nukeb2g]

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/85234/diff/1-2/

Comment 17

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

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.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
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

Comment 19

[(no longer active)]

Valentin, it seems like two declarations of NS_IsAppOffline in nsNetUtil.h remains after these patches.  :-)

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5c0780a3a6cb
https://hg.mozilla.org/mozilla-central/rev/b0c3f48a9c5d
https://hg.mozilla.org/mozilla-central/rev/fe032494b410
https://hg.mozilla.org/mozilla-central/rev/76085b0fc074
https://hg.mozilla.org/mozilla-central/rev/8168208334e5

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d868cd247b5
follow-up: Remove two remaining declarations of NS_IsAppOffline

Comment 22

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

(In reply to Pulsebot from comment #21)
> Pushed by [e-mail]:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/7d868cd247b5
> follow-up: Remove two remaining declarations of NS_IsAppOffline

Thanks for pushing that for me :)

Comment 23

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7d868cd247b5