1730481 - Remove workaround for localhost browser chrome test as added by bug 1553849

Status: RESOLVED FIXED

(Remote Protocol :: CDP, enhancement, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Follow-up from bug 1553849, which added workarounds for browser chrome tests to check offline mode also for localhost. With bug 1634246 fixed these are no longer necessary anymore.

Comment 1

[Etienne Bruines]

Attachment: Bug 1730481 - [remote] Remove workaround for localhost CDP browser chrome tests

Since Bug 1634246 got resolved, connections to localhost can be
blocked when in offline mode. This commit now makes use of that
in the testing of the offline-part of emulateNetworkConditions.

Comment 2

[Etienne Bruines]

I reuploaded a new commit / with a fresh commit message, so https://phabricator.services.mozilla.com/D125040 is no longer needed.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d025ff107cc
[remote] Remove workaround for localhost CDP browser chrome tests r=whimboo,webdriver-reviewers

Comment 4

[Cristian Tuns]

Backed out for causing mochitest-remote failures on browser_emulateNetworkConditions.js.

• Backout link
• Push with failures
• Failure Log

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

As it looks like the failures only happen on these builds which use networking on the socket channel:

https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Cusercancel&revision=2d025ff107cc08664a90e3da1640aee9e9b3c304&searchStr=remote%2Cmochi

Does that specific feature as added via bug 1634246 require a dedicated patch? Nihanth and Valentin, could you please help?

Comment 6

[(inactive) Nihanth Subramanya [:nhnt11]]

Kershaw, do you know if the patch in bug 1634246 requires a follow-up for socket process?

Comment 7

[Kershaw Chang [:kershaw]]

(In reply to Nihanth Subramanya [:nhnt11] from comment #6)

Kershaw, do you know if the patch in bug 1634246 requires a follow-up for socket process?

I think we need to let socket process know that the pref network.disable-localhost-when-offline is changed. To do this, we need to add network.disable-localhost-when-offline in this array.
I'll file another bug to do this.

Comment 8

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

Since the tests added in bug 1634246 don't fail for socket process, we should probably see if they're running correctly on the socket process.

Comment 9

[Kershaw Chang [:kershaw]]

With the patch in bug 1730983, the test in this bug is not failed.
https://treeherder.mozilla.org/jobs?repo=try&revision=a188d2ed9fcc3e9344d8d702d4424a625f7bb443&selectedTaskRun=NXb7bumJSACpW1oaF0ySDQ.0

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Great news. Thanks a lot for fixing it! We will push once that mentioned bug has been reached mozilla-central.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f60b7786873
[remote] Remove workaround for localhost CDP browser chrome tests r=whimboo,webdriver-reviewers

Comment 12

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/8f60b7786873