In the performance tests we have some very specific metrics we want to collect and they require us to make some changes to the apps themselves. As a result we include JS we don't actually need in production for testing only. Given the very small amount of code we are looking at today it is not a big issue but we should have conditional statements that allow us to include debug/test code only in un-optimized (or debug/test) builds so we don't have to worry about actually slowing the production application just for testing. This could also be useful for things like desktop compatibility shims.

Also- these statements should not break normal web content. Ideally we can use some very specific comments around some code/html that will get removed during build time. The worst that can happen is we ship with the comments though ideally we remove them during the optimized build entirely.

We're thinking about going in that direction. Right now the in-app metrics are varying a lot [1] and we sometimes don't receive events that are dispatched very early in the app lifetime (we never receive the full 5 first chunk event [2]). So we probably want the performance helper to do more when included. [1] https://datazilla.mozilla.org/b2g?branch=master&range=7&test=rendering_time_>_last_chunk&app=communications/contacts&app_list=browser,calendar,camera,clock,communications/contacts,contacts,email,fm_radio,ftu,gallery,marketplace,messages,music,phone,settings,usage&gaia_rev=c12c99b6862ef9da&gecko_rev=9306ad2157bb28da [2] https://datazilla.mozilla.org/b2g?branch=master&range=7&test=rendering_time_>_first_chunk&app=communications/contacts&app_list=browser,calendar,camera,clock,communications/contacts,contacts,email,fm_radio,ftu,gallery,marketplace,messages,music,phone,settings,usage&gaia_rev=aafcb4fbcfb30371&gecko_rev=9306ad2157bb28da

I talked with Vivien about our issues and he mentioned loadFrameScript, a method that can allow us to insert our code in the Gecko process before the app is launched. That sounds like a cleaner way to solve our issues.

From a discussion on IRC with jgriffin, we should switch to "chrome" context before executing our script that will use loadFrameScript. loadFrameScript itself needs an URL (either chrome:// or data:) so I suspect we'll need to convert our script as a data-url. After that, we need to switch back to "content" context.

I still plan to work on this ( I had a 25% working patch ) but I probably won't get to finish it this week :( once I wrap up my tef+ stuff I will work on this.

I wrote some code that can do this recently: https://github.com/mozilla-b2g/marionette-content-script but it will need some changes to work OOP I believe.

Proposed implementation on top of the branch for bug 915156.

Comment on attachment 8340562 [details] Pull request This does not look quite right to me... Usually more boilerplate is needed so each frame in the content process has the correct setup. See: https://github.com/mozilla-b2g/gaia/blob/master/apps/email/test/marionette/lib/mocks/mock_navigator_mozalarms.js I am no expert here... I stole this code from some other code in the old keyboard (gecko side) code base.

On overall it does work, but there seems to be a discrepency in the output before and after the patch. It is not that obvious though, nor does it cause any error :-/

Marionette Content Script seems to need this pull request to work. https://github.com/mozilla-b2g/marionette-content-script/pull/4

All things consider, I doubt of the benefit of doing so. We already have the PerformanceHelper as a no-op if there is no listener. Unlike in other places where this is used to "mock" other subsystems for testing purpose. Deprioritizing.

I don't see the relation between the no-op thingie and the purpose of that bug: having more reliable measurements.

I don't see where it would be more reliable either.

Right now I have some other ideas about this that would be worth it.

James, I believe this fix is important.

Looks like mike fixed it https://github.com/mozilla-b2g/marionette-content-script/blob/master/index.js#L50

P1, see bug 980978

Comment on attachment 8412837 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18697 Questions and suggesting in the PR. I haven't tested it yet, but I am really happy to see this happening !

Comment on attachment 8412837 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18697 I have updated the PR with some of the issues addressed and answers given.

And for the record, it seems that this doesn't work for Tarako (1.3T) due to differences in the platform. I will need to figure out a proper workaround for that.

(In reply to Hubert Figuiere [:hub] from comment #22) > And for the record, it seems that this doesn't work for Tarako (1.3T) due to > differences in the platform. I will need to figure out a proper workaround > for that. That was bug 1006064 - and then bug 1007401 is about switch automation over. So we should be good to go.

Sorry for the delay, I was in the middle of 1.3t+ issues. Now it's over but it's the end of the week but I'll be sure to review this asap next week. On a side note, it may be a good idea to shift progressively the reviews to your team.

Comment on attachment 8412837 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18697 r=me with the nits

Comment on attachment 8412837 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18697 oops, wrong bug

Comment on attachment 8412837 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18697 This looks good, but I still have a question about the pref idea. I'm giving r+ because we badly need this patch I want it landed. But I think using a pref to control how it works could be a good idea.

Merged https://github.com/mozilla-b2g/gaia/commit/2dabcaf728e8fd74c0d2959ad28dfe8e5dc5e9b2

Filed bug 1009650 to do it using preferences for the marionette client.