All users were logged out of Bugzilla on October 13th, 2018

remove check for built-in self-support

RESOLVED FIXED

Status

()

RESOLVED FIXED
a year ago
9 months ago

People

(Reporter: rhelmer, Assigned: rhelmer)

Tracking

unspecified
Points:
---

Firefox Tracking Flags

(firefox55+ fixed, firefox56 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
In bug 1383338 unfortunately I didn't include the removal of self-support, which is required for that change to be effective.
Comment hidden (mozreview-request)
I understand that we want this in 55 for some new user retention studies.
status-firefox55: --- → affected
status-firefox56: --- → affected
tracking-firefox55: --- → +

Comment 3

a year ago
mozreview-review
Comment on attachment 8891190 [details]
Bug 1385177 - Remove check for built-in self-support from shield recipe client

https://reviewboard.mozilla.org/r/162402/#review167850
Attachment #8891190 - Flags: review?(mcooper) → review+

Comment 4

a year ago
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3b9509ef04d0
Remove check for built-in self-support from shield recipe client r=mythmon
(Assignee)

Comment 5

a year ago
Comment on attachment 8891190 [details]
Bug 1385177 - Remove check for built-in self-support from shield recipe client

Approval Request Comment
[Feature/Bug causing the regression]: followup for bug 1383338
[User impact if declined]: not able to run first-run retention experiments
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: yes, coordinating w/ QA now and steps will be provided in bug 1383338
[List of other uplifts needed for the feature/fix]: just bug 1383338 (already landed on beta)
[Is the change risky?]: no
[Why is the change risky/not risky?]: removes detection for obsolete "self-support" feature which was causing shield to exit early on first-run
[String changes made/needed]: none
Attachment #8891190 - Flags: approval-mozilla-beta?
Backed out on suspicion of causing exceptions in browser-chrome tests and connections in wpt tests:

https://hg.mozilla.org/integration/autoland/rev/50ce68c9412ce80cf4bf4f1dc4be9f3c7fcd0c6d

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3b9509ef04d0c8e87a59aa1b03e4a0beca12f1dc&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable

Failure logs browser-chrome:
https://treeherder.mozilla.org/logviewer.html#?job_id=119045097&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=119044063&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=119044015&repo=autoland
E.g. browser/components/extensions/test/browser/browser_ext_windows_update.js | A promise chain failed to handle a rejection: JSON.parse: unexpected character at line 1 column 1 of the JSON data - stack: null Rejection date: Fri Jul 28 2017 11:03:20 GMT-0700 (PDT) - false == true

Failure log wpt: https://treeherder.mozilla.org/logviewer.html#?job_id=119054649&repo=autoland
FATAL ERROR: Non-local network connections are disabled and a connection attempt to example.com (93.184.216.34) was made.
Flags: needinfo?(rhelmer)
(Assignee)

Comment 7

a year ago
These failures seem really unlikely to be caused by this patch... I'll do a try run.
Flags: needinfo?(rhelmer)
(Assignee)

Comment 8

a year ago
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #6)
> Backed out on suspicion of causing exceptions in browser-chrome tests and
> connections in wpt tests:
> 
> https://hg.mozilla.org/integration/autoland/rev/
> 50ce68c9412ce80cf4bf4f1dc4be9f3c7fcd0c6d
> 
> Push with failures:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=3b9509ef04d0c8e87a59aa1b03e4a0beca12f1dc&filter-
> resultStatus=testfailed&filter-resultStatus=busted&filter-
> resultStatus=exception&filter-resultStatus=retry&filter-
> resultStatus=usercancel&filter-resultStatus=runnable
> 
> Failure logs browser-chrome:
> https://treeherder.mozilla.org/logviewer.html#?job_id=119045097&repo=autoland
> https://treeherder.mozilla.org/logviewer.html#?job_id=119044063&repo=autoland
> https://treeherder.mozilla.org/logviewer.html#?job_id=119044015&repo=autoland
> E.g.
> browser/components/extensions/test/browser/browser_ext_windows_update.js | A
> promise chain failed to handle a rejection: JSON.parse: unexpected character
> at line 1 column 1 of the JSON data - stack: null Rejection date: Fri Jul 28
> 2017 11:03:20 GMT-0700 (PDT) - false == true

I can't reproduce this at all. Maybe we got an unlucky revision from m-c; I'll rebase and do a try run.


> Failure log wpt:
> https://treeherder.mozilla.org/logviewer.html#?job_id=119054649&repo=autoland
> FATAL ERROR: Non-local network connections are disabled and a connection
> attempt to example.com (93.184.216.34) was made.

This appears to be an existing bug - before this patch, shield always exited early on first-run.
Comment hidden (mozreview-request)

Comment 10

a year ago
mozreview-review
Comment on attachment 8891190 [details]
Bug 1385177 - Remove check for built-in self-support from shield recipe client

https://reviewboard.mozilla.org/r/162402/#review167990
(Assignee)

Comment 11

a year ago
Looks like this is a one-line fix to a test config... doing a try run to make sure before re-landing.
(Assignee)

Comment 12

a year ago
(In reply to Robert Helmer [:rhelmer] from comment #8)
> (In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on
> intermittent or backout) from comment #6)
> > Backed out on suspicion of causing exceptions in browser-chrome tests and
> > connections in wpt tests:
> > 
> > https://hg.mozilla.org/integration/autoland/rev/
> > 50ce68c9412ce80cf4bf4f1dc4be9f3c7fcd0c6d
> > 
> > Push with failures:
> > https://treeherder.mozilla.org/#/
> > jobs?repo=autoland&revision=3b9509ef04d0c8e87a59aa1b03e4a0beca12f1dc&filter-
> > resultStatus=testfailed&filter-resultStatus=busted&filter-
> > resultStatus=exception&filter-resultStatus=retry&filter-
> > resultStatus=usercancel&filter-resultStatus=runnable
> > 
> > Failure logs browser-chrome:
> > https://treeherder.mozilla.org/logviewer.html#?job_id=119045097&repo=autoland
> > https://treeherder.mozilla.org/logviewer.html#?job_id=119044063&repo=autoland
> > https://treeherder.mozilla.org/logviewer.html#?job_id=119044015&repo=autoland
> > E.g.
> > browser/components/extensions/test/browser/browser_ext_windows_update.js | A
> > promise chain failed to handle a rejection: JSON.parse: unexpected character
> > at line 1 column 1 of the JSON data - stack: null Rejection date: Fri Jul 28
> > 2017 11:03:20 GMT-0700 (PDT) - false == true
> 
> I can't reproduce this at all. Maybe we got an unlucky revision from m-c;
> I'll rebase and do a try run.

Still can't repro this locally or on try (also I meant to say mozilla-incoming above not m-c)

> > Failure log wpt:
> > https://treeherder.mozilla.org/logviewer.html#?job_id=119054649&repo=autoland
> > FATAL ERROR: Non-local network connections are disabled and a connection
> > attempt to example.com (93.184.216.34) was made.
> 
> This appears to be an existing bug - before this patch, shield always exited
> early on first-run.

With the one-line test config fix, I can't repro this either.

Comment 13

a year ago
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7a3c198ec229
Remove check for built-in self-support from shield recipe client r=mythmon

Comment 14

a year ago
Backout by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/392203e46397
Backed out changeset 7a3c198ec229 for breaking PGO
(Assignee)

Comment 15

a year ago
(In reply to Pulsebot from comment #14)
> Backout by gszorc@mozilla.com:
> https://hg.mozilla.org/integration/autoland/rev/392203e46397
> Backed out changeset 7a3c198ec229 for breaking PGO

Ugh. This is a minor problem in the one-line patch... example.com should be replaced with %(server)s not %(server) in the test config.
Comment hidden (mozreview-request)

Comment 17

a year ago
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0aaf1e3ab6ea
Remove check for built-in self-support from shield recipe client r=mythmon
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 20

a year ago
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7f4229cbc665
Followup to fix bustage a=bustage
Backed out for failing browser-chrome's browser_sanitizeDialog.js and browser_clientAuth_connection.js:

https://hg.mozilla.org/integration/autoland/rev/b9abf4de15297fa10e1ee09c99713a2c6286fd69
https://hg.mozilla.org/integration/autoland/rev/1aa98e99c8792d7d76b0069319c51f8f5004690f

Follow-up push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7f4229cbc66563ced5b3277f58fc30295cfeb685&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable

Failure log sanitize: https://treeherder.mozilla.org/logviewer.html#?job_id=119196158&repo=autoland
> FAIL | browser/base/content/test/general/browser_sanitizeDialog.js | A promise chain failed to handle a rejection: NetworkError when attempting to fetch resource. - stack: open@chrome://mochitests/content/browser/browser/base/content/test/general/browser_sanitizeDialog.js:822:5 test_cannot_clear_history@chrome://mochitests/content/browser/browser/base/content/test/general/browser_sanitizeDialog.js:379:3 Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:798:21 TaskImpl_run@resource://gre/modules/Task.jsm:331:42 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:408:7 TaskImpl_run@resource://gre/modules/Task.jsm:339:15 TaskImpl@resource://gre/modules/Task.jsm:280:3 asyncFunction@resource://gre/modules/Task.jsm:252:14 Task_spawn@resource://gre/modules/Task.jsm:166:12 Tester_execTest@chrome://mochikit/content/browser-test.js:789:9 Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:689:7 SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59 Rejection date: Sat Jul 29 2017 06:36:51 GMT+0000 (Coordinated Universal Time) - false == true 

Failure log auth: https://treeherder.mozilla.org/logviewer.html#?job_id=119196171&repo=autoland
>  FAIL | security/manager/ssl/tests/mochitest/browser/browser_clientAuth_connection.js | A promise chain failed to handle a rejection: NetworkError when attempting to fetch resource. - stack: null Rejection date: Sat Jul 29 2017 06:44:09 GMT+0000 (Coordinated Universal Time) - false == true
Flags: needinfo?(rhelmer)
(Assignee)

Comment 22

a year ago
Thanks. What's going on here is the test config is pointing to the local test server, and a few other tests in the tree aren't expecting to see this connection.

Reviewing the shield tests, they just expect the pref to be present but they actually set it themselves.
Flags: needinfo?(rhelmer)
Comment hidden (mozreview-request)

Comment 24

a year ago
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a15f665d03f4
Remove check for built-in self-support from shield recipe client r=mythmon

Comment 25

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a15f665d03f4
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox56: affected → fixed
Resolution: --- → FIXED
Comment on attachment 8891190 [details]
Bug 1385177 - Remove check for built-in self-support from shield recipe client

shield fix for 55 rc1
Attachment #8891190 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 27

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/66bd0b4c3b50
status-firefox55: affected → fixed

Updated

9 months ago
Product: Shield → Firefox
You need to log in before you can comment on or make changes to this bug.