Run a Telemetry experiment to vet the unaccelerated compositor process in Beta 56

RESOLVED WONTFIX

Status

()

P3
normal
RESOLVED WONTFIX
2 years ago
a year ago

People

(Reporter: ashughes, Assigned: ashughes)

Tracking

(Depends on: 1 bug)

56 Branch
Unspecified
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 unaffected)

Details

(Whiteboard: [gfx-noted])

Attachments

(6 attachments)

(Assignee)

Description

2 years ago
+++ This bug was initially created as a clone of Bug #1367246 +++

This is bug tracks running a Telemetry Experiment to A/B test the unaccelerated compositor process in Beta 56 as implemented in bug 1356091.
(Assignee)

Updated

2 years ago
No longer blocks: 1381599
(Assignee)

Comment 1

2 years ago
Created attachment 8902353 [details] [diff] [review]
bug1394903_v1.patch

David and Felipe, can you please review this patch? I've basically taken the code from our Nightly 56 telemetry experiment and re-purposed it for a similar experiment in Beta 56. 

This code has not yet been tested in a stock Beta build as that requires signed add-ons. However I need to get the code to pass review before I can request add-on signing.

Thanks
Assignee: nobody → anthony.s.hughes
Status: NEW → ASSIGNED
Attachment #8902353 - Flags: review?(felipc)
Attachment #8902353 - Flags: review?(dvander)
I should mention that the "sideloaded addon" warning for experiments bug never got fixed, so please be aware about it showing up for users on beta
Attachment #8902353 - Flags: review?(dvander) → review+
Comment on attachment 8902353 [details] [diff] [review]
bug1394903_v1.patch

Review of attachment 8902353 [details] [diff] [review]:
-----------------------------------------------------------------

::: experiments/unaccelerated-compositor-process-beta56/code/install.rdf
@@ +10,5 @@
> +    <!-- Firefox -->
> +    <em:targetApplication>
> +      <Description>
> +        <em:id>{ec8030f7-c20a-464f-9b0e-13a3a9e97384}</em:id>
> +        <em:minVersion>56.0</em:minVersion>

I'm not sure, but I think _early_ betas are marked as 56.0b, and only late betas have 56.0. If you don't want to target users of the first beta builds then this is fine.. Otherwise it's worth checking.
Attachment #8902353 - Flags: review?(felipc) → review+
(Assignee)

Comment 4

2 years ago
Created attachment 8902747 [details]
experiment.xpi (unsigned)

