488702 - Invalidation of containers is broken if the container or a descendant is filtered

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

When we invalidate a container frame, we are not taking account of filters or the container or its children. I noticed the latter from looking at the code, and discovered the former from testing.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: testcase - filter on changed container

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: tescase - filter on descendant of changed container

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: testcase - filter on ancestor of changed container (this works)

Comment 4

[Jonathan Watt [:jwatt]]

The reason that filters on children of a changed container are not taken into account during invalidation is that nsSVGOuterSVGFrame::UpdateAndInvalidateCoveredRegion calls GetCoveredRegion on the container, which does not take account of filters, and then calls FindFilterInvalidation, which only takes account of filters on the container (supposedly) and its descendants.

I'm not sure yet why a filter on the changed container itself is not taken into account. FindFilterInvalidation *should* be taking it into account, but an initial walkthrough in the debugger while running the first testcase shows that GetPropertyInternal isn't finding the filter property.

  nsPropertyTable::GetPropertyInternal
  nsPropertyTable::GetProperty
  nsIFrame::GetProperty
  nsSVGEffects::GetFilterProperty
  nsSVGEffects::GetFilterFrame
  nsSVGUtils::FindFilterInvalidation
  nsSVGOuterSVGFrame::UpdateAndInvalidateCoveredRegion
  DoApplyRenderingChangeToTree
  ApplyRenderingChangeToTree
  nsCSSFrameConstructor::ProcessRestyledFrames
  nsCSSFrameConstructor::RestyleElement
  nsCSSFrameConstructor::ProcessOneRestyle
  nsCSSFrameConstructor::ProcessPendingRestyles
  PresShell::DoFlushPendingNotifications
  PresShell::FlushPendingNotifications
  nsCSSFrameConstructor::RestyleEvent::Run

Comment 5

[Robert Longson [:longsonr]]

Does this work for you now? I think bug 536677 may have improved things.

Comment 6

[Jonathan Watt [:jwatt]]

Strangely it works if the mouse is in the content window when the invalidation occurs, but not if the mouse is outside it.

Comment 7

[Jonathan Watt [:jwatt]]

This is fixed for real by bug 738192, but I'll leave this open for now until I can get round to landing the tests.

Comment 8

[Robert Longson [:longsonr]]

Attachment: Bug 488702 - Add tests for dynamic filter invalidation

Comment 9

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/jobs?repo=try&revision=e2c33202696bcfdddb053e3c702c3ce96604d919

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c0a915fbd132
Add tests for dynamic filter invalidation r=dholbert

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/c0a915fbd132