Closed
Bug 1439777
Opened 7 years ago
Closed 7 years ago
Simplify sync's ErrorHandler
Categories
(Firefox :: Sync, enhancement, P1)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
Firefox 60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: tcsc, Assigned: eoger)
Details
Attachments
(2 files)
It's a somewhat complex piece of code (with very complex tests) that is primarially concerned with whether or not we should be displaying errors in the UI. Since we never do this anymore, it should be simplified.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → eoger
Status: NEW → ASSIGNED
Priority: -- → P1
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years ago
|
||
Note that we will also need to update the about:sync addon which relies on weave:ui:* [0]
[0] https://github.com/mhammond/aboutsync/blob/77ef1595df68ed71d7f55ae68018d650061ca3ed/bootstrap.js#L178-L179
Reporter | ||
Comment 4•7 years ago
|
||
We could instead consider emitting weave:ui:*:error events when we'd otherwise write a sync error log. I spent some time looking into how to convert about:sync to not depend on those events, and the best I could come up with (without changing the sync code) was to listen for when we wrote an log file, and see if doing so caused a new file with error in its name to appear in our sync log directory.
A simpler solution would be to let about:sync know here https://searchfox.org/mozilla-central/source/services/sync/modules/policies.js#854 somehow (using weave:ui:* would be backwards compatible, admittedly the name is bad)
Reporter | ||
Comment 5•7 years ago
|
||
(FWIW, reporting an error in this case will catch cases where an error is logged outside of a sync -- at the moment we completely miss those)
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8952983 [details]
Bug 1439777 p2 - Remove weave:ui:* related code.
https://reviewboard.mozilla.org/r/222246/#review228490
nice cleanup, thanks!
Attachment #8952983 -
Flags: review?(markh) → review+
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8952982 [details]
Bug 1439777 p1 - Remove X-Weave-Alert support.
https://reviewboard.mozilla.org/r/222244/#review228502
Attachment #8952982 -
Flags: review?(markh) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•7 years ago
|
||
Reporter | ||
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8952983 [details]
Bug 1439777 p2 - Remove weave:ui:* related code.
https://reviewboard.mozilla.org/r/222246/#review229398
It's great to see this gone.
Attachment #8952983 -
Flags: review?(tchiovoloni) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 16•7 years ago
|
||
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8856efbc78a7
p1 - Remove X-Weave-Alert support. r=markh
https://hg.mozilla.org/integration/autoland/rev/6e83f07e8f2f
p2 - Remove weave:ui:* related code. r=markh,tcsc
Comment 17•7 years ago
|
||
Backed out 2 changesets (bug 1439777) for browser chrome in browser/components/customizableui/test/browser_remote_tabs_button.js and xpc-shell failures in services/sync/tests/unit/test_errorhandler_2.js on a CLOSED TREE
https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=166018817
https://hg.mozilla.org/integration/autoland/rev/f00eccec1c8378c6468215ccfe1cd3d8a252b640
Flags: needinfo?(eoger)
Comment 18•7 years ago
|
||
Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/25507e523aec
Backed out 2 changesets for browser chrome in browser/components/customizableui/test/browser_remote_tabs_button.js and xpc-shell failures in services/sync/tests/unit/test_errorhandler_2.js on a CLOSED TREE
Assignee | ||
Comment 19•7 years ago
|
||
Flags: needinfo?(eoger)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 22•7 years ago
|
||
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1af3426dc956
p1 - Remove X-Weave-Alert support. r=markh
https://hg.mozilla.org/integration/autoland/rev/d99402ad9ecc
p2 - Remove weave:ui:* related code. r=markh,tcsc
Comment 23•7 years ago
|
||
Backout by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fea7978f3af6
Backed out 2 changesets for xpcshell failures on /test_errorhandler_2.js
Assignee | ||
Comment 24•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 27•7 years ago
|
||
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/76c0ebfa6f08
p1 - Remove X-Weave-Alert support. r=markh
https://hg.mozilla.org/integration/autoland/rev/157f93b585b1
p2 - Remove weave:ui:* related code. r=markh,tcsc
Comment 28•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/76c0ebfa6f08
https://hg.mozilla.org/mozilla-central/rev/157f93b585b1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in
before you can comment on or make changes to this bug.
Description
•