Turn on Activity Stream by default for Nightly

VERIFIED FIXED in Firefox 56

Status

()

Firefox
Activity Streams: General
VERIFIED FIXED
10 months ago
26 days ago

People

(Reporter: Mardak, Assigned: Mardak)

Tracking

(Depends on: 2 bugs)

Trunk
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

10 months ago
We'll need to match the building of activity-stream as we don't want to be enabled in Firefox but not actually have the content to load:
https://searchfox.org/mozilla-central/source/browser/extensions/moz.build#39-42
Comment hidden (mozreview-request)
(Assignee)

Comment 2

10 months ago
This can't land until bug 1381335 and bug 1381525 make it to mozilla-central (or I suppose technically on autoland, where they currently are, if this patch also lands on autoland). Doesn't make sense to push to try unless those other commits are included.
Depends on: 1381525, 1381335
(Assignee)

Comment 3

10 months ago
Comment on attachment 8887154 [details]
Bug 1381569 - Turn on Activity Stream by default for Nightly.

This patch turns on activity stream in nightly, but what should happen outside of nightly? I.e., should our tests be running in beta/release if the extension isn't being built?

On one hand, it could be good to test the expected behavior with activity-stream enabled and disabled by having our tests run (where load_location only passes with enabled=true).

On the other hand, as we add additional tests, making sure they all pass when enabled AND disabled can be tricky.

Who should r? the enable patch? mconley?
Flags: needinfo?(khudson)
Flags: needinfo?(dmose)

Comment 4

10 months ago
I would defer to whatever other similar roll-out strategies have done, but I think it would be a good idea to have tests running with both for now since we're eventually planning on doing a staged roll-out in release.

I also think mconley would be a good reviewer so he can share in the joy.
Flags: needinfo?(khudson)

Comment 5

10 months ago
Tests running both as in, running with activity-stream in nightly and tiles in beta etc., if possible.
(Assignee)

Comment 6

10 months ago
Tiles tests will be running in nightly and beta, etc, and this is supporting activity-stream enabled/disabled. The current patch has activity-stream tests only running in nightly and only supports activity-stream enabled.

Comment 8

10 months ago
As far as what existing practice is for this kind of thing, maybe Corey Price would know?  Or someone from PI (eg bsmedberg)

(In reply to Ed Lee :Mardak from comment #6)
> Tiles tests will be running in nightly and beta, etc, and this is supporting
> activity-stream enabled/disabled. The current patch has activity-stream
> tests only running in nightly and only supports activity-stream enabled.

Can you unpack that first sentence somewhat?  I'm having a hard time figuring out how it's not in conflict with the second sentence.
Flags: needinfo?(dmose) → needinfo?(edilee)
(Assignee)

Comment 9

10 months ago
(In reply to Dan Mosedale (:dmose) from comment #8)
> Can you unpack that first sentence somewhat?  I'm having a hard time
> figuring out how it's not in conflict with the second sentence.
browser/base/content/test/newtab tests pass whether activity-stream.enabled is true or false (because head.js forces the pref to false).
browser/extensions/activity-stream/test/functional/mochitest only passes when activity-stream.enabled is true.

The newtab tests are currently running on nightly, beta, etc.

The activity-stream tests are not running anywhere except our pine/try builds when we modify moz.build to have BROWSER_CHROME_MANIFESTS += ['test/functional/mochitest/browser.ini']. But this patch will turn it on in nightly but keeps them only running nightly.
Flags: needinfo?(edilee)

Comment 10

10 months ago
Would having the activity-stream test behavior match the tiles test behavior be the win here?  This way they're effectively each running with the pref value that they're expected to have

> On the other hand, as we add additional tests, making sure they all pass when enabled AND disabled can be tricky.

Where does the trickiness come in?  Since we'd be forcing activity-stream on for the activity-stream tests, even if the build had things disabled, we'd end up with (effectively) a single environment...
Flags: needinfo?(edilee)
(Assignee)

Comment 11

10 months ago
Oh, I suppose we could add a head.js to force activity-stream on. And maybe make the load_location test with the pref enabled and disabled as a sanity check (even though tiles newtab would be checking it anyway).
Flags: needinfo?(edilee)

Comment 12

10 months ago
Right, and also presumably make the tests run on all channels, not just Nightly.
(Assignee)

Comment 13

10 months ago
Comment on attachment 8887154 [details]
Bug 1381569 - Turn on Activity Stream by default for Nightly.

This patch turns on activity stream in nightly as well as its related tests. We'll have a followup issue to clean up activity-stream test coverage to make sure the transition from enabled (nightly) -> disabled (beta) doesn't cause additional test failures.

Perfherder looks good:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=e0b0865639cebc1b5afa0268a4b073fcdde0e69c&newProject=try&newRevision=790f8426d1641c96bdccce74f7f0c221fdfa5e00&framework=1

Treeherder looks good except windows 10 debug:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=790f8426d1641c96bdccce74f7f0c221fdfa5e00&group_state=expanded&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified

Just making sure, we will need to make sure there aren't test regressions on Windows 10 debug? (I retriggered a bunch to see if they're perma or intermittent.)
Attachment #8887154 - Flags: review?(mconley)
Comment on attachment 8887154 [details]
Bug 1381569 - Turn on Activity Stream by default for Nightly.

https://reviewboard.mozilla.org/r/157898/#review163140

Looks good! Assuming those failures on try are indeed intermittent, I say we land this puppy.

Might be worth a firefox-dev mailing list post as well.
Attachment #8887154 - Flags: review?(mconley) → review+
(Assignee)

Comment 15

10 months ago
Filed https://github.com/mozilla/activity-stream/issues/2880 to track the test behavior of default true/false and build/not-build.
(Assignee)

Comment 16

10 months ago
Unfortunately it looks like those Windows 10 x64 debug tests are mostly perma-orange:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=728274b408b41fa6e5fdb4070fe099357ecf69f7

Tracking them here:
https://github.com/mozilla/activity-stream/projects/15

Seems like we missed these as our `pine` pushes test with Windows x64 on Windows 8 instead of 10……………
(Assignee)

Comment 17

10 months ago
Oh wait, I thought I remembered looking at this, and that's why I was asking if Windows 10 x64 debug was important, but I seemed to have confused myself in the process somewhere.

Those same tests are failing even without activity stream enabled:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=728274b408b41fa6e5fdb4070fe099357ecf69f7&selectedJob=115021514

I think I searched for "windows 10 x64 debug browser-chrome" on mozilla-central treeherder and too quickly glanced over the 8 vs 10 assuming the passing tests were for Windows 10, but they were actually only run on Windows 8!
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-searchStr=windows%2010%20x64%20debug%20browser-chrome

I'M LANDING THIS!? 

Comment 18

10 months ago
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 36668b53d2aa -d b8f66d5a9748: rebasing 407525:36668b53d2aa "Bug 1381569 - Turn on Activity Stream by default for Nightly. r=mconley" (tip)
merging browser/extensions/activity-stream/test/functional/mochitest/browser.ini
warning: conflicts while merging browser/extensions/activity-stream/test/functional/mochitest/browser.ini! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request)

