Closed
Bug 1015390
Opened 10 years ago
Closed 10 years ago
[SMS] Implement new startup loading events
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect, P2)
Tracking
(b2g-v2.0 fixed, b2g-v2.1 fixed)
RESOLVED
FIXED
2.1 S1 (1aug)
People
(Reporter: Eli, Assigned: steveck)
References
Details
(Keywords: perf, Whiteboard: [c=automation p= s=2014.08.01.t u=][sms-sprint-2.0S6])
Attachments
(1 file)
46 bytes,
text/x-github-pull-request
|
julienw
:
review+
Eli
:
feedback+
praghunath
:
approval-gaia-v2.0+
|
Details | Review |
+++ This bug was initially created as a clone of Bug #837668 +++ We need to measure when the app is usable by the user. For that we'll need to send an event (the moment is specific to the app) to |window| that the performance test will be able to receive. The events for implementation are outlined in bug 996038.
Reporter | ||
Comment 1•10 years ago
|
||
Bug 996038 introduces new events outlining the phases of application startup. Each of these 5 events needs to be implemented.
Summary: [SMS] "ready to use" perf measurement → [SMS] Implement new startup loading events
Reporter | ||
Comment 2•10 years ago
|
||
As an FYI, this implementation needs to land in 2.0 as it is important for meeting release performance acceptance criteria. https://wiki.mozilla.org/FirefoxOS/Performance/Release_Acceptance
Comment 3•10 years ago
|
||
I don't see where you see this on this wiki page.
Comment 4•10 years ago
|
||
Note that Bug 837666 already introduced some events. This bug should both rename them and add the new ones.
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #4) > Note that Bug 837666 already introduced some events. This bug should both > rename them and add the new ones. Since we already have PerformanceTestingHelper to dispatch the event for profiling, is there any reason that we don't dispatch these dom-loaded/interactive/... events directly, but add the listener in helper then dispatch instead?
Flags: needinfo?(felash)
Reporter | ||
Comment 6•10 years ago
|
||
The reason is that there are plans in the future to do more with these events than just performance testing, but possible indicators to the platform for other optimizations, etc. Not to mention that the helper script is used by several other applications and cuts down on the amount of duplicate code.
Flags: needinfo?(felash)
Comment 7•10 years ago
|
||
Steve, well, since that's how all other apps are doing now, let's do the same :) For example, I think the integration tests might use some of these events to know when the app is ready. Right now they can do it but they need to set mozPerfHasListener to true before and soon enough. This looked like a good idea back when Rik and I designed the API (don't do work that we don't need), but maybe it was too much complexity for a very small arguable perf win.
Assignee | ||
Comment 8•10 years ago
|
||
Hi Julien, I replace the events with new one, but I still keep the performanceTestHelper for profiling other events we want. Hi Eli, hope that I didn't misunderstand your comments, and thanks for the reply.
Attachment #8457819 -
Flags: review?(felash)
Attachment #8457819 -
Flags: feedback?(eperelman)
Comment 9•10 years ago
|
||
Comment on attachment 8457819 [details] [review] Link to github r=me with the changes I requested; if you don't agree with this let's discuss :)
Attachment #8457819 -
Flags: review?(felash) → review+
Reporter | ||
Comment 10•10 years ago
|
||
Comment on attachment 8457819 [details] [review] Link to github The only thing missing from the patch is a whilelist entry in the array at [1]. Add "sms" to that array and everything else looks good to go. [1] https://github.com/mozilla-b2g/gaia/blob/master/tests/performance/startup_events_test.js#L26
Reporter | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 11•10 years ago
|
||
Comment on attachment 8457819 [details] [review] Link to github f- until the changes in comment 10 are made.
Attachment #8457819 -
Flags: feedback?(eperelman) → feedback-
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8457819 [details] [review] Link to github Thanks for the feedback, I already applied them to the patch. Just a question about the whitelist: Do we still need the whitelistedUnifiedApps for distinct the type of lastEvent?
Attachment #8457819 -
Flags: feedback- → feedback?
Reporter | ||
Comment 13•10 years ago
|
||
Comment on attachment 8457819 [details] [review] Link to github This patch looks good. r+ In regards to the whitelistedUnifiedApps, that array will be going away once all apps have implemented the new events.
Attachment #8457819 -
Flags: feedback? → feedback+
Assignee | ||
Comment 14•10 years ago
|
||
In master: f6bb7a99803f040b811fadd5f49d0990d7eccbbf
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 15•10 years ago
|
||
Comment on attachment 8457819 [details] [review] Link to github Requesting uplift to 2.0 as it is important for meeting our release performance acceptance criteria. [Feature/regressing bug #]: bug 996038 [User impact if declined]: none [Describe test coverage new/current, TBPL]: Feature only triggers events for testing, no user-facing features or tests [Risks and why]: Low, as there are no user-perceived changes [String/UUID change made/needed]: n/a
Attachment #8457819 -
Flags: approval-gaia-v2.0?
Updated•10 years ago
|
Assignee: nobody → schung
Whiteboard: [c=automation p= s= u=] → [c=automation p= s=2014.08.01.t u=]
Updated•10 years ago
|
Blocks: sms-sprint-2.0S6
Whiteboard: [c=automation p= s=2014.08.01.t u=] → [c=automation p= s=2014.08.01.t u=][not-part-of-initial-sprint]
Updated•10 years ago
|
No longer blocks: sms-sprint-2.0S6
Whiteboard: [c=automation p= s=2014.08.01.t u=][not-part-of-initial-sprint] → [c=automation p= s=2014.08.01.t u=][sms-sprint-2.0S6]
Updated•10 years ago
|
Attachment #8457819 -
Flags: approval-gaia-v2.0? → approval-gaia-v2.0+
Comment 16•10 years ago
|
||
v2.0: https://github.com/mozilla-b2g/gaia/commit/96e8dc653b78e7cfa8ab1d473bb3f50a5144df8d
You need to log in
before you can comment on or make changes to this bug.
Description
•