Closed Bug 1436510 Opened 4 years ago Closed 4 years ago

Crash in PLDHashTable::Search | mozilla::FrameLayerBuilder::DrawPaintedLayer

Categories

(Core :: Web Painting, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: marcia, Assigned: mattwoodrow)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-6326b995-dfc6-4a67-afbe-0a7980180206.
=============================================================

Seen while looking at nightly crash stats: http://bit.ly/2sf3JB9. Seems to affect Windows and Linux only. 19 crashes/11 installs. Crashes started using 20180123102017.

Possible regression range based on Build ID: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5faab9e619901b1513fd4ca137747231be550def&tochange=c5461973d6ee7845b3f560c05e1502429fd63184

Top 10 frames of crashing thread:

0 xul.dll PLDHashTable::Search xpcom/ds/PLDHashTable.cpp:538
1 xul.dll mozilla::FrameLayerBuilder::DrawPaintedLayer layout/painting/FrameLayerBuilder.cpp:6112
2 xul.dll mozilla::layers::BasicPaintedLayer::PaintThebes gfx/layers/basic/BasicPaintedLayer.cpp:94
3 xul.dll mozilla::layers::BasicLayerManager::PaintSelfOrChildren gfx/layers/basic/BasicLayerManager.cpp:708
4 xul.dll mozilla::layers::BasicLayerManager::PaintLayer gfx/layers/basic/BasicLayerManager.cpp:891
5 xul.dll mozilla::layers::BasicLayerManager::PaintSelfOrChildren gfx/layers/basic/BasicLayerManager.cpp:731
6 xul.dll mozilla::layers::BasicLayerManager::PaintLayer gfx/layers/basic/BasicLayerManager.cpp:891
7 xul.dll mozilla::layers::BasicLayerManager::EndTransactionInternal gfx/layers/basic/BasicLayerManager.cpp:616
8 xul.dll RegularFramePaintCallback::Paint layout/svg/nsSVGIntegrationUtils.cpp:424
9 xul.dll nsFilterInstance::BuildSourceImage layout/svg/nsFilterInstance.cpp:489

=============================================================
Display list optimization in that potential regression range, but also no crashes in a few days.
Flags: needinfo?(matt.woodrow)
It looks like  aLayer->Manager()->GetLayerBuilder() is returning nullptr.

I can't see how that would happen though, except if we recursed into painting or similar.

Seems to be restricted to filters too.

Very low volume, so not super worried about this right now.
Flags: needinfo?(matt.woodrow)
Priority: -- → P3
Fix-optional for 60 based on volume, per triage team.
Duplicate of this bug: 1441302
Copying over the STR from bug 1441302:

Steps to reproduce:

1) Type in a file URL of an SVG file. The one that I used is:
   file:///C:/mozilla-central/toolkit/themes/shared/incontent-icons/blocked.svg
2) Right click anywhere on the page and click "Inspect Element"
3) Wait for about 2 seconds

Expected result:
  I can inspect the elements on the page.
Actual result:
  The content process crashes.

On my machine, I can reproduce this on remote URLs as well, but it only seems to happen after I have explore through the DOM tree a bit. On a different machine, it seems that remote URLs do not cause the same problem at all.




Note: I can't reproduce the issue myself (Linux / Nightly 20180226230123) :/
Matt, Marcia, looks like there were a lot more reports lately, according to crash-reports. You may want to bump up the priority.
Flags: needinfo?(mozillamarcia.knous)
Looks like in yesterday's build there were 35 crashes, overall over 100 in the last 7 days. I also changed the platform since the crashes are not Windows only on nightly.
Flags: needinfo?(mozillamarcia.knous)
OS: Windows 10 → All
Hardware: Unspecified → All
I managed to reproduce this consistently on https://www.usnews.com/news/best-states/rankings

The issue is that nsDisplayPerspective is made unique by adding an index that increments each time we create one.

When doing a partial build with retained-dl, we iterate a subset, and the indexes no longer line up.

We should just give up on partial builds with perspective, until bug 1431249 fixes the underlying sadness.
Assignee: nobody → matt.woodrow
Attachment #8954597 - Flags: review?(mikokm)
Comment on attachment 8954597 [details] [diff] [review]
Rebuild all items within perspective

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

LGTM.

::: layout/generic/nsFrame.cpp
@@ +2827,5 @@
>        aBuilder->MarkFrameModifiedDuringBuilding(this);
>      }
>    }
>  
> +  if (aBuilder->IsRetainingDisplayList() &&

Could you add a comment about why we are doing this?
And maybe a reference to bug 1431249 with the hope that this code is some day removed.
Attachment #8954597 - Flags: review?(mikokm) → review+
Comment on attachment 8954597 [details] [diff] [review]
Rebuild all items within perspective

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

LGTM.

::: layout/generic/nsFrame.cpp
@@ +2827,5 @@
>        aBuilder->MarkFrameModifiedDuringBuilding(this);
>      }
>    }
>  
> +  if (aBuilder->IsRetainingDisplayList() &&

Could you add a comment about why we are doing this?
And maybe a reference to bug 1431249 with the hope that this code is some day removed.

@@ +2828,5 @@
>      }
>    }
>  
> +  if (aBuilder->IsRetainingDisplayList() &&
> +      ChildrenHavePerspective()) {

Please use ChildrenHavePerspective(disp) here to avoid unnecessary StyleDisplay() call.
Just for reference, I think that after this patch, the changes in bug 1416991 are not needed anymore.
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1fe29595995
Don't attempt partial display list building within perspective as we need to build all children to get consistent indices. r=miko
https://hg.mozilla.org/mozilla-central/rev/e1fe29595995
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
This brought in some performance improvements:

== Change summary for alert #11877 (as of Thu, 01 Mar 2018 01:54:12 GMT) ==

Improvements:

  3%  displaylist_mutate windows10-64 pgo e10s stylo     6,744.11 -> 6,547.69
  2%  displaylist_mutate linux64 pgo e10s stylo          3,077.69 -> 3,015.95

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11877
You need to log in before you can comment on or make changes to this bug.