Expensive is relative here since it tooks a few ms, but this hurts page start when this stuff is only really useful for a case where the application is in landscape mode and takes time to load. It worth trying to move it out of the startup path.

Yep, I timed that again on a Tarako. It's pretty terrible, ranging from a few ms to sometimes as long as 25/30ms

Is there a way to eagerly check or pass in orientation to avoid doing this every time?

If I understand apps/system/js/window_manager.js:340 correctly we could delay that a bit with no harm (unless the user is rotating its phone like crazy...).

[Tracking Requested - why for this release]:

Based on Wilson's test results, he says this is costs around 30ms. Though it is not a huge win, small costs are adding up. Thinker/Bobby: could we get an assessment on what it would take to address this bug in 2.5? All small improvements will make a big difference. Profile info on bug 1164539

If the style is always calculated before first paint, will this still be helpful for bug 1164539? Will talk to Kanru after he's back from PTO.

Profile with c++ stack traces: https://people.mozilla.org/~bgirard/cleopatra/#report=5c719c423891c6a2e7c493947f631741fed25f64

Removing the call to getComputedStyle()[1] improved first postMessage() time by 38ms. PROFILE: https://people.mozilla.org/~bgirard/cleopatra/#report=695a354d4c2580dca07f6212a6b0e77fa4fd0958 [1] https://dxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChildPreload.js#1596

AS mentioned earlier in this bug, getComputedStyle() is called on every app's launch path to determine the background-color. This background-color is then set on an element in the system app that contains the mozbrowser <iframe>. This is done so that when devices are rotated and apps take some time to reflow we can show a better color than white in the empty space in the period before the app has reflowed to fill. If this is the only use-case, it seems we could delay this work till after the launch path. I don't know enough about mozbrowser to suggest when would be a better time. I think Dale knows more about this. Dale any guidance here?

(In reply to Wilson Page [:wilsonpage] from comment #7) > Profile with c++ stack traces: > https://people.mozilla.org/~bgirard/cleopatra/ > #report=5c719c423891c6a2e7c493947f631741fed25f64 It's GetOrCreateDOMReflector() of getComputedStyle() taking time. BTW, I don't see this bug in normal application launch.

IIRC Vivien said this code isn't actual required any more and could be stripped. NI to clarify and maybe patch ;)

(In reply to Wilson Page [:wilsonpage] from comment #13) > IIRC Vivien said this code isn't actual required any more and could be > stripped. NI to clarify and maybe patch ;) Remove the code will see white screen. In bug 869903, there was a discussion which to set the default backgrdound color black, also :mwu tried to set the background color at mozbrowserfirstpaint but it failed some tests.

My plan is to remove the code from bug 869903 and fix the white screen properly. The background color of browser iframe was black, but then change to white [1], not sure about the reason. Revert it [2] back to #fff and remove the getComputedStyle() will not see bug 869903. But we need some background for the code of bug 1047829 [3], Vivien? [1] https://github.com/mozilla-b2g/gaia/commit/9e8e6d5318c31d75fc3fbeaaa0dab44297213fd8 [2] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/window.css#L66 [3] https://github.com/mozilla-b2g/gaia/commit/2e20dfb616a8ec8f01a7000dd6bdb32eb2b10dc1#diff-46b349b14289333755febebd351e16ccL35

(In reply to Ting-Yu Chou [:ting] from comment #15) > The background color of browser iframe was black, but then change to white > [1], not sure about the reason. > [1] > https://github.com/mozilla-b2g/gaia/commit/ > 9e8e6d5318c31d75fc3fbeaaa0dab44297213fd8 Ben, do you remember the reason of this change?

(In reply to Ting-Yu Chou [:ting] from comment #15) > The background color of browser iframe was black, but then change to white > [1], not sure about the reason. Revert it [2] back to #fff and remove the > getComputedStyle() will not see bug 869903. Correction: Revert it [2] back to #000.

iirc, we need a white background (and black text color) by default for pages that have no styling to display properly.

(In reply to [:fabrice] Fabrice Desré from comment #18) > iirc, we need a white background (and black text color) by default for pages > that have no styling to display properly. Then maybe some code has been changed, otherwise this should be broken once orientationchange is fired as it changes browser iframe's background color. I'll double check, thanks for the info :)

(In reply to Ting-Yu Chou [:ting] from comment #19) > (In reply to [:fabrice] Fabrice Desré from comment #18) > > iirc, we need a white background (and black text color) by default for pages > > that have no styling to display properly. > > Then maybe some code has been changed, otherwise this should be broken once > orientationchange is fired as it changes browser iframe's background color. > I'll double check, thanks for the info :) I meant mozbrowserloadend.

Double checked, Fabrice (comment 18) is right, set browser iframe background color to black will break the page without styling.

Clearing as it looks like this has ownership and details now, cheers

From LayerScope, I found the RefLayerComposite of content has rotated orientation, but its reference layer (ContainerLayerComposite) is still unrotated.

(In reply to Ting-Yu Chou [:ting] from comment #23) > From LayerScope, I found the RefLayerComposite of content has rotated > orientation, but its reference layer (ContainerLayerComposite) is still > unrotated. I've talked to :mtseng whether in this case to fill remaining area black make sense or not, it sounds ok, but there're some other cases could have similar situation, e.g., 3d transform.

(In reply to Ting-Yu Chou [:ting] from comment #24) > (In reply to Ting-Yu Chou [:ting] from comment #23) > > From LayerScope, I found the RefLayerComposite of content has rotated > > orientation, but its reference layer (ContainerLayerComposite) is still > > unrotated. > > I've talked to :mtseng whether in this case to fill remaining area black > make sense or not, it sounds ok, but there're some other cases could have > similar situation, e.g., 3d transform. I think the code can be in RefLayerComposite::RenderLayer(), but I guess it's better to check a flag that they're in different orientation instead of comparing the rect/region everytime. Bas, I am not familiar with the code, could you please suggest how should I proceed? Thank you.

Once support for background_color will land (in bug 1215077), the code that reads the background color of the page can just be removed. There will be a background-color derived from the theme_color, or from background_color if present and support is enabled.

(In reply to vnicolas from comment #26) > Created attachment 8674233 [details] [diff] [review] > stop.sending.bgcolor.on.loadend.patch > > Once support for background_color will land (in bug 1215077), the code that > reads the background color of the page can just be removed. There will be a > background-color derived from the theme_color, or from background_color if > present and support is enabled. What will be the consequences for pages that have no manifest to get a background color from?

(In reply to [:fabrice] Fabrice Desré from comment #27) > (In reply to vnicolas from comment #26) > > Created attachment 8674233 [details] [diff] [review] > > stop.sending.bgcolor.on.loadend.patch > > > > Once support for background_color will land (in bug 1215077), the code that > > reads the background color of the page can just be removed. There will be a > > background-color derived from the theme_color, or from background_color if > > present and support is enabled. > > What will be the consequences for pages that have no manifest to get a > background color from? They can always use a <meta name="theme-color" content="..."> instruction to control the background-color of this part of the UI. It is not perfect but more flexible than the one shot default background-color.

Clear NI to bas as this is resolved through another route.

This bug caused bug 1213051. That bug is a 2.5+ blocker. Vivien agrees that we should back this patch out until 1215077 lands. See https://bugzilla.mozilla.org/show_bug.cgi?id=1213051#c17

Setting NI flag for Farbrice to help with backout. Thanks

backed out in https://hg.mozilla.org/integration/b2g-inbound/rev/8a995569a4c0

c

Moving to p3 because no activity for at least 1 year(s). See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information