1520321 - Implement XPCOM interface for interacting with the update agent

Status: RESOLVED FIXED

(Toolkit :: Application Update, enhancement, P1)

Description

[Robin Steuber (they/them) [:bytesized]]

Update logic is done in Javascript (largely in nsUpdateService.js); we don't want to move or split that logic. Therefore, in order to enable updating via BITS (the Windows Background Intelligent Transfer Service), there needs to be XPCOM interface exposing that functionality to Javascript.

Furthermore, we have future plans to run these BITS downloads as the Local Service user. Firefox will only be able to start BITS downloads directly as the current user. Therefore, we want the Update Agent (which will have the ability to start BITS downloads as Local Service) to do the BITS download task creation.

Therefore, we want the XPCOM interface to interact with the Update Agent, rather than with BITS directly.

Comment 1

[Robin Steuber (they/them) [:bytesized]]

Let me give a bit of clarification of the capabilities of this interface. The Update Agent will have capabilities for autonomously downloading and installing updates, and additionally, a BitsClient interface which will essentially function only as a conduit for interaction with BITS.

The XPCOM interface is meant to allow Javascript to interact with the BitsClient interface only. It will not provide the ability to control other aspects of the Update Agent.

Comment 2

[Robin Steuber (they/them) [:bytesized]]

Myk-

The XPCOM interface that I am working on for this is written in Rust. Naturally, I have been following your work in Bug 1490496 since it looks likely to be the first XPCOM interface implemented in Rust to be checked into the tree. I have, in fact, been building parts of my implementation on top of yours. It currently uses much of moz_task as well as the xpcom_method macro.

We are planning to ship this feature in Firefox 67. Do you think that Bug 1490496 will have merged by then? If not, maybe we could discuss merging the features of the patch that I am using earlier?

Thanks!

Comment 3

[Myk Melez [:myk] [@mykmelez]]

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #2)

We are planning to ship this feature in Firefox 67. Do you think that Bug 1490496 will have merged by then? If not, maybe we could discuss merging the features of the patch that I am using earlier?

Yes, I think that patch will merge early in the Firefox 67 cycle (which begins on Monday, January 28, if I understand correctly). It already has r+ from Nika and Lina, and there's only one more outstanding issue to address, for which Nika has just suggested a solution.

If for any reason that doesn't happen, however, then I'm happy to merge features of the patch sooner. Let's touch base the first week of the Firefox 67 cycle!

Comment 4

[Myk Melez [:myk] [@mykmelez]]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #3)

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #2)

We are planning to ship this feature in Firefox 67. Do you think that Bug 1490496 will have merged by then? If not, maybe we could discuss merging the features of the patch that I am using earlier?

Yes, I think that patch will merge early in the Firefox 67 cycle (which begins on Monday, January 28, if I understand correctly). It already has r+ from Nika and Lina, and there's only one more outstanding issue to address, for which Nika has just suggested a solution.

If for any reason that doesn't happen, however, then I'm happy to merge features of the patch sooner. Let's touch base the first week of the Firefox 67 cycle!

Update: I did resolve that last outstanding issue with bug 1490496 via the fix that Nika suggested, but it turned out that there are also three build issues that prevent landing of the patch: bug 1522614, bug 1522609, and bug 1512541.

Those are all well-owned by glandium, who has landed some patches already and has review on others. But I'm unsure of how long it'll take to resolve them all. So if bug 1490496 is becoming a bottleneck for you, then let me know which parts of my patch you're using, and I'll create another patch to land them separately.

Comment 5

[Robin Steuber (they/them) [:bytesized]]

At this point, it looks like we are targeting Firefox 68 rather than Firefox 67, so perhaps that will give enough time for those to be merged?

Glandium, will Bug 1522614, Bug 1522609, and Bug 1512541 be merged in time for Firefox 68? Or should Myk and I look into splitting the patch for Bug 1490496 to allow some of it to land earlier?

Comment 6

[Mike Hommey [:glandium]]

Bug 1522614 is landed. Bug 1522609 is in the landing queue. Bug 1512541 is not far off.

Comment 7

