UX branch: crashes on 10.6 in browser_fullscreen-window-open.js | application crashed [@ mozalloc_abort(char const*)] | application crashed [@ gfxQuartzNativeDrawing::~gfxQuartzNativeDrawing()]

RESOLVED FIXED in Firefox 28

Status

()

defect
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Gijs, Assigned: mstange)

Tracking

({crash, intermittent-failure})

unspecified
mozilla28
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox26 unaffected, firefox27 unaffected, firefox28 fixed, firefox-esr24 unaffected)

Details

(Whiteboard: [Australis:M9][Australis:P1][fixed-in-ux])

Attachments

(1 attachment)

Reporter

Description

6 years ago
Started on:

https://tbpl.mozilla.org/php/getParsedLog.php?id=29986964&tree=UX

https://tbpl.mozilla.org/?tree=UX&rev=644d2d08eb58

This is an m-c merge cset, but mconley and I were thinking this is probably our fancy window buttons stuff. :-\

Seems to be 10.6 only so far.
Reporter

Comment 1

6 years ago
(there were no conflicts I had to resolve for this merge, although that might not mean everything. Something did touch cocoa's nsChildView.mm as well as nsDisplayList.cpp .

Pushlog: https://hg.mozilla.org/projects/ux/pushloghtml?startID=583&endID=584
> Started on:
> 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=29986964&tree=UX
> 
> https://tbpl.mozilla.org/?tree=UX&rev=644d2d08eb58

How can you tell?  I don't see any crashes at Socorro whose signatures contain "gfxQuartzNativeDrawing".
Reporter

Comment 3

6 years ago
(In reply to Steven Michaud from comment #2)
> > Started on:
> > 
> > https://tbpl.mozilla.org/php/getParsedLog.php?id=29986964&tree=UX
> > 
> > https://tbpl.mozilla.org/?tree=UX&rev=644d2d08eb58
> 
> How can you tell?  I don't see any crashes at Socorro whose signatures
> contain "gfxQuartzNativeDrawing".

Uhh, it's the stack in that log? These appear on TBPL test runs.
(Following up comment #2)

Never mind.  Now I see these are from test failures.
Reporter

Comment 5

6 years ago
(In reply to :Gijs Kruitbosch from comment #1)
> (there were no conflicts I had to resolve for this merge, although that
> might not mean everything. Something did touch cocoa's nsChildView.mm as
> well as nsDisplayList.cpp .
> 
> Pushlog: https://hg.mozilla.org/projects/ux/pushloghtml?startID=583&endID=584

It seems all changes to those two files were backed out. I do see this when diffing all of widget/cocoa:

-LDFLAGS	+= \
-	-framework QuickTime \
-	-framework IOKit \
-	-F/System/Library/PrivateFrameworks -framework CoreUI \
-	$(NULL)

There's no corresponding + change in widget/cocoa. This was part of the changes from bug 933071. I... would have expected this to not compile at all if it were a problem, but someone who understands this code better than I do would probably know better.
Reporter

Updated

6 years ago
Severity: normal → critical
Keywords: crash
Reporter

Comment 6

6 years ago
(In reply to Nathan Froyd (:froydnj) from bug 933071 comment #1)
> Created attachment 825293 [details] [diff] [review]
> add --with-macos-private-frameworks to support cross-compiling
> 
> With this flag, I can get libxul to link.  The widget/cocoa changes are a
> drive-by, but
> they appear totally unused in the current build configuration.

Hmm. So let's at least try what happens when we back this out: https://tbpl.mozilla.org/?tree=Try&rev=a9b845ee682b
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Reporter

Comment 9

6 years ago
(In reply to :Gijs Kruitbosch from comment #6)
> (In reply to Nathan Froyd (:froydnj) from bug 933071 comment #1)
> > Created attachment 825293 [details] [diff] [review]
> > add --with-macos-private-frameworks to support cross-compiling
> > 
> > With this flag, I can get libxul to link.  The widget/cocoa changes are a
> > drive-by, but
> > they appear totally unused in the current build configuration.
> 
> Hmm. So let's at least try what happens when we back this out:
> https://tbpl.mozilla.org/?tree=Try&rev=a9b845ee682b

So this (by itself) wasn't it. Bisecting the various merges to slim down the ~180 cset range to something more workable:

https://tbpl.mozilla.org/?tree=Try&rev=32ef841d86b8 (for https://tbpl.mozilla.org/?rev=5297b280ebeb )
https://tbpl.mozilla.org/?tree=Try&rev=23511f209d6b (for https://tbpl.mozilla.org/?rev=b199e59e7f6d )
https://tbpl.mozilla.org/?tree=Try&rev=bd0fb94c326f (for https://tbpl.mozilla.org/?rev=5bb07c1ae9f5 )

(I didn't bother doing another merge of tip at the same point as the original merge, which was definitely orange)
Reporter

Comment 10

6 years ago
(In reply to :Gijs Kruitbosch from comment #9)
> https://tbpl.mozilla.org/?tree=Try&rev=32ef841d86b8 (for
> https://tbpl.mozilla.org/?rev=5297b280ebeb )

All of these went orange, so the first (the above) apparently broke this. Pushlog:

https://hg.mozilla.org/mozilla-central/pushloghtml?startID=25563&endID=25564
Reporter

Comment 11

6 years ago
Try bisecting party goes on:

https://tbpl.mozilla.org/?tree=Try&rev=d5eebae9bafd for https://hg.mozilla.org/mozilla-central/rev/885ec75e5600

https://tbpl.mozilla.org/?tree=Try&rev=21c8cd0e2230 for https://hg.mozilla.org/mozilla-central/rev/4d642c4a393c

A quarter of the csets after that have been backed out before the merge so I'm not going any further before these two results are in (I suspect it isn't the first bit of that push, ie I hope the first one at least will come back green, but I'm not sure).
Reporter

Comment 12

6 years ago
(In reply to :Gijs Kruitbosch from comment #11)
> Try bisecting party goes on:
> 
> https://tbpl.mozilla.org/?tree=Try&rev=d5eebae9bafd for
> https://hg.mozilla.org/mozilla-central/rev/885ec75e5600
> 
> https://tbpl.mozilla.org/?tree=Try&rev=21c8cd0e2230 for
> https://hg.mozilla.org/mozilla-central/rev/4d642c4a393c
> 
> A quarter of the csets after that have been backed out before the merge so
> I'm not going any further before these two results are in (I suspect it
> isn't the first bit of that push, ie I hope the first one at least will come
> back green, but I'm not sure).

So looks like it *is* the first one of those. :-(
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Reporter

Comment 16

6 years ago
Regression range on inbound: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d73c866f9c0c&tochange=9ca072ad958c

(Top 20 stack frames, because I forgot it all this time:

15:46:56     INFO -   0  XUL!gfxQuartzNativeDrawing::~gfxQuartzNativeDrawing() [BorrowedContext.h:d7425c9884fb : 118 + 0x0]
15:46:56     INFO -      rbx = 0x00007fff7031d2f8   r12 = 0x000000014a2d4db0
15:46:56     INFO -      r13 = 0x000000014d19b740   r14 = 0x00000001283d7a90
15:46:56     INFO -      r15 = 0x00000001208b0720   rip = 0x0000000101d9ac99
15:46:56     INFO -      rsp = 0x00007fff5fbf87d0   rbp = 0x00007fff5fbf87e0
15:46:56     INFO -      Found by: given as instruction pointer in context
15:46:56     INFO -   1  XUL!nsNativeThemeCocoa::DrawWidgetBackground(nsRenderingContext*, nsIFrame*, unsigned char, nsRect const&, nsRect const&) [gfxQuartzNativeDrawing.h:d7425c9884fb : 16 + 0x4]
15:46:56     INFO -      rbx = 0x0000000000000001   r12 = 0x000000014a2d4db0
15:46:56     INFO -      r13 = 0x000000014d19b740   r14 = 0x00000001283d7a90
15:46:56     INFO -      r15 = 0x00000001208b0720   rip = 0x0000000102d48de4
15:46:56     INFO -      rsp = 0x00007fff5fbf87f0   rbp = 0x00007fff5fbf8d60
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -   2  XUL!nsDisplayThemedBackground::PaintInternal(nsDisplayListBuilder*, nsRenderingContext*, nsRect const&, nsRect*) [nsDisplayList.cpp:d7425c9884fb : 2374 + 0x24]
15:46:56     INFO -      rbx = 0x00007fff5fbf8d80   r12 = 0x000000012aa23250
15:46:56     INFO -      r13 = 0x00000001208b07f8   r14 = 0x00000001045e5ca2
15:46:56     INFO -      r15 = 0x00007fff5fbf8d90   rip = 0x0000000101c69b9c
15:46:56     INFO -      rsp = 0x00007fff5fbf8d70   rbp = 0x00007fff5fbf8de0
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -   3  XUL!mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*) [FrameLayerBuilder.cpp:d7425c9884fb : 3303 + 0x19]
15:46:56     INFO -      rbx = 0x000000014a2d4db0   r12 = 0x0000000120524220
15:46:56     INFO -      r13 = 0x0000000000000000   r14 = 0x0000000120524208
15:46:56     INFO -      r15 = 0x000000012aa23220   rip = 0x0000000101c1afcf
15:46:56     INFO -      rsp = 0x00007fff5fbf8df0   rbp = 0x00007fff5fbf9190
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -   4  XUL!mozilla::layers::ClientThebesLayer::PaintBuffer(gfxContext*, nsIntRegion const&, nsIntRegion const&, nsIntRegion const&, bool) [ClientThebesLayer.cpp:d7425c9884fb : 166 + 0xd]
15:46:56     INFO -      rbx = 0x0000000151ebdf40   r12 = 0x00007fff5fbf9260
15:46:56     INFO -      r13 = 0x000000014a2d4db0   r14 = 0x00007fff5fbf92e8
15:46:56     INFO -      r15 = 0x0000000151ebe130   rip = 0x000000010364baeb
15:46:56     INFO -      rsp = 0x00007fff5fbf91a0   rbp = 0x00007fff5fbf9220
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -   5  XUL!mozilla::layers::ClientThebesLayer::PaintThebes() [ClientThebesLayer.cpp:d7425c9884fb : 107 + 0x4]
15:46:56     INFO -      rbx = 0x0000000149b7f2e8   r12 = 0x0000000151ef0dc8
15:46:56     INFO -      r13 = 0x000000014a2d4db0   r14 = 0x0000000151ebdf40
15:46:56     INFO -      r15 = 0x0000000151ebdf40   rip = 0x000000010364b8d9
15:46:56     INFO -      rsp = 0x00007fff5fbf9230   rbp = 0x00007fff5fbf93b0
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -   6  XUL!mozilla::layers::ClientThebesLayer::RenderLayer() [ClientThebesLayer.cpp:d7425c9884fb : 140 + 0x7]
15:46:56     INFO -      rbx = 0x000000011f31b1a0   r12 = 0x0000000151ef0dc8
15:46:56     INFO -      r13 = 0x00007fff5fbf94e0   r14 = 0x0000000151ebdf40
15:46:56     INFO -      r15 = 0x00007fff5fbf9408   rip = 0x000000010364bd57
15:46:56     INFO -      rsp = 0x00007fff5fbf93c0   rbp = 0x00007fff5fbf93e0
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -   7  XUL!mozilla::layers::ClientContainerLayer::RenderLayer() [ClientContainerLayer.h:d7425c9884fb : 81 + 0x5]
15:46:56     INFO -      rbx = 0x0000000000000000   r12 = 0x0000000151ef0dc8
15:46:56     INFO -      r13 = 0x00007fff5fbf94e0   r14 = 0x0000000000000000
15:46:56     INFO -      r15 = 0x00007fff5fbf9408   rip = 0x0000000103649148
15:46:56     INFO -      rsp = 0x00007fff5fbf93f0   rbp = 0x00007fff5fbf94c0
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -   8  XUL!mozilla::layers::ClientLayerManager::EndTransactionInternal(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) [ClientLayerManager.cpp:d7425c9884fb : 187 + 0x9]
15:46:56     INFO -      rbx = 0x0000000151ef0b40   r12 = 0x0000000151ef0dc8
15:46:56     INFO -      r13 = 0x00007fff5fbf94e0   r14 = 0x0000000103648cc0
15:46:56     INFO -      r15 = 0x000000014bad7f10   rip = 0x000000010364a6ea
15:46:56     INFO -      rsp = 0x00007fff5fbf94d0   rbp = 0x00007fff5fbf9550
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -   9  XUL!mozilla::layers::ClientLayerManager::EndTransaction(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) [ClientLayerManager.cpp:d7425c9884fb : 210 + 0xd]
15:46:56     INFO -      rbx = 0x000000014bad7f10   r12 = 0x0000000101c1a2e0
15:46:56     INFO -      r13 = 0x000000014bad7f10   r14 = 0x0000000000000002
15:46:56     INFO -      r15 = 0x00007fff5fbf9950   rip = 0x000000010364a7aa
15:46:56     INFO -      rsp = 0x00007fff5fbf9560   rbp = 0x00007fff5fbf9580
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -  10  XUL!nsDisplayList::PaintForFrame(nsDisplayListBuilder*, nsRenderingContext*, nsIFrame*, unsigned int) const [nsDisplayList.cpp:d7425c9884fb : 1277 + 0x1b]
15:46:56     INFO -      rbx = 0x0000000000000002   r12 = 0x0000000151ef0b40
15:46:56     INFO -      r13 = 0x000000014bad7f10   r14 = 0x00007fff5fbf9950
15:46:56     INFO -      r15 = 0x0000000000000000   rip = 0x0000000101c6437e
15:46:56     INFO -      rsp = 0x00007fff5fbf9590   rbp = 0x00007fff5fbf9760
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -  11  XUL!nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) const [nsDisplayList.cpp:d7425c9884fb : 1133 + 0x10]
15:46:56     INFO -      rbx = 0x00007fff5fbf9950   r12 = 0x00007fff5fbf9928
15:46:56     INFO -      r13 = 0x000000011ec45420   r14 = 0x000000000000000d
15:46:56     INFO -      r15 = 0x0000000000000000   rip = 0x0000000101c63a5a
15:46:56     INFO -      rsp = 0x00007fff5fbf9770   rbp = 0x00007fff5fbf97a0
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -  12  XUL!nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) [nsLayoutUtils.cpp:d7425c9884fb : 2266 + 0xb]
15:46:56     INFO -      rbx = 0x0000000000000001   r12 = 0x00007fff5fbf9ea0
15:46:56     INFO -      r13 = 0x000000011ec45420   r14 = 0x00000000ffffff00
15:46:56     INFO -      r15 = 0x0000000000000000   rip = 0x0000000101c9a743
15:46:56     INFO -      rsp = 0x00007fff5fbf97b0   rbp = 0x00007fff5fbf9f30
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -  13  XUL!PresShell::Paint(nsView*, nsRegion const&, unsigned int) [nsPresShell.cpp:d7425c9884fb : 5674 + 0x13]
15:46:56     INFO -      rbx = 0x000000011ec45420   r12 = 0x000000015859aa50
15:46:56     INFO -      r13 = 0x0000000000000304   r14 = 0x000000014bad7f10
15:46:56     INFO -      r15 = 0x0000000000000001   rip = 0x0000000101cc5ae3
15:46:56     INFO -      rsp = 0x00007fff5fbf9f40   rbp = 0x00007fff5fbfa0f0
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -  14  XUL!nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) [nsViewManager.cpp:d7425c9884fb : 420 + 0x11]
15:46:56     INFO -      rbx = 0x000000015859aa50   r12 = 0x000000014f444c50
15:46:56     INFO -      r13 = 0x00007fff5fbfa148   r14 = 0x00000001045e5ca2
15:46:56     INFO -      r15 = 0x00007fff5fbfa118   rip = 0x00000001024005fd
15:46:56     INFO -      rsp = 0x00007fff5fbfa100   rbp = 0x00007fff5fbfa180
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -  15  XUL!nsViewManager::WillPaintWindow(nsIWidget*) [nsViewManager.cpp:d7425c9884fb : 1053 + 0x10]
15:46:56     INFO -      rbx = 0x000000014f444c50   r12 = 0x000000014c8a6b60
15:46:56     INFO -      r13 = 0x0000000000000098   r14 = 0x000000014f444c50
15:46:56     INFO -      r15 = 0x000000012b61c030   rip = 0x000000010240133c
15:46:56     INFO -      rsp = 0x00007fff5fbfa190   rbp = 0x00007fff5fbfa1b0
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -  16  XUL!nsView::WillPaintWindow(nsIWidget*) [nsView.cpp:d7425c9884fb : 1037 + 0xa]
15:46:56     INFO -      rbx = 0x000000014f444c50   r12 = 0x000000014c8a6b60
15:46:56     INFO -      r13 = 0x0000000000000098   r14 = 0x000000012b61c030
15:46:56     INFO -      r15 = 0x000000011a01d210   rip = 0x00000001023ff1e6
15:46:56     INFO -      rsp = 0x00007fff5fbfa1c0   rbp = 0x00007fff5fbfa1d0
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -  17  XUL!nsChildView::WillPaintWindow() [nsChildView.mm:d7425c9884fb : 1673 + 0x4]
15:46:56     INFO -      rbx = 0x000000012b61c030   r12 = 0x000000014c8a6b60
15:46:56     INFO -      r13 = 0x0000000000000098   r14 = 0x0000000000000002
15:46:56     INFO -      r15 = 0x000000011a01d210   rip = 0x0000000102cfa970
15:46:56     INFO -      rsp = 0x00007fff5fbfa1e0   rbp = 0x00007fff5fbfa200
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -  18  XUL!-[ChildView viewWillDraw] [nsChildView.mm:d7425c9884fb : 3701 + 0x8]
15:46:56     INFO -      rbx = 0x00000001045e5ca2   r12 = 0x000000014c8a6b60
15:46:56     INFO -      r13 = 0x0000000000000098   r14 = 0x0000000000000002
15:46:56     INFO -      r15 = 0x000000011a01d210   rip = 0x0000000102d0524a
15:46:56     INFO -      rsp = 0x00007fff5fbfa210   rbp = 0x00007fff5fbfa2e0
15:46:56     INFO -      Found by: call frame info
15:46:56     INFO -  19  AppKit + 0xf8e94
15:46:56     INFO -      rbx = 0x000000014c8a6b60   r12 = 0x0000000000000001
15:46:56     INFO -      r13 = 0x0000000000000000   r14 = 0x0000000000000002
15:46:56     INFO -      r15 = 0x000000011a01d210   rip = 0x00007fff80717e95
15:46:56     INFO -      rsp = 0x00007fff5fbfa2f0   rbp = 0x00007fff5fbfa470
15:46:56     INFO -      Found by: call frame info

)
Reporter

Comment 17

6 years ago
And this is possibly coincidence, but I just noticed this in the log:

15:46:37     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/general/browser_fullscreen-window-open.js | Opened Window is expected: test_open_from_chrome
15:46:37     INFO -  WARNING: NS_ENSURE_TRUE(mMutable) failed: file ../../../../netwerk/base/src/nsSimpleURI.cpp, line 264
15:46:37     INFO -  ++DOMWINDOW == 93 (0x15487cd58) [serial = 1307] [outer = 0x15446cf58]
15:46:37     INFO -  2013-11-03 15:46:37.648 firefox[891:903] -[BorderlessWindow unifiedToolbarHeight]: unrecognized selector sent to instance 0x155bf4c90
15:46:37     INFO -  Assertion failure: !cg, at ../../dist/include/mozilla/gfx/BorrowedContext.h:118

The bottom two messages don't appear in the logs before this regressed. I'm guessing the selector failure is courtesy of http://hg.mozilla.org/integration/mozilla-inbound/rev/90ca96b3e198 (Bug 932870) but I don't see how that could be causing the crash here...

OTOH, then there's a Windows change and a bunch of webidl changes, and I don't see how any of those have a hand in this one, either. :-\
Reporter

Comment 19

6 years ago
We have a winner. Boris, any idea why this broke? :-(
Blocks: 882541
Flags: needinfo?(bzbarsky)
Comment hidden (Legacy TBPL/Treeherder Robot)
Only the most general... Presumably someone is passing undefined somewhere, we used to treat that as something other than "not passed" but now we treat it as "not passed" and some code doesn't deal properly with the "not passed" case.

Now what that code is, I can't tell from the stack.  It's crashing in code that's quite far removed from DOM bindings...

This is limited to 10.6, not happening on 10.8?  I guess I could try disabling bug 882541 on an interface-by-interface and then function-by-function basis until we find the culprit.  Is https://hg.mozilla.org/projects/ux/ the repo that shows the problem?
Flags: needinfo?(bzbarsky)
Reporter

Comment 22

6 years ago
(In reply to Boris Zbarsky [:bz] from comment #21)
> Only the most general... Presumably someone is passing undefined somewhere,
> we used to treat that as something other than "not passed" but now we treat
> it as "not passed" and some code doesn't deal properly with the "not passed"
> case.
> 
> Now what that code is, I can't tell from the stack.  It's crashing in code
> that's quite far removed from DOM bindings...
> 
> This is limited to 10.6, not happening on 10.8? 

Or 10.7, no. Seems like it's 10.6 only. No idea why. :-\

> I guess I could try
> disabling bug 882541 on an interface-by-interface and then
> function-by-function basis until we find the culprit.  Is
> https://hg.mozilla.org/projects/ux/ the repo that shows the problem?

Yes.
Comment hidden (Legacy TBPL/Treeherder Robot)
So the results of those three try-builds just came back all green, which means there's some interaction between at least two of them that's causing this. :/

Next steps, bz? Did you want us to push all 3 duo combinations?
Flags: needinfo?(bzbarsky)
No, it just means that the issue was triggered by the fourth changeset from the patch, which is good: that's what comment 21 was assuming.

So I did some more try pushes:

Vanilla UX branch: https://tbpl.mozilla.org/?tree=Try&rev=7206a514d8b2
Disabling bug 882541 part 4 entirely: https://tbpl.mozilla.org/?tree=Try&rev=9dcbd7d8eb26
Disabling bug 882541 part 4 for interfaces starting a-m: https://tbpl.mozilla.org/?tree=Try&rev=e03f98f41b54

If that method works, I should be able to hunt down the relevant interface and function eventually...
Flags: needinfo?(bzbarsky)
Er, that third one disabled for interfaces _not_ starting with a-m.  So it's one of those that's responsible.
(In reply to Boris Zbarsky [:bz] from comment #27)
> Er, that third one disabled for interfaces _not_ starting with a-m.  So it's
> one of those that's responsible.

Try builds came back green for patches 2 and 3, so I guess that means that we're looking for an interface starting from n-z...
Indeed.  And I pushed https://tbpl.mozilla.org/?tree=Try&rev=d6e6c7740d11 a few hours ago to narrow that down further.
Comment hidden (Legacy TBPL/Treeherder Robot)
Reporter

Comment 31

6 years ago
(In reply to Boris Zbarsky [:bz] from comment #29)
> Indeed.  And I pushed https://tbpl.mozilla.org/?tree=Try&rev=d6e6c7740d11 a
> few hours ago to narrow that down further.

This was green too.

t-v: https://tbpl.mozilla.org/?tree=Try&rev=3747f5f5199b

w: https://tbpl.mozilla.org/?tree=Try&rev=4f1c6d21ba17

x-z: https://tbpl.mozilla.org/?tree=Try&rev=7f849b97a15b
Reporter

Comment 32

6 years ago
(In reply to :Gijs Kruitbosch from comment #31)
> (In reply to Boris Zbarsky [:bz] from comment #29)
> > Indeed.  And I pushed https://tbpl.mozilla.org/?tree=Try&rev=d6e6c7740d11 a
> > few hours ago to narrow that down further.
> 
> This was green too.
> 
> t-v: https://tbpl.mozilla.org/?tree=Try&rev=3747f5f5199b
> x-z: https://tbpl.mozilla.org/?tree=Try&rev=7f849b97a15b

These two are orange, so the interface we're looking for starts with 'w' (assuming the other one goes green - it's still running). Boris, my guess is "window" (that was converted to webidl, right?), but then again... not 100% sure how to test this.
Flags: needinfo?(bzbarsky)
More try pushes.  Doing those right now.  ;)
Flags: needinfo?(bzbarsky)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Reporter

Comment 37

6 years ago
(In reply to Boris Zbarsky [:bz] from comment #36)
> In particular, only 5 interfaces start with W and have optional arguments:
> 
> WebGLRenderingContext: https://tbpl.mozilla.org/?tree=Try&rev=ccc558ef4000
> WebSocket: https://tbpl.mozilla.org/?tree=Try&rev=372c141527f1
> WheelEvent: https://tbpl.mozilla.org/?tree=Try&rev=ad5e619862d7
> Window: https://tbpl.mozilla.org/?tree=Try&rev=a90ca05c00e5
> WorkerMessagePort: https://tbpl.mozilla.org/?tree=Try&rev=8138470ce0de

So everything went orange except the Window one which is still running. As I discussed with bz on IRC, I suspect window.open or window.getComputedStyle. Not sure how to proceed here, though.
Pushed two more try runs:

https://tbpl.mozilla.org/?tree=Try&rev=1d29175ddf22 will log the JS stack whenever a Window function is called with an explicit undefined argument.

https://tbpl.mozilla.org/?tree=Try&rev=ce3a272ba5b5 reverts bug 882541 only for window.open.
Comment hidden (Legacy TBPL/Treeherder Robot)
Reporter

Comment 40

6 years ago
(In reply to Boris Zbarsky [:bz] from comment #38)
> Pushed two more try runs:
> 
> https://tbpl.mozilla.org/?tree=Try&rev=1d29175ddf22 will log the JS stack
> whenever a Window function is called with an explicit undefined argument.
> 
> https://tbpl.mozilla.org/?tree=Try&rev=ce3a272ba5b5 reverts bug 882541 only
> for window.open.

So it's definitely window.open, as the try push with that disabled has been running for half an hour now, and broken builds error out in <10 minutes. The log info from the other test isn't super-elucidating. Mike tried taking out the last test before the breakage occurs, and that didn't seem to help.
Reporter

Comment 41

6 years ago
(In reply to :Gijs Kruitbosch from comment #40)
> Mike tried taking out the last test before the breakage occurs, and that didn't seem to help.

Of course, maybe some of the other tests pass the same undefined parameters?
So the only caller that passes undefined that I see in the log is:

2:40:03     INFO -  UNDEFINED DETECTED for argument 3 of Window.open JS Stack:
12:40:03     INFO -  0 waitForWindowOpenFromChrome(aOptions = [object Object]) ["chrome://mochitests/content/browser/browser/base/content/test/general/browser_fullscreen-window-open.js":344]
12:40:03     INFO -      this = [object Object]
12:40:03     INFO -  1 test_open_from_chrome() ["chrome://mochitests/content/browser/browser/base/content/test/general/browser_fullscreen-window-open.js":185]
12:40:03     INFO -      this = [object ChromeWindow]
12:40:03     INFO -  2 anonymous() ["chrome://mochikit/content/browser-test.js":660]
12:40:03     INFO -      this = [object Object]

which does:

  344   let testWindow = window.open(URI, message.title, message.option);

Presumably "message" has no "option" property in this case (is that an issue specific to the UX branch?).

So the net effect is that the feature string ends up being "" now whereas it ended up being "undefined" before.  So in nsGlobalWindow::OpenInternal we get:

11093   const char *options_ptr = aOptions.IsEmpty() ? nullptr : options.get();

which is then passed to the window watcher.  I'd have to check what happens to it after that, exactly...
Reporter

Comment 43

6 years ago
So in this test, we call waitForWindowOpenFromChrome ( http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/general/browser_fullscreen-window-open.js#304 ) with aOptions.message having a param ("") and a title property, and we pass the option property of it (ie undefined, *not* "") to window.open as the flags/options part of window.open(url, name, flag/options).

So this explains how we're currently hitting another code path here:

http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#11090

On line 1093, if I understood Boris correctly, we would have taken the "you passed 'undefined'" path, and left it as the string that it is, and now we're making it be nullptr, because we'll be passed empty string. So we end up passing a nullptr to the window watcher's openWindow2 as aFeatures.

Going further down the rabbit hole at the moment...
Reporter

Comment 44

6 years ago
So, as Mike just helped us find out, if we explicitly pass "" as the features argument to window.open before the merge, we crash just as hard. So AFAICT this is just an issue with the UX-specific graphics/widget/cocoa/layout/thingy code when opening a window with an empty (ie default) featurestring, from chrome, while in fullscreen.

So while this depends on bug 882541 to be exposed, it's actually a bug elsewhere.
So I'm still waiting on my try build that would tell me what happens in the build when the crash doesn't happen (<https://tbpl.mozilla.org/?tree=Try&rev=001a5ef0c1ac>), but in the crashing build (<https://tbpl.mozilla.org/?tree=Try&rev=1fd46464b2a3>) we end up with 0xffe as output from CalculateChromeFlags in the windowwatcher.  And then this happens:

20:52:14     INFO -  2013-11-05 20:52:14.084 firefox[879:903] -[BorderlessWindow unifiedToolbarHeight]: unrecognized selector sent to instance 0x11f365140
20:52:14     INFO -  2013-11-05 20:52:14.084 firefox[879:903] Mozilla has caught an Obj-C exception [NSInvalidArgumentException: -[BorderlessWindow unifiedToolbarHeight]: unrecognized selector sent to instance 0x11f365140]

followed by a ton of:

20:52:14     INFO -  Tue Nov  5 20:52:14 talos-r4-snow-075.build.scl1.mozilla.com firefox[879] <Error>: CGContextGetType: invalid context 0x0
20:52:14     INFO -  Tue Nov  5 20:52:14 talos-r4-snow-075.build.scl1.mozilla.com firefox[879] <Error>: CGContextSaveGState: invalid context 0x0
20:52:14     INFO -  Tue Nov  5 20:52:14 talos-r4-snow-075.build.scl1.mozilla.com firefox[879] <Error>: CGContextConcatCTM: invalid context 0x0
20:52:14     INFO -  Tue Nov  5 20:52:14 talos-r4-snow-075.build.scl1.mozilla.com firefox[879] <Error>: CGContextRestoreGState: invalid context 0x0

etc, etc; all sorts of CoreGraphics methods, and then finally a crash in CoreGraphics drawing code.

So at a guess, we turn on all window features because we asked for the defaults, something in the unified toolbar stuff on Mac 10.6 ends up throwing an exception as a result, that somehow leaves us in an inconsistent state in terms of gfx, and then things are bad.
And in the crashing debug build we have:

20:49:27     INFO -  2013-11-05 20:49:27.057 firefox[879:903] -[BorderlessWindow unifiedToolbarHeight]: unrecognized selector sent to instance 0x1571ad050
20:49:27     INFO -  Assertion failure: !cg, at ../../dist/include/mozilla/gfx/BorrowedContext.h:118

which crashes us, afaict, without us hitting the other messages there.

Looking at BorrowedContext.h, we're asserting in ~BorrowedCGContext.  Probably because we never called Finish() and hence never called ReturnCGContextToDrawTarget so now there's a busted draw target around (and presumably that causes the CG errors and crash in the opt builds).

Normally, Finish() would get called by gfxQuartzNativeDrawing::EndNativeDrawing, I think.  And there's totally unifiedToolbarHeight stuff in nsNativeThemeCocoa.  But gfxQuartzNativeDrawing seems to be largely used from nsObjectFrame, which shouldn't really be involved in this test or in the native theme codepaths...

Is it possible for us to hit the unifiredToolbarHeight exception when the gfxQuartzNativeDrawing is on the stack and somehow unwind without tearing it down properly?
(In reply to Boris Zbarsky [:bz] from comment #46)
> Is it possible for us to hit the unifiredToolbarHeight exception when the
> gfxQuartzNativeDrawing is on the stack and somehow unwind without tearing it
> down properly?

I believe mstange is the resident unified-toolbar expert, so maybe he can answer this.
Flags: needinfo?(mstange)
Assignee

Comment 48

6 years ago
(In reply to Boris Zbarsky [:bz] from comment #46)
> Is it possible for us to hit the unifiredToolbarHeight exception when the
> gfxQuartzNativeDrawing is on the stack and somehow unwind without tearing it
> down properly?

Yes it is! https://hg.mozilla.org/projects/ux/annotate/176f2644c521/widget/cocoa/nsNativeThemeCocoa.mm#l1988 puts gfxQuartzNativeDrawing on the stack and https://hg.mozilla.org/projects/ux/annotate/176f2644c521/widget/cocoa/nsNativeThemeCocoa.mm#l2193 calls unifiedToolbarHeight without checking whether the window is actually a ToolbarWindow.

I'll patch in a second, thanks for tracking this down!
Assignee: nobody → mstange
Flags: needinfo?(mstange)
Comment hidden (Legacy TBPL/Treeherder Robot)
Assignee

Comment 50

6 years ago
Posted patch patchSplinter Review
This should do it. To fix it properly, we should also stop using BorderlessWindows for fullscreen on 10.6. There's already a patch in bug 890997 (attachment 803077 [details] [diff] [review]) that does that, I just need to clean it up and put it up for review.
Attachment #827935 - Flags: review?(smichaud)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment on attachment 827935 [details] [diff] [review]
patch

Nice catch, Boris and Markus!
Attachment #827935 - Flags: review?(smichaud) → review+
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Reporter

Comment 56

6 years ago
https://hg.mozilla.org/mozilla-central/rev/030e365d7c26
https://hg.mozilla.org/projects/ux/rev/030e365d7c26
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M9][Australis:P1] → [Australis:M9][Australis:P1][fixed-in-ux]
Reporter

Updated

6 years ago
Depends on: 935609
Target Milestone: --- → mozilla28
Restrict Comments: true
You need to log in before you can comment on or make changes to this bug.