(In reply to :Felipe Gomes (needinfo me!) from comment #3)
> I'm not sure, but I think _early_ betas are marked as 56.0b, and only late
> betas have 56.0. If you don't want to target users of the first beta builds
> then this is fine.. Otherwise it's worth checking.

Thanks, I'll update the versions to 56.0b1 - 56.0b99 to catch all the Betas before RC. 

Jason, can you please sign the attached xpi? I'll need this to test in stock beta builds. Thanks.
Flags: needinfo?(jthomas)
Created attachment 8902761 [details]
experiment.xpi signed

Please see attached.
Flags: needinfo?(jthomas)
(Assignee)

Comment 6

2 years ago
Created attachment 8902922 [details]
experiment.xpi (unsigned v2)

Hi Jason, I've reverted back to using "56.0" in my manifest. For some reason max_version was failing if I used 56.0b99 or 56.0b*. Can you please sign this new xpi?
Flags: needinfo?(jthomas)
Created attachment 8903147 [details]
experiment.xpi (signed v2)

Please see attached.
Flags: needinfo?(jthomas)
(Assignee)

Comment 8

2 years ago
Created attachment 8903250 [details]
debug.txt

Felipe, I am getting an error I haven't seen before on install (please see attached) with the signed addon using Firefox Beta 56.0b6. Do you know what this might be about?
Flags: needinfo?(felipc)
(Assignee)

Comment 9

2 years ago
(In reply to Anthony Hughes (:ashughes) [GFX][QA][Mentor] from comment #8)
> Created attachment 8903250 [details]
> debug.txt
> 
> Felipe, I am getting an error I haven't seen before on install (please see
> attached) with the signed addon using Firefox Beta 56.0b6. Do you know what
> this might be about?

PS

Error in question is at timestamp 1504203421823.
Hmm I've never seen that too. I don't know which URL is this.. Let's ask Rob Helmer
Flags: needinfo?(felipc) → needinfo?(rhelmer)
(In reply to :Felipe Gomes (needinfo me!) from comment #10)
> Hmm I've never seen that too. I don't know which URL is this.. Let's ask Rob
> Helmer

Hm so I followed the stack trace back to:

http://searchfox.org/mozilla-central/source/browser/experiments/Experiments.jsm#1786

According to the docs, there should be a "xpiURL" in the manifest:
http://firefox-source-docs.mozilla.org/browser/experiments/experiments/manifest.html

Felipe, do you know if that is set in the manifest and if so to what?
Flags: needinfo?(rhelmer) → needinfo?(felipc)
Anthony, this xpiURL should be a value in the generated firefox-manifest.json
Flags: needinfo?(felipc) → needinfo?(anthony.s.hughes)
(Assignee)

Comment 13

2 years ago
(In reply to :Felipe Gomes (needinfo me!) from comment #12)
> Anthony, this xpiURL should be a value in the generated firefox-manifest.json

Here is what's returned from the manifest:
> {
>   "experiments": [{
>               "xpiURL": "%SERVER%:8080/unaccelerated-compositor-process-beta56%40experiments.mozilla.org/experiment.xpi", 
>              "xpiHash": "sha256:6a2447bd95a46f59b5f584fed7c687ddcb02879719119fa9a2207b2fae621926", 
>              "appName": ["Firefox"], 
>     "maxActiveSeconds": 864000, 
>           "maxVersion": "56.0", 
>               "sample": 0.25, 
>            "startTime": 1503878400, 
>           "minVersion": "56.0", 
>              "endTime": 1505865600, 
>                   "os": ["WINNT"], 
>                   "id": "unaccelerated-compositor-process-beta56@experiments.mozilla.org", 
>              "channel": ["beta"]
>   }], 
>   "version": 1
> }
Flags: needinfo?(anthony.s.hughes) → needinfo?(felipc)
Do you use a script to build the server? this %SERVER% part should have been http://localhost for your own testing, if you used `python build.py http://localhost:8080 ../build`, from https://wiki.mozilla.org/QA/Telemetry/Developing_a_Telemetry_Experiment#Building_the_server

If there's some bug in the build process we will need to figure it out, but to unblock your own testing you can manually fix it in your firefox-manifest.json
Flags: needinfo?(felipc)
(Assignee)

Comment 15

2 years ago
(In reply to :Felipe Gomes (needinfo me!) from comment #14)
> Do you use a script to build the server? this %SERVER% part should have been http://localhost for your 
> own testing, if you used `python build.py http://localhost:8080 ../build`, from
> https://wiki.mozilla.org/QA/Telemetry/Developing_a_Telemetry_Experiment#Building_the_server

The manifest shows the IP address of the server, I just replaced it with %SERVER% in my comment above to obscure it from public view.
Ah ok.. does it include the http:// part though? It should. That field (appears to be) causing the malformed URL message..
(Assignee)

Comment 17

2 years ago
(In reply to :Felipe Gomes (needinfo me!) from comment #16)
> Ah ok.. does it include the http:// part though? It should. That field
> (appears to be) causing the malformed URL message..

Good catch. I forgot to include http:// when running the build.py command. It installs correctly now. I wonder if we can add some error checking to that script to prevent stupid mistakes like that.
(Assignee)

Comment 18

2 years ago
Felipe, I have finished testing this locally and it works as expected. Can you please push this to staging for further testing?
Flags: needinfo?(felipc)
(Assignee)

Comment 19

2 years ago
(In reply to Anthony Hughes (:ashughes) [GFX][QA][Mentor] from comment #18)
> Felipe, I have finished testing this locally and it works as expected. Can
> you please push this to staging for further testing?

Note, we should push the reviewed patch along with the v2-signed version of the extension. Thanks.
status-firefox57: --- → unaffected
(Assignee)

Comment 21

a year ago
(In reply to :Felipe Gomes (needinfo me!) from comment #20)
> Done:
> https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/
> a9f597a8c405c8890fc4a78acbc3b66df6fa8a0c

Thanks for pushing this, Felipe. However I was away for a couple weeks following and I guess this never got the testing it needed to go live. I think we're too late at this point to push this live. Can you pull back this change?

Milan, any reason to move forward with this?
Flags: needinfo?(milan)
Flags: needinfo?(felipc)
At this point, we don't want to change the default, we've been running with it enabled for too long.  We could do a test anyway, but it will probably be for informational purposes alone.
Flags: needinfo?(milan)
(In reply to Anthony Hughes (:ashughes) from comment #21)
> Thanks for pushing this, Felipe. However I was away for a couple weeks
> following and I guess this never got the testing it needed to go live. I
> think we're too late at this point to push this live. Can you pull back this
> change?

Done:
https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/b130b38531dc71b1272020fb987388733ebea33b
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Flags: needinfo?(felipc)
Resolution: --- → WONTFIX
(Assignee)

Comment 24

a year ago
Thanks Felipe.
You need to log in before you can comment on or make changes to this bug.