:Gavin Sharp [email: gavin@gavinsharp.com] Reporter
	Description • 8 years ago

STR:
1) Start Nightly (I'm currently testing the Feb 25 build, built off mozilla-central c1e0d1890cfee9d86c8d566b0490053f21e0afc6) with a new profile
2) load https://500px.com/popular
3) click one of the photos to "zoom in"

Nightly hangs. At first just the content process is non-responsive (switching tabs results in content spinner), but eventually the whole app gets unresponsive too. This has also caused OS X to freak out and pop up the "System out of memory" dialog, which in one case caused all my other open apps to be paused and hang and required a reboot.

	:Gavin Sharp [email: gavin@gavinsharp.com] Reporter
	Comment 1 • 8 years ago

Seems to only be an issue on Mac, markh says Windows worked fine.

	Mark Hammond [:markh] [:mhammond]
	Comment 2 • 8 years ago

I can repro this on OSX - content process seems to grow by 4GB per photo. I can't repro it on Windows - browser stays responsive, content process memory doesn't grow significantly.

	Timothy Nikkel (:tnikkel)
	Comment 3 • 8 years ago

Only happens with e10s for me.

	Timothy Nikkel (:tnikkel)
	Comment 4 • 8 years ago

Regression window:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3f41d7d0f544ebd98273e39bd945c28878a47427&tochange=c0ba5835ca489d15f8f170d5deb01f8dad92709a

	:Gavin Sharp [email: gavin@gavinsharp.com] Reporter
	Comment 5 • 8 years ago

First bad revision:
changeset:   281578:c99e8abb5e00
user:        Matt Woodrow <mwoodrow@mozilla.com>
date:        Tue Jan 26 13:36:48 2016 +1300
summary:     Bug 1231538 - Build a ContainerLayer for position:fixed and background-attachment:fixed content. r=roc

	Jim Mathies [:jimm]
	Comment 6 • 8 years ago

[Tracking Requested - why for this release]:
regression from recent landing

	Jim Mathies [:jimm]
	Comment 7 • 8 years ago

This is waiting on a backout of bug 1231538.

	Matt Woodrow (:mattwoodrow) Assignee
	Comment 8 • 8 years ago

Backing that out will likely be difficult.

I'm on a work week this week, I'll fix this early next week.

	Benjamin Kerensa [:bkerensa]
	Comment 9 • 8 years ago

Pretty bad regression lets track this can we assign you Matt?

	Matt Woodrow (:mattwoodrow) Assignee
	Comment 10 • 8 years ago

Yes, absolutely.

	Matt Woodrow (:mattwoodrow) Assignee
	Comment 11 • 8 years ago

This is clearly wrong.

I can't see what purpose it served, and removing it doesn't make it fail any tests.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4571c99eda5b

Might be worth waiting a bit before uplifting, in case I missed a valid reason for this existing.

	Markus Stange [:mstange]
	Comment 12 • 8 years ago

Comment on attachment 8728182 [details] [diff] [review]
Don't set the visible region to the bounds

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

Yeah this seems very strange. Have you checked the history of this line?

	Matt Woodrow (:mattwoodrow) Assignee
	Comment 13 • 8 years ago

(In reply to Markus Stange [:mstange] from comment #12)
> Comment on attachment 8728182 [details] [diff] [review]
> Don't set the visible region to the bounds
> 
> Review of attachment 8728182 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Yeah this seems very strange. Have you checked the history of this line?

Yes, I wrote it for the regressing patch. I have no idea why.

	Markus Stange [:mstange]
	Comment 14 • 8 years ago

Ah, probably for background-attachment:fixed then. But if removing it doesn't fail any tests then we should be fine.

	Pulsebot
	Comment 15 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/735e3eec00ad

	Carsten Book [:Tomcat]
	Comment 16 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/735e3eec00ad

	:Felipe Gomes (needinfo for replies!)
	Comment 17 • 8 years ago

Good ol' fixing a bug with just code removal!

	Chris Peterson [:cpeterson]
	Comment 18 • 8 years ago

Matt, can we uplift this e10s fix to Beta 46? We're running an e10s experiment on Beta 46, so we want to be testing with all the e10s fixes for known issues.

	Matt Woodrow (:mattwoodrow) Assignee
	Comment 19 • 8 years ago

(In reply to Chris Peterson [:cpeterson] from comment #18)
> Matt, can we uplift this e10s fix to Beta 46? We're running an e10s
> experiment on Beta 46, so we want to be testing with all the e10s fixes for
> known issues.

46 isn't affected by this bug.

We should probably uplift to 47, but I'd rather let it sit for a bit to make sure it doesn't introduce new regressions.

	Chris Peterson [:cpeterson]
	Comment 20 • 8 years ago

Sounds good. We'll keep tracking this for 47.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 21 • 8 years ago

It's been close to a month with no regressions, should be safe to uplift to 47?

	Chris Peterson [:cpeterson]
	Comment 22 • 8 years ago

Matt, should this fix be uplifted to Aurora 47 in preparation for our e10s experiments on Beta 47?

	Matt Woodrow (:mattwoodrow) Assignee
	Comment 23 • 8 years ago

Comment on attachment 8728182 [details] [diff] [review]
Don't set the visible region to the bounds

Approval Request Comment
[Feature/regressing bug #]: Bug 1231538
[User impact if declined]: Massive memory usage (with e10s/apz) on sites with large background-attachment:fixed images.
[Describe test coverage new/current, TreeHerder]: Tested manually.
[Risks and why]: Fairly low risk, just removes some unnecessary code that the previous bug added.
[String/UUID change made/needed]: None

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 24 • 8 years ago

Comment on attachment 8728182 [details] [diff] [review]
Don't set the visible region to the bounds

Regression in e10s, Aurora47+

	Carsten Book [:Tomcat]
	Comment 25 • 8 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/80ee357092b5