Nicolas Chevobbe [:nchevobbe] Reporter
	Description • 8 years ago

Since we're running an experiment having Reps in Github (https://github.com/devtools-html/devtools-reps), it would be nice to have the ability to require the webpack bundle generated there instead of the Rep components in m-c.

As a first step, we can have a user preference to tell the browser whether we want to load Reps from the bundle or from the regular path.
Along with that, we can have a reps-bundle.js stub file in m-c, which then will be replaced by the real module when launching the script to run mochitests in the Github project.

Once we're confident about the bundle, we could then remove the stub and use the real bundle in m-c, and always load from the bundle in the console.

	Jason Laster [:jlast] Assignee
	Comment 2 • 8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe9c490505953c5a21c32a95c5f53bdd67ceb8b3

	Jason Laster [:jlast] Assignee
	Comment 3 • 8 years ago

This patch adds a reps.js and reps.css bundle file to tree along with a new pref use-reps-bundle, which will let us try the bundle in tree.

It also sets us up well to update the reps module while we work on it in github.

	Tom Tromey :tromey
	Comment 4 • 8 years ago

Comment on attachment 8825071 [details] [diff] [review]
bundle-reps.patch

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

Thank you.  This looks good and is fine with one nit.
To be clear, I didn't actually read the reps.js file, but I assume it's fine.

::: devtools/client/preferences/devtools.js
@@ +323,5 @@
>  #else
>  pref("devtools.webconsole.new-frontend-enabled", false);
>  #endif
>  
> +pref("devtools.webconsole.use-reps-bundle", false);

I think this should have a comment explaining its purpose.  That seems to be the standard in this file.

	Pulsebot
	Comment 6 • 8 years ago

Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/35986e7aa356
Start using Reps bundle behind a flag. r=ttromey

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 7 • 8 years ago

Backed out for unknown -webkit-scrollbar in reps.css:

https://hg.mozilla.org/integration/mozilla-inbound/rev/abb4c9666456dcbb7a05eb7093e73ba8098afb06

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=22fa3b948119d788d50a02394ab9e3624e8505ef
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=67424819&repo=mozilla-inbound

15:03:22     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_parsable_css.js | Got error message for jar:file:///builds/slave/test/build/application/firefox/browser/omni.ja!/chrome/devtools/modules/devtools/client/shared/components/reps.css: Unknown pseudo-class or pseudo-element ‘-webkit-scrollbar’.  Ruleset ignored due to bad selector. - 
15:03:22     INFO - Stack trace:
15:03:22     INFO -     chrome://mochitests/content/browser/browser/base/content/test/general/browser_parsable_css.js:messageIsCSSError:185
15:03:22     INFO -     chrome://mochitests/content/browser/browser/base/content/test/general/browser_parsable_css.js:checkAllTheCSS:343
15:03:22     INFO -     Tester_execTest@chrome://mochikit/content/browser-test.js:737:9
15:03:22     INFO -     Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:657:7
15:03:22     INFO -     SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:744:59

	Jason Laster [:jlast] Assignee
	Comment 9 • 8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=af18ad771528976d5b72e4878b7e96dd05ce98a5

	Pulsebot
	Comment 10 • 8 years ago

Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8091269b852c
Start using Reps bundle behind a flag. r=bgrins

	Ryan VanderMeulen [:RyanVM]
	Comment 11 • 8 years ago

Backed out for breaking ESLint in the same manner as shown in the last Try push.
https://hg.mozilla.org/integration/autoland/rev/f03d25dd8af07454613c00d7aca1e11a04cb81b9

	Julian Descottes [:jdescottes]
	Comment 13 • 8 years ago

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=75348af83aa5cae0dab44d25cd6457838d928253

	Julian Descottes [:jdescottes]
	Comment 15 • 8 years ago

New try push : https://treeherder.mozilla.org/#/jobs?repo=try&revision=8db7e3c54b5af25fc58318349741efb983eafa23

The reps bundle used here was built by using a modified version of devtools-launchpad, see issue on github: https://github.com/devtools-html/devtools-core/issues/91

	Julian Descottes [:jdescottes]
	Comment 17 • 8 years ago

Updated the patch to remove the reps.css from the whitelist of browser_parsable_css.js. The version we have right now is not triggering any warning and as such doesn't need to be ignored. Except for this, try was green, so I propose to move forward with this.

Jason: how did you build the reps.css you had in your previous patch? It looked quite different from the one in the devtools-reps source.

	Julian Descottes [:jdescottes]
	Comment 20 • 8 years ago

The second changeset is splitting the devtools/client/shared/components/test/mochitest tests into reps/ and (existing) mochitest/ folders. 

All the tests for reps now live in their dedicated folder which will make synchronisation to/from github easier.

	Jason Laster [:jlast] Assignee
	Comment 21 • 8 years ago
mozreview-review

Comment on attachment 8827469 [details]
Bug 1325401 - extract reps chrome mochitests to a dedicated folder;

https://reviewboard.mozilla.org/r/105142/#review105974

Looks good!

	Jason Laster [:jlast] Assignee
	Comment 22 • 8 years ago
mozreview-review

Comment on attachment 8827113 [details]
Bug 1325401 - Add a reps bundle, disabled by default;

https://reviewboard.mozilla.org/r/104938/#review105976

Looks good. Impressive we get all of the call sites covered.

	Julian Descottes [:jdescottes]
	Comment 27 • 8 years ago

Thanks for the reviews, let's just wait for the last try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0d383ca01c2f5e67a555a88cb4ee3cfe2818832

Clearing ni? as we clarified this on IRC.

	Julian Descottes [:jdescottes]
	Comment 28 • 8 years ago

Some strange build failures on Linux. Retriggered builds : https://treeherder.mozilla.org/#/jobs?repo=try&revision=d2d96793dff8a00de652506cfac7a2e8068a9d81

	Jason Laster [:jlast] Assignee
	Comment 33 • 8 years ago
mozreview-review

Comment on attachment 8827627 [details]
Bug 1325401 - Export all reps from rep.js without createFactories wrappers;

https://reviewboard.mozilla.org/r/105242/#review106194

	Jason Laster [:jlast] Assignee
	Comment 34 • 8 years ago
mozreview-review

Comment on attachment 8827638 [details]
Bug 1325401 - update reps bundle;

https://reviewboard.mozilla.org/r/105250/#review106196

	Julian Descottes [:jdescottes]
	Comment 37 • 8 years ago

Thanks for the reviews! This should hopefully be the last version for this patch.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=63683d535cdf06aefd29c614d7905371fc36fd5c

	Julian Descottes [:jdescottes]
	Comment 40 • 8 years ago

try : https://treeherder.mozilla.org/#/jobs?repo=try&revision=d50954b06278f0aebf4a008911b488d0975535ca

	Pulsebot
	Comment 41 • 8 years ago

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/50aca7a3d253
Add a reps bundle, disabled by default;r=jlast
https://hg.mozilla.org/integration/autoland/rev/03792b9efb64
extract reps chrome mochitests to a dedicated folder;r=jlast

	Jason Laster [:jlast] Assignee
	Comment 43 • 8 years ago
mozreview-review

Comment on attachment 8827960 [details]
Bug 1325401 - use load-reps in netmonitor;

https://reviewboard.mozilla.org/r/105528/#review106338

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 44 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/50aca7a3d253
https://hg.mozilla.org/mozilla-central/rev/03792b9efb64