956108 - [Holly] Crashing on many tests

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

During a Holly merge, I was forced to re-implement Swap in nsRegion.h due to a dependency having been backed out of Holly (See bug 942250 and bug 845874). 

Luckily, jrmuizel was around to help me do that.

Unfortunately, I think it wasn't enough - something's going horribly wrong, and we're crashing like crazy on a bunch of tests.

Here is an example trace:

10:31:48     INFO -  Operating system: Linux
10:31:48     INFO -                    0.0.0 Linux 3.2.0-23-generic-pae #36-Ubuntu SMP Tue Apr 10 22:19:09 UTC 2012 i686
10:31:48     INFO -  CPU: x86
10:31:48     INFO -       GenuineIntel family 6 model 45 stepping 7
10:31:48     INFO -       1 CPU
10:31:48     INFO -  Crash reason:  SIGSEGV
10:31:48     INFO -  Crash address: 0x50
10:31:48     INFO -  Thread 0 (crashed)
10:31:48     INFO -   0  libxul.so!RgnRectMemoryAllocator::Alloc() [nsRegion.cpp:92c48446f439 : 174 + 0x0]
10:31:48     INFO -      eip = 0xb417e09c   esp = 0xbf96f740   ebp = 0xbf96f748   ebx = 0xb6d8c4e4
10:31:48     INFO -      esi = 0xa17db450   edi = 0xbf96f838   eax = 0x0000003c   ecx = 0xb73e081c
10:31:48     INFO -      edx = 0x00000012   efl = 0x00010202
10:31:48     INFO -      Found by: given as instruction pointer in context
10:31:48     INFO -   1  libxul.so!nsRegion::SetToElements(unsigned int) [nsRegion.cpp:92c48446f439 : 287 + 0xf]
10:31:48     INFO -      eip = 0xb417e2ca   esp = 0xbf96f750   ebp = 0xbf96f778   ebx = 0xb6d8c4e4
10:31:48     INFO -      esi = 0xbf96f830   edi = 0xbf96f838
10:31:48     INFO -      Found by: call frame info
10:31:48     INFO -   2  libxul.so!nsRegion::Copy(nsRect const&) [nsRegion.cpp:92c48446f439 : 615 + 0xb]
10:31:48     INFO -      eip = 0xb417f9a9   esp = 0xbf96f780   ebp = 0xbf96f798   ebx = 0xb6d8c4e4
10:31:48     INFO -      esi = 0xbf96f7c0   edi = 0xbf96f7c0
10:31:48     INFO -      Found by: call frame info
10:31:48     INFO -   3  libxul.so!nsRegion::And(nsRegion const&, nsRect const&) [nsRegion.cpp:92c48446f439 : 741 + 0x8]
10:31:48     INFO -      eip = 0xb417ff10   esp = 0xbf96f7a0   ebp = 0xbf96f808   ebx = 0xb6d8c4e4
10:31:48     INFO -      esi = 0xbf96f830   edi = 0xbf96f7c0
10:31:48     INFO -      Found by: call frame info
10:31:48     INFO -   4  libxul.so!nsDisplayWrapList::ComputeVisibility(nsDisplayListBuilder*, nsRegion*, nsRect const&) [nsDisplayList.cpp:92c48446f439 : 2885 + 0x12]
10:31:48     INFO -      eip = 0xb4b48ca6   esp = 0xbf96f810   ebp = 0xbf96f8c8   ebx = 0xb6d8c4e4
10:31:48     INFO -      esi = 0xbf96f830   edi = 0x9283de8c
10:31:48     INFO -      Found by: call frame info
10:31:48     INFO -   5  libxul.so!nsDisplayItem::RecomputeVisibility(nsDisplayListBuilder*, nsRegion*) [nsDisplayList.cpp:92c48446f439 : 1567 + 0x1]
10:31:48     INFO -      eip = 0xb4b413b1   esp = 0xbf96f8d0   ebp = 0xbf96f978   ebx = 0xb6d8c4e4
10:31:48     INFO -      esi = 0xbf96f940   edi = 0x9283de9c
10:31:48     INFO -      Found by: call frame info
10:31:48     INFO -   6  libxul.so!nsDisplayTransform::ComputeVisibility(nsDisplayListBuilder*, nsRegion*, nsRect const&) [nsDisplayList.cpp:92c48446f439 : 4418 + 0x12]
10:31:48     INFO -      eip = 0xb4b4fb44   esp = 0xbf96f980   ebp = 0xbf96f9e8   ebx = 0xb6d8c4e4
10:31:48     INFO -      esi = 0xbf96f9a0   edi = 0xbf96f9a0
10:31:48     INFO -      Found by: call frame info
10:31:48     INFO -   7  libxul.so!nsDisplayItem::RecomputeVisibility(nsDisplayListBuilder*, nsRegion*) [nsDisplayList.cpp:92c48446f439 : 1567 + 0x1]

Comment 1

[Mike Conley (:mconley) (:⚙️)]

After speaking with jrmuizel and attacking this locally, I'm taking the following approach:

1) I'm going to backout 9e7f91032d7d from Holly. This was the revision that backed out d0255c4b2adc - so this is like a relanding of bug 845874.
2) Copy over mozilla-central's nsRegion.cpp and nsRegion.h to Holly so that they're sync'd up.
3) Disable the test_leaf_layers_partition_browser_window.xul test on Windows, which was the test that resulted in us doing the backout in the first place.

I've got a try push with those steps carried out, and it's looking pretty green so far:

https://tbpl.mozilla.org/?tree=Try&rev=dd042d07636a

jrmuizel's cool with this plan roc. Any objections?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Cc'ing other people who probably care about the Holly branch.

I'll carry out the above plan if I don't hear any objections within 24 hours.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

I'm fine with deferring to roc and jrmuizel on this.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

A slight addendum - having talked with jrmuizel, my plan for disabling the test on all versions of Windows is a bit too heavy handed. I'm going to modify the test to skip if we're running on Windows with window layer manager type != "Basic".

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Attachment: Disable test_leaf_layers_partition_browser_window.xul for d2d enabled Windows

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Disable test_leaf_layers_partition_browser_window.xul for d2d enabled Windows

This one actually works.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Ok, I went for it.

Part 1: https://hg.mozilla.org/projects/holly/rev/3ac91167007a
Part 2: https://hg.mozilla.org/projects/holly/rev/62a257df17c4
Part 3: https://hg.mozilla.org/projects/holly/rev/b82e348671c1

Comment 8

[Mike Conley (:mconley) (:⚙️)]

tbpl is coming back green - I think we're done here.