1474414 - Move most of browser/extensions/activity-stream to browser/components/newtab

Reporter

Description

•

6 years ago

With AboutNewTab handling init bug 1474410 and other bootstrap functionality bug 1474411, we shouldn't need bootstrap.js or install.rdf anymore. The rest of the files can be moved with moz.build updated in browser/extensions and browser/components/newtab.

The existing activity-stream/moz.build will probably simplify a bit not needing the DEFINES, bootstrap, install.rdf.in entries.

I believe jar.mn can be left unchanged, and the % resource entry should work again (as it was only removed for extensions in bug 1446585).

Ed Lee :Mardak

Reporter

Updated

•

6 years ago

Blocks: 1474419

Kate Hudson :k88hudson

Updated

•

6 years ago

Severity: normal → enhancement

Ed Lee :Mardak

Reporter

Comment 1

•

6 years ago

Eh, I'm not sure what I was thinking earlier… :P

But jar.mn most likely will need a lot of changes as part of browser.jar and having the appropriate browser.jar entry scoped/not conflicting with others. Maybe I was thinking the local reference (./lib/*) stuff is unchanged.

Ed Lee :Mardak

Reporter

Comment 2

•

6 years ago

There looks to be various references to browser/extensions/activity-stream outside of activity stream:

https://searchfox.org/mozilla-central/search?q=browser/extensions/activity-stream&path=%2a
- eslintignore
- license.html
- codespell
- various tests/testing

We can probably leave most references within the activity-stream directory unchanged to be cleaned up in the followup 1474419.

Ed Lee :Mardak

Reporter

Comment 3

•

6 years ago

Here's what I mentioned on irc earlier where removing bootstrap.js and install.rdf.in seemed to work with this jar.mn:

```
browser.jar:
% resource activity-stream %res/activity-stream/ contentaccessible=yes
  res/activity-stream/lib/ (./lib/*)
```

Basically:
s![features/activity-stream@mozilla.org] chrome.jar!browser.jar!
s!content/!res/activity-stream/!

We might want to also just combine the existing components/newtab/tests directory into the existing extensions/activity-stream/test directory… ?

Of course tests fail with just these changes without bug 1474410 as activity stream doesn't actually load, but for manual testing from browser console:

(new (Cu.import("resource://activity-stream/lib/ActivityStream.jsm").ActivityStream)).init()

Ed Lee :Mardak

Reporter

Updated

•

6 years ago

See Also: → https://bugzilla.mozilla.org/show_bug.cgi?id=1477054

Jenn Chaulk (:jchaulk)

Updated

•

6 years ago

Iteration: 63.3 - Aug 6 → 63.4 - Aug 20

Ursula Sarracini (:ursula)

Assignee

Updated

•

6 years ago

Assignee: nobody → usarracini

Ursula Sarracini (:ursula)

Assignee

Updated

•

6 years ago

Iteration: 63.4 - Aug 20 → 63.3 - Aug 6

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/259458/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259458/

Mozilla Autoland

Comment 5

•

6 years ago

hg error in cmd: hg push -r . -f try: pushing to ssh://hg.mozilla.org/try
remote: Permission denied (publickey).
abort: no suitable response from remote hg!

Ursula Sarracini (:ursula)

Assignee

Updated

•

6 years ago

Attachment #8994966 - Flags: review?(edilee)

Ed Lee :Mardak

Reporter

Comment 6

•

6 years ago

Looks like some dot files weren't moved/added… Here's the files after applying the patch skipping plain renames:

	deleted:    browser/components/newtab/tests/browser/browser.ini
	deleted:    browser/extensions/activity-stream/.eslintignore
	deleted:    browser/extensions/activity-stream/.eslintrc.js
	deleted:    browser/extensions/activity-stream/.mcignore
	deleted:    browser/extensions/activity-stream/.nvmrc
	deleted:    browser/extensions/activity-stream/.sass-lint.yml
	deleted:    browser/extensions/activity-stream/.taskcluster.yml
	deleted:    browser/extensions/activity-stream/.travis.yml
	deleted:    browser/extensions/activity-stream/bootstrap.js
	deleted:    browser/extensions/activity-stream/install.rdf.in
	deleted:    browser/extensions/activity-stream/jar.mn
	deleted:    browser/extensions/activity-stream/moz.build
	deleted:    browser/extensions/activity-stream/test/.eslintrc.js
	deleted:    browser/extensions/activity-stream/test/functional/mochitest/.eslintrc.js

	new file:   browser/components/newtab/jar.mn

	modified:   .eslintignore
	modified:   CLOBBER
	modified:   browser/components/newtab/moz.build
	modified:   browser/extensions/moz.build
	modified:   js/xpconnect/src/xpcprivate.h
	modified:   python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py
	modified:   testing/runtimes/mochitest-browser-chrome-e10s.runtimes.json
	modified:   testing/runtimes/mochitest-browser-chrome.runtimes.json
	modified:   toolkit/content/license.html

	renamed:    browser/extensions/activity-stream/test/functional/mochitest/* -> browser/components/newtab/test/browser/*
	renamed:    browser/components/newtab/tests/browser/* -> browser/components/newtab/test/browser/*
	renamed:    browser/components/newtab/tests/xpcshell/* -> browser/components/newtab/test/xpcshell/*

So looks like browser/components/newtab/tests/browser/browser.ini got replaced/merged with that from browser/extensions/activity-stream/test/functional/mochitest/browser.ini and similar for jar.mn and moz.build; and all that seems fine. And we do want to delete bootstrap and install.rdf. So of the deleted files, it does look like just the missing dot files.

Just making sure, we're *not* moving AboutNewTab.jsm from browser/modules at this time, and separately in the porting to activity-stream, we'll need to rename test/functional/mochitest to test/browser as well as adding the original browser/components/newtab files to activity-stream repository.

Ed Lee :Mardak

Reporter

Comment 7

•

6 years ago

Looks like codespell.yml needs to be updated:

LinterParseError: tools/lint/codespell.yml: The include directive contains the following paths that don't exist:
  browser/extensions/activity-stream/prerendered/locales/en-US/

Comment hidden (mozreview-request)

Comment on attachment 8994966 [details]
Bug 1474414 - Move most of browser/extensions/activity-stream to browser/components/newtab

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/259458/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8994966 [details]
Bug 1474414 - Move most of browser/extensions/activity-stream to browser/components/newtab

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/259458/diff/2-3/

GitHub Bugzilla PR Linker

Comment 10

•

6 years ago

Attached file Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4265 (deleted) — Details

Ed Lee :Mardak

Reporter

Comment 11

•

6 years ago

mozreview-review

Comment on attachment 8994966 [details]
Bug 1474414 - Move most of browser/extensions/activity-stream to browser/components/newtab

https://reviewboard.mozilla.org/r/259458/#review266758

Looks like it's working and passing (mozilla-central) tests.

There's some references to the add-on id, but those seem to be textual uses not functional: https://searchfox.org/mozilla-central/search?q=activity-stream%40mozilla.org

::: browser/components/newtab/jar.mn:4
(Diff revision 3)
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +browser.jar:

nit: newline before browser.jar

::: browser/components/newtab/moz.build:8
(Diff revision 3)
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  with Files("**"):
>      BUG_COMPONENT = ("Firefox", "New Tab Page")

Let's update this to be what it was for extensions/activity-stream for now:

    BUG_COMPONENT = ("Firefox", "Activity Streams: Newtab")

::: python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py:304
(Diff revision 3)
>                  ],
>                  'dist/bin/modules/osfile/osfile_async_worker.js': [
>                      'toolkit/components/osfile/modules/osfile_async_worker.js',
>                      None
>                  ],
>                  'dist/bin/browser/features/activity-stream@mozilla.org/chrome/content/lib/': [

Pretty sure this should be changed.. to `dist/bin/browser/chrome/browser/res/activity-stream/lib/` ?

Attachment #8994966 - Flags: review?(edilee) → review+

Comment hidden (mozreview-request)

Comment on attachment 8994966 [details]
Bug 1474414 - Move most of browser/extensions/activity-stream to browser/components/newtab

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/259458/diff/3-4/

Pulsebot

Comment 13

•

6 years ago

Pushed by usarracini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/357206a19974
Move most of browser/extensions/activity-stream to browser/components/newtab r=Mardak

Ed Lee :Mardak

Reporter

Updated

•

6 years ago

Blocks: 1449052

Raul Gurzau (:RaulG)

Comment 14

•

6 years ago

Backed out changeset 357206a19974 (bug 1474414) for build bustage at mozbuild/test/codecoverage/test_lcov_rewrite.py on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/autoland/rev/eae45845985074d6a9a20766153a9e3d6091d0ed

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=357206a199742c49efb66ef526953dd4ea77e4da

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=190378075&repo=autoland&lineNumber=41989

Log snippet: 

[task 2018-07-26T21:23:20.051Z] 21:23:20     INFO - /builds/worker/workspace/build/src/python/mozbuild/mozbuild/test/backend/test_test_manifest.py
[task 2018-07-26T21:23:20.051Z] 21:23:20     INFO - ============================= test session starts ==============================
[task 2018-07-26T21:23:20.051Z] 21:23:20     INFO - platform linux2 -- Python 2.7.9, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /builds/worker/workspace/build/src/obj-firefox/_virtualenvs/src-UL-dti-o-2.7/bin/python
[task 2018-07-26T21:23:20.051Z] 21:23:20     INFO - rootdir: /builds/worker/workspace/build/src, inifile: /builds/worker/workspace/build/src/config/mozunit/mozunit/pytest.ini
[task 2018-07-26T21:23:20.051Z] 21:23:20     INFO - collecting ... collected 4 items
[task 2018-07-26T21:23:20.051Z] 21:23:20     INFO - ../python/mozbuild/mozbuild/test/backend/test_test_manifest.py::TestTestManifestBackend::test_all_tests_metadata_file_written PASSED
[task 2018-07-26T21:23:20.051Z] 21:23:20     INFO - ../python/mozbuild/mozbuild/test/backend/test_test_manifest.py::TestTestManifestBackend::test_test_defaults_metadata_file_written PASSED
[task 2018-07-26T21:23:20.051Z] 21:23:20     INFO - ../python/mozbuild/mozbuild/test/backend/test_test_manifest.py::TestTestManifestBackend::test_test_installs_metadata_file_written PASSED
[task 2018-07-26T21:23:20.051Z] 21:23:20     INFO - ../python/mozbuild/mozbuild/test/backend/test_test_manifest.py::TestTestManifestBackend::test_test_manifest_sources PASSED
[task 2018-07-26T21:23:20.051Z] 21:23:20     INFO - =========================== 4 passed in 0.08 seconds ===========================
[task 2018-07-26T21:23:20.191Z] 21:23:20     INFO - /builds/worker/workspace/build/src/python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py
[task 2018-07-26T21:23:20.191Z] 21:23:20     INFO - ============================= test session starts ==============================
[task 2018-07-26T21:23:20.192Z] 21:23:20     INFO - platform linux2 -- Python 2.7.9, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /builds/worker/workspace/build/src/obj-firefox/_virtualenvs/src-UL-dti-o-2.7/bin/python
[task 2018-07-26T21:23:20.192Z] 21:23:20     INFO - rootdir: /builds/worker/workspace/build/src, inifile: /builds/worker/workspace/build/src/config/mozunit/mozunit/pytest.ini
[task 2018-07-26T21:23:20.209Z] 21:23:20     INFO - collecting ... collected 7 items
[task 2018-07-26T21:23:20.209Z] 21:23:20     INFO - ../python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py::TestLcovParser::test_basic_parse PASSED
[task 2018-07-26T21:23:20.209Z] 21:23:20     INFO - ../python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py::TestLcovParser::test_multiple_commas PASSED
[task 2018-07-26T21:23:20.209Z] 21:23:20     INFO - ../python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py::TestLineRemapping::test_map_multiple_included PASSED
[task 2018-07-26T21:23:20.209Z] 21:23:20     INFO - ../python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py::TestLineRemapping::test_remap_lcov PASSED
[task 2018-07-26T21:23:20.209Z] 21:23:20  WARNING - ../python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py::TestUrlFinder::test_chrome_resource_paths TEST-UNEXPECTED-FAIL
[task 2018-07-26T21:23:20.209Z] 21:23:20     INFO - ../python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py::TestUrlFinder::test_jar_paths PASSED
[task 2018-07-26T21:23:20.209Z] 21:23:20     INFO - ../python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py::TestUrlFinder::test_wrong_scheme_paths PASSED
[task 2018-07-26T21:23:20.209Z] 21:23:20     INFO - =================================== FAILURES ===================================
[task 2018-07-26T21:23:20.209Z] 21:23:20     INFO - ___________________ TestUrlFinder.test_chrome_resource_paths ___________________
[task 2018-07-26T21:23:20.209Z] 21:23:20     INFO - self = <test_lcov_rewrite.TestUrlFinder testMethod=test_chrome_resource_paths>
[task 2018-07-26T21:23:20.210Z] 21:23:20     INFO -     def test_chrome_resource_paths(self):
[task 2018-07-26T21:23:20.210Z] 21:23:20     INFO -         paths = [
[task 2018-07-26T21:23:20.210Z] 21:23:20     INFO -             # Path with default url prefix
[task 2018-07-26T21:23:20.210Z] 21:23:20     INFO -             ('resource://gre/modules/osfile/osfile_async_worker.js', ('toolkit/components/osfile/modules/osfile_async_worker.js', None)),
[task 2018-07-26T21:23:20.210Z] 21:23:20     INFO -             # Path with url prefix that is in chrome map
[task 2018-07-26T21:23:20.210Z] 21:23:20     INFO -             ('resource://activity-stream/lib/PrefsFeed.jsm', ('browser/components/newtab/lib/PrefsFeed.jsm', None)),
[task 2018-07-26T21:23:20.210Z] 21:23:20     INFO -             # Path which is in url overrides
[task 2018-07-26T21:23:20.211Z] 21:23:20     INFO -             ('chrome://global/content/netError.xhtml', ('browser/base/content/aboutNetError.xhtml', None)),
[task 2018-07-26T21:23:20.211Z] 21:23:20     INFO -             # Path which ends with > eval
[task 2018-07-26T21:23:20.211Z] 21:23:20     INFO -             ('resource://gre/modules/osfile/osfile_async_worker.js line 3 > eval', None),
[task 2018-07-26T21:23:20.211Z] 21:23:20     INFO -             # Path which ends with > Function
[task 2018-07-26T21:23:20.211Z] 21:23:20     INFO -             ('resource://gre/modules/osfile/osfile_async_worker.js line 3 > Function', None),
[task 2018-07-26T21:23:20.211Z] 21:23:20     INFO -             # Path which contains "->"
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -             ('resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/osfile/osfile_async_worker.js', ('toolkit/components/osfile/modules/osfile_async_worker.js', None)),
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -             # Path with pp_info
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -             ('resource://gre/modules/AppConstants.jsm', ('toolkit/modules/AppConstants.jsm', {
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -                 '101,102': [
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -                     'toolkit/modules/AppConstants.jsm',
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -                     135
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -                 ],
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -             })),
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -             # Path with query
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -             ('resource://activity-stream/lib/PrefsFeed.jsm?q=0.9098419174803978', ('browser/components/newtab/lib/PrefsFeed.jsm', None)),
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -         ]
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -         url_finder = lcov_rewriter.UrlFinder(self._chrome_map_file, '', 'dist/bin/', [])
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO -         for path, expected in paths:
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO - >           self.assertEqual(url_finder.rewrite_url(path), expected)
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO - ../python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py:388:
[task 2018-07-26T21:23:20.215Z] 21:23:20     INFO - _ _

Flags: needinfo?(usarracini)

Ed Lee :Mardak

Reporter

Comment 15

•

6 years ago

Ah sorry, I missed this in the updated patch. You did indeed want to update both features/activity-stream references to point to the new location, but they're slightly different.

Before:

https://searchfox.org/mozilla-central/source/browser/extensions/activity-stream/jar.mn
[features/activity-stream@mozilla.org] chrome.jar:
% resource activity-stream %content/ contentaccessible=yes

https://searchfox.org/mozilla-central/source/python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py#277-305
                'resource://activity-stream/': [
                    'dist/bin/browser/features/activity-stream@mozilla.org/chrome/content',
                'dist/bin/browser/features/activity-stream@mozilla.org/chrome/content/lib/': [
                    'browser/extensions/activity-stream/lib/*',

Autoland:

https://hg.mozilla.org/integration/autoland/file/357206a19974/browser/components/newtab/jar.mn#l5
browser.jar:
% resource activity-stream %res/activity-stream/ contentaccessible=yes

https://hg.mozilla.org/integration/autoland/file/357206a19974/python/mozbuild/mozbuild/test/codecoverage/test_lcov_rewrite.py#l277
                'resource://activity-stream/': [
                    'dist/bin/browser/chrome/browser/res/activity-stream/content',
                'dist/bin/browser/chrome/browser/res/activity-stream/lib/': [
                    'browser/components/newtab/lib/*',

The first one for mapping resource://activity-stream should actually point to …/res/activity-stream (without /content).

You can test locally with `./mach python-test test_lcov_rewrite`

Ed Lee :Mardak

Reporter

Comment 16

•

6 years ago

Bug 1478275 landed in the mean time removing the comment referring to extensions/activity-stream
https://hg.mozilla.org/mozilla-central/rev/49dceb93cc0c

Pulsebot

Comment 17

•

6 years ago

Pushed by edilee@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b183dbe7924
Move most of browser/extensions/activity-stream to browser/components/newtab r=Mardak

Bogdan Tara[:bogdan_tara | bogdant]

Updated

•

6 years ago

Blocks: 1478908

Dorel Luca [:dluca]

Comment 18

•

6 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/3b183dbe7924

Status: NEW → RESOLVED

Closed: 6 years ago

status-firefox63: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → Firefox 63

Ursula Sarracini (:ursula)

Assignee

Comment 19

•

6 years ago

Thanks for re-landing Ed :)

Flags: needinfo?(usarracini)

[github robot]

Comment 20

•

6 years ago

Commits pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/b39f7a58fac768ae4a4bb7c99d0f7200de27533d
Backport Bug 1474414 - Move most of browser/extensions/activity-stream to browser/components/newtab

https://github.com/mozilla/activity-stream/commit/9324a0fcf93eeb5292f0a4381f94e1ffe62519de
Merge pull request #4265 from sarracini/backport_bug_1474414

Backport Bug 1474414 - Move most of browser/extensions/activity-stream to browser/components/newtab

Ed Lee :Mardak

Reporter

Updated

•

6 years ago

Blocks: 1479055

Nobody; OK to take it and work on it

Updated

•

5 years ago

Component: Activity Streams: Newtab → New Tab Page

Bug 1474414 - Move most of browser/extensions/activity-stream to browser/components/newtab 6 years ago Ursula Sarracini (:ursula) (deleted), text/x-review-board-request	Mardak : review+	Details
Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4265 6 years ago GitHub Bugzilla PR Linker (deleted), text/x-github-pull-request		Details