Comment 21

10 months ago
Pushed by edilee@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/87e8ccdd8aa6
Turn on Activity Stream by default for Nightly. r=mconley

Comment 22

10 months ago
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/b9031e50fd76
Backed out changeset 87e8ccdd8aa6 for failing browser/base/content/test/performance/browser_windowopen_reflows.js on Windows 8 x64. r=backout
Backed out 

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=87e8ccdd8aa6f6b549922b4debc19a4126b7d940&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=115179839&repo=autoland

03:51:22     INFO - TEST-START | browser/base/content/test/performance/browser_windowopen_reflows.js
03:51:22     INFO - TEST-INFO | started process screenshot
03:51:22     INFO - TEST-INFO | screenshot: exit 0
03:51:22     INFO - Buffered messages logged at 03:51:22
03:51:22     INFO - Entering test bound 
03:51:22     INFO - Skipping this test because of perma-failures on Windows 8 x64 (bug 1381521)
03:51:22     INFO - Leaving test bound 
03:51:22     INFO - Buffered messages finished
03:51:22     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_windowopen_reflows.js | This test contains no passes, no fails and no todos. Maybe it threw a silent exception? Make sure you use waitForExplicitFinish() if you need it. - 
03:51:22     INFO - GECKO(3368) | MEMORY STAT | vsize 1875MB | vsizeMaxContiguous 6839480MB | residentFast 350MB | heapAllocated 141MB
03:51:22     INFO - TEST-OK | browser/base/content/test/performance/browser_windowopen_reflows.js | took 22ms

Mike, can you take a look at this, please? Seems the check from bug 1363361 doesn't work anymore.
Flags: needinfo?(mconley)
Flags: needinfo?(edilee)

Comment 24

10 months ago
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/07a7f3038e14
Turn on Activity Stream by default for Nightly. r=mconley
Relanded because the failing test had only been automatically moved into a different bucket when this landed.
Flags: needinfo?(mconley)
Flags: needinfo?(edilee)
Hey Mardak,

Weren't we supposed to also turn off the content process preloader at the same time as turning Activity Stream on?
Flags: needinfo?(edilee)

Comment 27

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/07a7f3038e14
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Assignee: nobody → edilee
(Assignee)

Comment 28

10 months ago
(In reply to Mike Conley (:mconley) - Digging out of needinfo / review backlog. from comment #26)
> Weren't we supposed to also turn off the content process preloader at the
> same time as turning Activity Stream on?
I believe the discussion originated from trying to fix bug 1372664, and when we tried flipping dom.ipc.processPrelaunch.enabled, it didn't help because that specific test was already setting prelaunch to false. The longer term fix discussed resulted in bug 1376895 being filed. I can flip the pref in bug 1381804 if you think it'll help with the memory regression?
Flags: needinfo?(edilee)
Duplicate of this bug: 1373321

Updated

10 months ago
Depends on: 1382139

Updated

10 months ago
Depends on: 1377317

Comment 30

10 months ago
I have reproduced this bug with Nightly 56.0a1 (2017-07-17)   on Ubuntu 16.04, 64 bit!

The fix is now verified on Latest Nightly.

Build ID 	20170719100147
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [bugday-20170719]

Comment 31

10 months ago
I have successfully reproduced this bug with Nightly 56.0a1 (2017-07-17) (32-bit) on windows 10(32bit)

this bug is verified fix with  latest nightly 56.0a1 (2017-07-19) (32-bit)

Build ID: 20170719030206
Mozilla/5.0 (Windows NT 10.0; rv:56.0) Gecko/20100101 Firefox/56.0

[bugday-20170719]

Comment 32

10 months ago
As per Comment 30 and Comment 31, I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
status-firefox56: fixed → verified
(Assignee)

Updated

10 months ago
Depends on: 1382719
Depends on: 1382887
(Assignee)

Updated

10 months ago
Depends on: 1382079
Depends on: 1383593
Depends on: 1383875

Updated

10 months ago
Depends on: 1383921

Updated

10 months ago
Depends on: 1384052

Updated

10 months ago
No longer depends on: 1383921

Updated

10 months ago
Depends on: 1384072

Updated

10 months ago
Depends on: 1382367
(Assignee)

Updated

8 months ago
Depends on: 1401201

Updated

8 months ago
Depends on: 1403414
Depends on: 1408026
Depends on: 1438143
You need to log in before you can comment on or make changes to this bug.