866579 - Dromaeo CSS regression from bug 860524

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Regression: Mozilla-Inbound-Non-PGO - Dromaeo (CSS) - Ubuntu HW 12.04 - 3.92% decrease
--------------------------------------------------------------------------------------
    Previous: avg 3541.793 stddev 26.417 of 12 runs up to revision fad77e38f79e
    New     : avg 3403.048 stddev 41.458 of 12 runs since revision d13fc07e67af
    Change  : -138.745 (3.92% / z=5.252)
    Graph   : http://mzl.la/ZOJ5wU

Changeset range: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fad77e38f79e&tochange=d13fc07e67af

Changesets:
  * http://hg.mozilla.org/integration/mozilla-inbound/rev/d13fc07e67af
    : Robert O'Callahan <[e-mail]> - Bug 860524. Remove hacky (and buggy) pseudo-destruction of DisplayItemClips created by GetCurrentClip; ensure that they're all destroyed properly when the arena goes away, by tracking them explicitly in nsDisplayListBuilder. r=mattwoodrow
    : http://bugzilla.mozilla.org/show_bug.cgi?id=860524

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Baseline try push: https://tbpl.mozilla.org/?tree=Try&rev=97f850c144bf
Regressing try push: https://tbpl.mozilla.org/?tree=Try&rev=4dc8404099eb

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

With proposed patch: https://tbpl.mozilla.org/?tree=Try&rev=cf28f863e559

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix crash

Fixes regression.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This patch causes leaks to be reported. The problem is that when DisplayListClipState::GetCurrentCombinedClip has to intersect two kinds of clipping, it first calls AllocateDisplayItemClip passing in the first clip and then modifies the result by intersecting with the second clip. That intersection can add rounded rects, but AllocateDisplayItemClip already decided not to put the DisplayItemClip in the cleanup list based on the fact that it didn't have rounded rects.

Modifying the result of AllocateDisplayItemClip is bogus, and AllocateDisplayItemClip should return a const pointer to fix this.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix v2

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8049e4af4809

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8049e4af4809