[Robin Steuber (they/them) [:bytesized]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e804889e3a3e471d1d6539af58a809fcf00deba

Comment 8

[Robin Steuber (they/them) [:bytesized]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=48afd2b0e6e3ff751fb48822e4c8dd7cea827dc1

Comment 9

[Robin Steuber (they/them) [:bytesized]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a32034287166fda4e099240fbd3c48931825f5c3

Comment 10

[Robin Steuber (they/them) [:bytesized]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0239f9d0c4d363e900186d8cfce257d83f508cfe

Comment 11

[Robin Steuber (they/them) [:bytesized]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c9bbf928b46cdbf3b51883718e8ae5325e68ea21

Comment 12

[Robin Steuber (they/them) [:bytesized]]

https://hg.mozilla.org/projects/oak/rev/41a355267e3aa17ca363dac20071918dd7e4d865
Bug 1520321 - Implement XPCOM interface for BITS

https://hg.mozilla.org/projects/oak/rev/331e2f5632f43482ec563f9bd674766dba1f4a99
Bug 1520321 - Use BITS in nsUpdateService

Comment 13

[Robin Steuber (they/them) [:bytesized]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b502c9a8acae4e2412537f3968ee027b9f4b124d

Comment 14

[Intermittent Failures Robot]

7 failures in 2981 pushes (0.002 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• oak: 7

Platform breakdown:

• macosx64-nightly: 1
• windows10-64-nightly: 2
• linux32-nightly: 1
• linux64-nightly: 1
• windows7-32-nightly: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1520321&startday=2019-02-25&endday=2019-03-03&tree=all

Comment 15

[Jim Mathies [:jimm]]

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #12)

https://hg.mozilla.org/projects/oak/rev/
41a355267e3aa17ca363dac20071918dd7e4d865
Bug 1520321 - Implement XPCOM interface for BITS

Hey Nathan, we're looking for a reviewer for this bit of RUST/XPCOM code. We figured you might be good, but we're open to alternative suggestions. :)

Comment 16

[Nathan Froyd [:froydnj]]

(In reply to Jim Mathies [:jimm] from comment #15)

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #12)

https://hg.mozilla.org/projects/oak/rev/
41a355267e3aa17ca363dac20071918dd7e4d865
Bug 1520321 - Implement XPCOM interface for BITS

Hey Nathan, we're looking for a reviewer for this bit of RUST/XPCOM code. We figured you might be good, but we're open to alternative suggestions. :)

I can certainly look at it, but we have other people who have been more active in Rust/XPCOM integration than I. Nika, Lina, or Myk all seem like potential candidates.

Comment 17

[Robin Steuber (they/them) [:bytesized]]

Attachment: Bug 1520321 - Implement XPCOM interface for BITS

This patch introduces an asynchronous XPCOM interface for the bits client added in Bug 1523417

Comment 18

[Robin Steuber (they/them) [:bytesized]]

Attachment: Bug 1520321 - Use BITS in nsUpdateService

nsUpdateService should use BITS for download. If the BITS download fails, it will fallback to the existing download mechanism (nsIIncrementalDownload).

Depends on D25161

Comment 19

[Robin Steuber (they/them) [:bytesized]]

Attachment: Bug 1520321 - Adds Telemetry for BITS update downloads

These Histograms were added:
UPDATE_CAN_USE_BITS_EXTERNAL
UPDATE_CAN_USE_BITS_NOTIFY
Used for telemetry indicating whether or not BITS can be used by this system. If BITS cannot be used, the probe will contain data indicating why not.
UPDATE_BITS_RESULT_COMPLETE
UPDATE_BITS_RESULT_PARTIAL
Used for telemetry indicating whether the BITS update download succeeded. If it failed, the probe will contain data indicating how it failed.

This scalar was added:
update.bitshresult
Used to indicate the hresult returned, if any, when a BITS download fails.

Depends on D25162

Comment 20

[Robin Steuber (they/them) [:bytesized]]

Attachment: Data-Review Request form

Comment 21

[Chris H-C :chutten]

Comment on attachment 9055587 [details]
Data-Review Request form

Passing to :tdsmith as I helped review the code.

Comment 22

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

Comment on attachment 9055587 [details]
Data-Review Request form

data-review+

1) Is there or will there be **documentation** that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes; the histograms are documented in Histograms.json and the scalar is documented in Scalars.yaml.

2) Is there a control mechanism that allows the user to turn the data collection on and off?

Yes; telemetry opt-out.

3) If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes; Robert Strong.

4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, technical data.

5) Is the data collection request for default-on or default-off?

Default-on.

6) Does the instrumentation include the addition of **any *new* identifiers**?

No.

7) Is the data collection covered by the existing Firefox privacy notice?

Yes.

8) Does there need to be a check-in in the future to determine whether to renew the data?

No, permanent collection.

9) Does the data collection use a third-party collection tool?

No.

Comment 23

[Robert Strong (they/them - no direct email)]

I just did a couple of manual tests using a build from
https://archive.mozilla.org/pub/firefox/nightly/2019/04/2019-04-03-23-58-37-oak/

