The default bug view has changed. See this FAQ.

bubblemark redraws the entire page on invalidation on mobile

RESOLVED FIXED in Firefox 16

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jrmuizel, Assigned: mattwoodrow)

Tracking

(Blocks: 1 bug, {perf})

unspecified
mozilla17
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox15+ wontfix, firefox16+ fixed)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
:(
(Reporter)

Updated

5 years ago
Blocks: 771374
OS: Mac OS X → Android
Hardware: x86 → ARM
Keywords: perf
Does DLBI help here?  (Is it better on one of the nightlies that had DLBI landed?)
(Reporter)

Comment 2

5 years ago
So here's a rough idea of what's going on.

PresShell::Paint is being called with a sane region of about 56x54. However, by the time we get down to BasicTiledThebesLayer::InvalidateRegion this has expanded to 480x690.

This happens because CreateOrRecycleThebesLayer() is calling layer->InvalidateRegion(Bounds())
because mInvalidateAllThebesContent is true.

mInvalidateAllThebesContent is set by ContainerSet::SetInvalidateAllThebesContent()

Which is caused by InternalInvalidateThebesLayersInSubtree(nsImageFrame*) deleting the ThebesLayerInvalidRegionProperty()
(Reporter)

Comment 3

5 years ago
This was debugged on my single ball bubblemark: http://people.mozilla.org/~jmuizelaar/tests/bubble.html
(Assignee)

Comment 4

5 years ago
Created attachment 642838 [details] [diff] [review]
Invalidate the frame bounds instead of deleting the frame property
(Reporter)

Comment 5

5 years ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #4)
> Created attachment 642838 [details] [diff] [review]
> Invalidate the frame bounds instead of deleting the frame property

This fixes the problem.
Attachment #642838 - Flags: review+

Updated

5 years ago
Blocks: 774756
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0be368bb172
Swift and indiscriminate back-out by ehsan for build failures:
http://hg.mozilla.org/integration/mozilla-inbound/rev/cdef000532ce

tree/layout/base/FrameLayerBuilder.cpp:2253: error: 'InvalidateThebesLayerContents' was not declared in this scope

Updated

5 years ago
Duplicate of this bug: 775146

Updated

5 years ago
Blocks: 767070
This would hopefully fix bug 767070.
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a9f2d2b75d4
tracking-firefox15: ? → ---
tracking-firefox16: ? → ---
(Assignee)

Updated

5 years ago
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?
Let's see if this fixes bug 767070 before tracking.
https://hg.mozilla.org/mozilla-central/rev/8a9f2d2b75d4
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
bug 767070, comment 24 says this did fix the issue so we'll track this for branches. Please nominate for uplift.
status-firefox15: --- → affected
status-firefox16: --- → affected
tracking-firefox15: ? → +
tracking-firefox16: ? → +
How risky is this?
Pretty low I think.
Comment on attachment 642838 [details] [diff] [review]
Invalidate the frame bounds instead of deleting the frame property

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

I think this is suitable for Aurora but not Beta.
Attachment #642838 - Flags: approval-mozilla-aurora?
Comment on attachment 642838 [details] [diff] [review]
Invalidate the frame bounds instead of deleting the frame property

[Triage Comment]
Approving for Aurora 16. Do you think we can gain confidence in this patch in time to uplift to Beta, or are you suggesting we wontfix already at this point in the cycle? Thanks!
Attachment #642838 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
If mobile really has to have it for beta, I think that's OK. Otherwise I wouldn't push for it.
https://hg.mozilla.org/releases/mozilla-aurora/rev/55664fef2487
status-firefox16: affected → fixed
Depends on: 776836

Comment 20

5 years ago
Brad, Mark, what do you think of this for 15?
I'm still seeing full page invalidating with a recent changset:
https://hg.mozilla.org/mozilla-central/rev/cb0b3e605cf1
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

5 years ago
Depends on: 778701
(In reply to JP Rosevear [:jpr] from comment #20)
> Brad, Mark, what do you think of this for 15?

It's a seemingly simple change, but I think we should not uplift for 15. Let it ride.
(In reply to Mark Finkle (:mfinkle) from comment #22)
> (In reply to JP Rosevear [:jpr] from comment #20)
> > Brad, Mark, what do you think of this for 15?
> 
> It's a seemingly simple change, but I think we should not uplift for 15. Let
> it ride.

Given mfinkle and roc's risk evaluation, wontfixing for 15.
status-firefox15: affected → wontfix
Assignee: nobody → matt.woodrow

Updated

5 years ago
Depends on: 781414

Updated

5 years ago
No longer depends on: 781414
(Assignee)

Comment 24

4 years ago
This looks fine on recent nightlies, probably fixed by DLBI.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.