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)

defect

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).
Alessio, is that something that needs a look soon?
Flags: needinfo?(alessio.placitelli)
I forgot to mention that at step 4 I did't see the "updated" page.
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)
(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)
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)
(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)
Priority: -- → P1
(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)
(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
Madalin, would you kindly check if the attached documentation change looks ok?
Flags: needinfo?(madalin.cotetiu)
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+
Comment on attachment 8907655 [details]
Bug 1398811 - Document the edge cases for the 'update' ping.

https://reviewboard.mozilla.org/r/179324/#review184864

r+
Flags: needinfo?(madalin.cotetiu)
Pushed by alessio.placitelli@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/680492068697
Document the edge cases for the 'update' ping. r=chutten
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.