Closed Bug 1473525 Opened Last year Closed Last year

canUpload() gives a difference answer depending on when it's called

Categories

(Toolkit :: Telemetry, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: scolville, Assigned: Dexter)

Details

Attachments

(2 files)

Attached image canupload-log.png
I'm using a dynamic module import to include the HCT lib. 

Once the import promise is resolved I can see I have the HCT object. If I then call canUpload() it returns false. 

However if I call canUpload() from the console later it returns true. 

This suggests there's a delay before canUpload returns the expected answer probably related to the registration.

See the attached screenshot which shows the logging for the STR.
Assignee: nobody → alessio.placitelli
Priority: -- → P1
The HCT content library needs the chrome to broadcast the value of the
Telemetry state before it can report a reliable value. This patch adds
a way to wait for the broadcast to happen and singnal that it's ok
to read the value from canUpload.
Comment on attachment 8990720 [details]
Bug 1473525 - Add an API endpoint to wait for HCT to init. r?chutten,janerik

Chris H-C :chutten has approved the revision.

https://phabricator.services.mozilla.com/D2030
Attachment #8990720 - Flags: review+
Ugh, it eats comments when accepting.

Looks good, I have one question about approach.
toolkit/components/telemetry/hybrid-content/HybridContentTelemetry-lib.js
13
	

Rather than storing this globally forever, did you consider passing the resolve function to policyChangeHandler via bindor closure capturing?
Comment on attachment 8990720 [details]
Bug 1473525 - Add an API endpoint to wait for HCT to init. r?chutten,janerik

Jan-Erik Rediger [:janerik] has approved the revision.

https://phabricator.services.mozilla.com/D2030
Attachment #8990720 - Flags: review+
Comment on attachment 8990720 [details]
Bug 1473525 - Add an API endpoint to wait for HCT to init. r?chutten,janerik

Jan-Erik Rediger [:janerik] has requested changes to the revision.

https://phabricator.services.mozilla.com/D2030
Attachment #8990720 - Flags: review+
Comment on attachment 8990720 [details]
Bug 1473525 - Add an API endpoint to wait for HCT to init. r?chutten,janerik

Jan-Erik Rediger [:janerik] has approved the revision.

https://phabricator.services.mozilla.com/D2030
Attachment #8990720 - Flags: review+
Pushed by aplacitelli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c42c7ac4fbac
Add an API endpoint to wait for HCT to init. r=janerik,chutten
The updated version is available on NPM (https://www.npmjs.com/package/mozilla-hybrid-content-telemetry/v/1.1.0). Stuart, does this fix the problem?
Flags: needinfo?(scolville)
(In reply to Alessio Placitelli [:Dexter] from comment #9)
> The updated version is available on NPM
> (https://www.npmjs.com/package/mozilla-hybrid-content-telemetry/v/1.1.0).
> Stuart, does this fix the problem?

Thanks Alessio, I'll take a look and report back.
https://hg.mozilla.org/mozilla-central/rev/c42c7ac4fbac
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
(In reply to Stuart Colville [:scolville] [:muffinresearch] from comment #10)
> (In reply to Alessio Placitelli [:Dexter] from comment #9)
> > The updated version is available on NPM
> > (https://www.npmjs.com/package/mozilla-hybrid-content-telemetry/v/1.1.0).
> > Stuart, does this fix the problem?
> 
> Thanks Alessio, I'll take a look and report back.

Yep this is now working. Thanks!
Flags: needinfo?(scolville)
You need to log in before you can comment on or make changes to this bug.