I replaced the crashreporter.exe in the oak install with one from m-c to cause the partial update to fail.

I noticed the following minor issue in the browser console.
AUS:SVC handleFallbackToCompleteUpdate - install of partial patch failed, downloading complete patch

There was a major issue in that the even though the partial was then downloaded using nsIIncrementalDownload when the partial failed again it didn't try to download the complete update using BITS. The about dialog continued to display "Applying" until I closed it and reopened it again. Also, the xml contained some information for the complete patch that should have been for the partial patch.
EDIT bytesized pointed out that the xml doesn't contain "some information for the complete patch that should have been for the partial patch".

<?xml version="1.0"?><updates xmlns="http://www.mozilla.org/2005/app-update"><update xmlns="http://www.mozilla.org/2005/app-update" appVersion="68.0a1" buildID="20190404220925" channel="nightly-oak" detailsURL="https://www.mozilla.org/en-US/firefox/nightly/notes/" displayVersion="68.0a1" installDate="1554438897830" isCompleteUpdate="false" name="Nightly 68.0a1" previousAppVersion="68.0a1" promptWaitTime="43200" serviceURL="https://aus5.mozilla.org/update/6/Firefox/68.0a1/20190403235837/WINNT_x86_64-msvc-x64/en-US/nightly-oak/Windows_NT%2010.0.0.0.17763.379%20(x64)/ISET:SSE4_2,MEM:32620/default/default/update.xml?force=1" type="minor" statusText="Install Pending" platformVersion="68.0a1" foregroundDownload="true" patchingFailed="partial" stagingFailed="true"><patch size="51578648" type="complete" URL="https://archive.mozilla.org/pub/firefox/nightly/2019/04/2019-04-04-22-09-25-oak/firefox-68.0a1.en-US.win64.complete.mar" hashFunction="sha512" hashValue="6d5f3040a80a7276b8dd53f1050e1ed9d26921d59dba4e81a5590b9f48f005b9f5174e3cd0710b50a503032be6ae763750faa09146d251d4331d97616db9155e"/><patch size="10751771" type="partial" URL="https://archive.mozilla.org/pub/firefox/nightly/partials/2019/04/2019-04-04-22-09-25-oak/firefox-oak-68.0a1-win64-en-US-20190403235837-20190404220925.partial.mar" errorCode="2" finalURL="https://archive.mozilla.org/pub/firefox/nightly/partials/2019/04/2019-04-04-22-09-25-oak/firefox-oak-68.0a1-win64-en-US-20190403235837-20190404220925.partial.mar" selected="true" state="pending-service" hashFunction="sha512" hashValue="f3339c186d25b8089c49a9ff3c7d8c176bec9c7f99ffc69487c75315dd9181105cab1fc585b1f2c10e5a520e1f25cc4945b49f65252d383db24ca3c1e8425b69" bitsId="{8E9F0DFF-1A67-4590-91B6-22BFEB3F65C1}" bitsResult="0" nonbitsResult="0"/></update>

I'll attach the browser console log as well

Comment 24

[Robert Strong (they/them - no direct email)]

Attachment: browser console log

Comment 25

[Jim Mathies [:jimm]]

Hey Lena, review ping. We're trying to get this into 68 early for testing. Would appreciate you taking time to get to the reviews here. Thanks!

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a1cbd2f6e5bc
Implement XPCOM interface for BITS r=lina
https://hg.mozilla.org/integration/autoland/rev/84cc6e41ba17
Use BITS in nsUpdateService r=rstrong
https://hg.mozilla.org/integration/autoland/rev/fb9ff7baed43
Adds Telemetry for BITS update downloads r=chutten,rstrong

Comment 27

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/a1cbd2f6e5bc
https://hg.mozilla.org/mozilla-central/rev/84cc6e41ba17
https://hg.mozilla.org/mozilla-central/rev/fb9ff7baed43

Comment 28

[Robert Strong (they/them - no direct email)]

I have over half of the update download tests working and pushed it to try. The Win debug build reported the following leak which occurred after running BITS download tests. The tests are the exact same ones that are used when downloading with nsIIncrementalDownload so the leaks are most likely due to this bug.

