Closed
Bug 1398811
Opened 7 years ago
Closed 7 years ago
The update ping is not sent after a newer Nightly version is installed over a previous older one
Categories
(Toolkit :: Telemetry, defect, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: madalin.cotetiu, Assigned: Dexter)
References
Details
Attachments
(1 file)
Steps to reproduce: 1. Open nightly (using a build that is not the latest nightly build) 2. Create a new profile and close nightly 3. Download the latest nightly build and install it 4. Open nightly with the profile used in step 2 Expected results: The update ping is sent. Actual results: The update ping is not sent. Notes/Issues: Verified in FF57 Win7(x64)(build id: 20170903220032).
Comment 1•7 years ago
|
||
Alessio, is that something that needs a look soon?
Flags: needinfo?(alessio.placitelli)
Reporter | ||
Comment 2•7 years ago
|
||
I forgot to mention that at step 4 I did't see the "updated" page.
Assignee | ||
Comment 3•7 years ago
|
||
Hey Matt, looks like this code [1] isn't getting hit using the STR at comment 0. This should be a pave-over install, that is an install performed on the same profile (same channel) to update Firefox. Is [1] supposed to detect that? If not, how can this situation be detected? [1] - http://searchfox.org/mozilla-central/rev/51eae084534f522a502e4b808484cde8591cb502/browser/components/nsBrowserContentHandler.js#521,529
Flags: needinfo?(alessio.placitelli) → needinfo?(mhowell)
Comment 4•7 years ago
|
||
(In reply to Alessio Placitelli [:Dexter] from comment #3) > Hey Matt, looks like this code [1] isn't getting hit using the STR at > comment 0. This should be a pave-over install, that is an install performed > on the same profile (same channel) to update Firefox. Is [1] supposed to > detect that? If not, how can this situation be detected? The update service doesn't make any effort to detect paveovers, which means the postupdate pref doesn't get set; it's not really something that app update typically needs to worry about. Profiles do record which version they were used with last, and that does get checked, but that's done very very early [0] and for specific reasons (seemingly just invalidating the startup cache), then the file that's involved is immediately rewritten [1]. [0] http://searchfox.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4201 [1] http://searchfox.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4245
Flags: needinfo?(mhowell)
Comment 5•7 years ago
|
||
Is that an edge case to be documented? Or do the users of the update ping data need to monitor this scenario?
Flags: needinfo?(alessio.placitelli)
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Matt Howell [:mhowell] from comment #4) > (In reply to Alessio Placitelli [:Dexter] from comment #3) > > Hey Matt, looks like this code [1] isn't getting hit using the STR at > > comment 0. This should be a pave-over install, that is an install performed > > on the same profile (same channel) to update Firefox. Is [1] supposed to > > detect that? If not, how can this situation be detected? > > The update service doesn't make any effort to detect paveovers, which means > the postupdate pref doesn't get set; it's not really something that app > update typically needs to worry about. Profiles do record which version they > were used with last, and that does get checked, but that's done very very > early [0] and for specific reasons (seemingly just invalidating the startup > cache), then the file that's involved is immediately rewritten [1]. > > [0] > http://searchfox.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4201 > [1] > http://searchfox.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4245 Thanks for the great answer Matt! @Ben, can document this as an edge case (user updating through re-installs) or do you need to monitor this scenario?
Flags: needinfo?(alessio.placitelli) → needinfo?(bhearsum)
Assignee | ||
Updated•7 years ago
|
Priority: -- → P1
Comment 7•7 years ago
|
||
(In reply to Alessio Placitelli [:Dexter] from comment #6) > (In reply to Matt Howell [:mhowell] from comment #4) > > (In reply to Alessio Placitelli [:Dexter] from comment #3) > > > Hey Matt, looks like this code [1] isn't getting hit using the STR at > > > comment 0. This should be a pave-over install, that is an install performed > > > on the same profile (same channel) to update Firefox. Is [1] supposed to > > > detect that? If not, how can this situation be detected? > > > > The update service doesn't make any effort to detect paveovers, which means > > the postupdate pref doesn't get set; it's not really something that app > > update typically needs to worry about. Profiles do record which version they > > were used with last, and that does get checked, but that's done very very > > early [0] and for specific reasons (seemingly just invalidating the startup > > cache), then the file that's involved is immediately rewritten [1]. > > > > [0] > > http://searchfox.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4201 > > [1] > > http://searchfox.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4245 > > Thanks for the great answer Matt! > > @Ben, can document this as an edge case (user updating through re-installs) > or do you need to monitor this scenario? I think it's fine to document this as an edge case.
Flags: needinfo?(bhearsum)
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Ben Hearsum (:bhearsum) from comment #7) > > @Ben, can document this as an edge case (user updating through re-installs) > > or do you need to monitor this scenario? > > I think it's fine to document this as an edge case. Thanks Ben! Let's also use this bug to document the edge cases from bug 1397322 and bug 1397765 as well.
Assignee: nobody → alessio.placitelli
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•7 years ago
|
||
Madalin, would you kindly check if the attached documentation change looks ok?
Flags: needinfo?(madalin.cotetiu)
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8907655 [details] Bug 1398811 - Document the edge cases for the 'update' ping. https://reviewboard.mozilla.org/r/179324/#review184510 Looks green to me. ::: toolkit/components/telemetry/docs/data/update-ping.rst:32 (Diff revision 1) > payload.reason > -------------- > This field supports the following values: > > - ``ready`` meaning that the ping was generated after an update was downloaded and marked as ready to be processed. For *non-staged* updates this happens as soon as the download finishes and is verified while for *staged* updates this happens before the staging step is started. > -- ``ready`` the ping was generated after the browser was restarted and the update correctly applied. > +- ``success`` the ping was generated after the browser was restarted and the update correctly applied. whoops!
Attachment #8907655 -
Flags: review?(chutten) → review+
Reporter | ||
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8907655 [details] Bug 1398811 - Document the edge cases for the 'update' ping. https://reviewboard.mozilla.org/r/179324/#review184864 r+
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(madalin.cotetiu)
Comment 13•7 years ago
|
||
Pushed by alessio.placitelli@gmail.com: https://hg.mozilla.org/integration/autoland/rev/680492068697 Document the edge cases for the 'update' ping. r=chutten
Updated•7 years ago
|
status-firefox57:
--- → fix-optional
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/680492068697
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•