Implement XPCOM interface for interacting with the update agent

RESOLVED FIXED in Firefox 68

Status

()

enhancement
P1
normal
RESOLVED FIXED
5 months ago
2 months ago

People

(Reporter: bytesized, Assigned: bytesized)

Tracking

(Depends on 2 bugs, Blocks 1 bug)

unspecified
mozilla68
Unspecified
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(5 attachments)

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.

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.

Depends on: 1490496

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!

Flags: needinfo?(myk)

(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!

Flags: needinfo?(myk)
Assignee

Updated

4 months ago
Depends on: 1525158

(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.

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?

Flags: needinfo?(mh+mozilla)

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

Flags: needinfo?(mh+mozilla)
Assignee

Updated

4 months ago
Depends on: 1526475
Comment hidden (Intermittent Failures Robot)

(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. :)

Flags: needinfo?(nfroyd)

(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.

Flags: needinfo?(nfroyd)

Updated

3 months ago
Priority: P2 → P1

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

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

Depends on D25161

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

Assignee

Updated

3 months ago
Blocks: 1540188
Assignee

Updated

3 months ago
Blocks: bits-download
No longer blocks: update-agent, 1540188
Assignee

Updated

3 months ago
Attachment #9054082 - Flags: data-review?(chutten)
Assignee

Updated

3 months ago
Attachment #9054082 - Flags: data-review?(chutten)
Attachment #9055587 - Flags: data-review?(chutten)
Comment on attachment 9055587 [details]
Data-Review Request form

Passing to :tdsmith as I helped review the code.
Attachment #9055587 - Flags: data-review?(chutten) → data-review?(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.
Attachment #9055587 - Flags: data-review?(tdsmith) → data-review+

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

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!

Flags: needinfo?(lina)
Assignee

Updated

2 months ago
Flags: needinfo?(lina)

Comment 26

2 months ago
Pushed by ksteuber@mozilla.com:
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

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee

Updated

2 months ago
Blocks: 1544894

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, ...)

You need to log in before you can comment on or make changes to this bug.