04:01:14 INFO - leakcheck | Processing log file c:\users\task_1555987818\appdata\local\temp\tmpqosti0.mozrunner\runtests_leaks.log
04:01:14 INFO - TEST-INFO | leakcheck | default process: leak threshold set at 0 bytes
04:01:14 INFO - TEST-INFO | leakcheck | plugin process: leak threshold set at 0 bytes
04:01:14 INFO - TEST-INFO | leakcheck | tab process: leak threshold set at 1000 bytes
04:01:14 INFO - TEST-INFO | leakcheck | gmplugin process: leak threshold set at 20000 bytes
04:01:14 INFO - TEST-INFO | leakcheck | gpu process: leak threshold set at 0 bytes
04:01:14 INFO - TEST-INFO | leakcheck | rdd process: leak threshold set at 400 bytes
04:01:14 INFO - TEST-INFO | leakcheck | vr process: leak threshold set at 0 bytes
04:01:14 INFO - TEST-INFO | leakcheck | socket process: leak threshold set at 0 bytes
04:01:14 INFO - leakcheck | Processing leak log file c:\users\task_1555987818\appdata\local\temp\tmpqosti0.mozrunner\runtests_leaks.log
04:01:14 INFO -
04:01:14 INFO - == BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, default process 10408
04:01:14 INFO -
04:01:14 INFO - |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
04:01:14 INFO - | | Per-Inst Leaked| Total Rem|
04:01:14 INFO - 0 |TOTAL | 35 18280|23611574 267|
04:01:14 INFO - 67 |BackstagePass | 128 128| 6 1|
04:01:14 INFO - 161 |CondVar | 64 64| 598 1|
04:01:14 INFO - 432 |IdlePeriod | 24 24| 1 1|
04:01:14 INFO - 541 |Mutex | 80 1520| 6862 19|
04:01:14 INFO - 987 |ThreadEventTarget | 48 48| 221 1|
04:01:14 INFO - 992 |ThreadTargetSink | 16 16| 221 1|
04:01:14 INFO - 1149 |XPCNativeInterface | 56 336| 534 6|
04:01:14 INFO - 1150 |XPCNativeMember | 16 96| 17621 6|
04:01:14 INFO - 1151 |XPCNativeSet | 32 128| 1518 4|
04:01:14 INFO - 1153 |XPCWrappedNative | 96 4704| 12290 49|
04:01:14 INFO - 1154 |XPCWrappedNativeProto | 40 40| 2071 1|
04:01:14 INFO - 1156 |XPCWrappedNativeTearOff | 32 3296| 14629 103|
04:01:14 INFO - 1605 |nsJSPrincipals | 24 24| 3758 1|
04:01:14 INFO - 1744 |nsStringBuffer | 12 24| 218674 2|
04:01:14 INFO - 1789 |nsTArray_base | 8 32| 8158246 4|
04:01:14 INFO - 1798 |nsThread | 264 264| 132 1|
04:01:14 INFO - 1805 |nsTimer | 32 576| 2189 18|
04:01:14 INFO - 1807 |nsTimerImpl | 200 3600| 2189 18|
04:01:14 INFO - 1889 |nsXPCWrappedJS | 112 3360| 4732 30|
04:01:14 INFO -
04:01:14 INFO - nsTraceRefcnt::DumpStatistics: 1932 entries
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 1 BackstagePass
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 1 CondVar
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 1 IdlePeriod
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 19 Mutex
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 1 ThreadEventTarget
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 1 ThreadTargetSink
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 6 XPCNativeInterface
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 6 XPCNativeMember
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 4 XPCNativeSet
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 49 XPCWrappedNative
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 1 XPCWrappedNativeProto
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 103 XPCWrappedNativeTearOff
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 1 nsJSPrincipals
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 2 nsStringBuffer
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 4 nsTArray_base
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 1 nsThread
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 18 nsTimer
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 18 nsTimerImpl
04:01:14 INFO - TEST-INFO | leakcheck | default leaked 30 nsXPCWrappedJS
04:01:14 INFO - TEST-UNEXPECTED-FAIL | leakcheck | default 18280 bytes leaked (BackstagePass, CondVar, IdlePeriod, Mutex, ThreadEventTarget, ...)

Comment 29

[Robert Strong (they/them - no direct email)]

Log file
https://queue.taskcluster.net/v1/task/ZW00sVHcSd62lV-iDdC_BQ/runs/0/artifacts/public/logs/live_backing.log

Comment 30

[Robert Strong (they/them - no direct email)]

Also happened in a different try run
https://queue.taskcluster.net/v1/task/FuQPafwVS2KPrboUEX53FA/runs/0/artifacts/public/logs/live_backing.log

Comment 31

[Robert Strong (they/them - no direct email)]

and another on Win 7
https://queue.taskcluster.net/v1/task/RiEoT9Q7R2u0zbK6CLn0gQ/runs/0/artifacts/public/logs/live_backing.log

Comment 32

[Robert Strong (they/them - no direct email)]

Filed bug 1546287 for the leak