Spurious RenderViewImpl::OnResize seen in critical rendering path |
|||||
Issue descriptionNeed to understand why the OnResize even is even sent on Android, and whether we could remove it from the critical rendering path (or at least while the user's finger is on glass). See issue 598248 for more context.
,
Apr 3 2016
,
Apr 3 2016
I can see one https://drive.google.com/open?id=0B0w8IQeoE99FWXlla1dDVjc1c3M and two in https://drive.google.com/open?id=0B0w8IQeoE99FZHRmcTJXV2JCVFk. Both are coming off ViewMsg_Resize, which means that RenderWidgetHostImpl::WasResized is sending them. Who calls it?
,
Apr 3 2016
Yes, showing or hiding top controls causes a viewport resize. They're not spurious, they are actual resizes -- and in principle, we should even be sending events at 60fps to allow synchronized effects, and anything less is a performance hack. (Personally I think it should be a higher priority for the web platform to allow vertical-only resizes to happen cheaply, since we've known for a long time that this causes expensive work while scrolling on Android. I'd be happy if the renewed focus on scroll performance could cause that to happen.) wangxianzhu@ made a change a while ago to delay the Blink resize until user gestures had stopped. This may have regressed, if the trace is showing them in the middle of a scroll. bokan@ recently implemented a plan to alter the semantics of top controls resize to eliminate some of the costs, see Intent to Ship thread here: https://groups.google.com/a/chromium.org/forum/#!search/Top$20Controls$20Don$27t$20resize/blink-dev/BK0oHURgmJ4/PZqveT2LBAAJ . We considered suppressing onResize events as well as part of that, but since that's necessary information to layout certain things correctly, we held off on that change.
,
Apr 3 2016
"we should even be sending events at 60fps to allow synchronized effects, and anything less is a performance hack" -- No, that's not the way the blink machinery should work. These resizes should just be dirtying the bit for the next time the developer needs to read these values or when the next frame needs to be baked. Poking at Blink directly like RenderViewImpl::OnResize does is a bug.
,
Apr 3 2016
... and thanks for the link. Educating myself/catching up on this now...
,
Apr 3 2016
> No, that's not the way the blink machinery should work. These resizes should just be dirtying the bit Sounds like a great idea. I'm not committed to "events", I just wanted to rhetorically state my preference that we fix this by making resizes cheaper (ideally, even cheap enough to run at 60fps!), as opposed to sweeping the problem under the rug by throttling or delaying the frequency of the resizes.
,
Apr 4 2016
SGTM.
,
Jun 23 2016
Renaming Blink>Architecture to Blink>Internals
,
Jun 23 2017
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dglazkov@chromium.org
, Apr 3 2016