489294 - XserverRegion leak with composited plugins

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/file/176afb7235f8/modules/plugin/base/src/nsPluginNativeWindowGtk2.cpp#l169

IIUC there should be a matching XFixesDestroyRegion.

"parts" isn't actually used, but as repair = None causes the Damage to be reset to empty, perhaps parts (rather than ev->area) should be invalidated rather than ev->area?

Or is it better to pass ev->area as repair?

Comment 1

[Jeff Muizelaar [:jrmuizel]]

I think we should just be doing XDamageSubtract (GDK_DISPLAY(), native_window->mDamage, None, None); and get rid of parts all together.

Comment 2

[Karl Tomlinson (:karlt)]

If the plugin were to draw to region A, then an event will be generated with
ev->area containing A.

If the plugin later draws to region B (where B - A is not empty) before the
event for A is processed, then no Damage event would be generated with
DamageReportNonEmpty.

If we later reset the Damage to empty (without updating B), then we wouldn't
ever get any notification that B needs updating.

Our options seem to be:

1) Deal with (invalidate) all of the Damage region using parts.  This would
   require a round trip from the server to get the bounds of the parts
   XserverRegion.
   (Maybe there are other round trips involved anyway - I don't know.)

2) Pass ev->area as repair (and parts can be None) to XDamageSubtract.  The
   XServer will then generate another event for whatever was left in Damage.

3) Use DamageReportDeltaRectangles instead of DamageReportNonEmpty.  We could
   then pass None as repair (and None as parts) to XDamageSubtract because the
   XServer will have already generated an event for B.

Option 3 seems to be the best way of getting the areas we need from the server
in a non-blocking but prompt manner.  It would generate more events, but
processing events should be fast.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: get rid of parts altogether

I tried DamageReportDeltaRectangles, but the painting code seems to get confused with small dirty regions with offsets.
With DamageReportNonEmpty, the ev->area seems to always cover the whole window.  I don't know why that is, or whether we can rely on it, but it works around the issue in comment 2.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 376538 [details] [diff] [review]
get rid of parts altogether

Looks fine

Comment 5

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/6b229c0840fa