Closed Bug 1068626 Opened 10 years ago Closed 9 years ago

content process crash in mozilla::layers::ShadowLayerForwarder::IsSameProcess()

Categories

(Core :: Graphics: Layers, defect)

x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
e10s m4+ ---
firefox33 --- unaffected
firefox34 --- affected
firefox35 --- affected

People

(Reporter: kairo, Assigned: gw280)

References

Details

(Keywords: crash, qawanted)

Crash Data

Attachments

(2 files, 2 obsolete files)

This bug was filed from the Socorro interface and is 
report bp-27fc5b61-286f-4b01-950d-d04422140917.
=============================================================

Top frames:
0 	xul.dll 	mozilla::layers::ShadowLayerForwarder::IsSameProcess() 	gfx/layers/ipc/ShadowLayers.cpp
1 	xul.dll 	mozilla::layers::ShadowLayerForwarder::EndTransaction(nsTArray<mozilla::layers::EditReply>*, nsIntRegion const&, unsigned __int64, bool, unsigned int, bool, mozilla::TimeStamp const&, bool*) 	gfx/layers/ipc/ShadowLayers.cpp
2 	xul.dll 	mozilla::layers::ClientLayerManager::ForwardTransaction(bool) 	gfx/layers/client/ClientLayerManager.cpp
3 	xul.dll 	mozilla::layers::ClientLayerManager::EndTransaction(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) 	gfx/layers/client/ClientLayerManager.cpp
4 	xul.dll 	nsDisplayList::PaintForFrame(nsDisplayListBuilder*, nsRenderingContext*, nsIFrame*, unsigned int) 	layout/base/nsDisplayList.cpp
5 	xul.dll 	nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) 	layout/base/nsLayoutUtils.cpp
6 	xul.dll 	PresShell::Paint(nsView*, nsRegion const&, unsigned int) 	layout/base/nsPresShell.cpp
7 	xul.dll 	nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) 	view/nsViewManager.cpp
8 	xul.dll 	nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) 	view/nsViewManager.cpp
9 	xul.dll 	nsRefreshDriver::Tick(__int64, mozilla::TimeStamp) 	layout/base/nsRefreshDriver.cpp


This started rising on nightly when the e10s prompt was added, but interestingly, it show up in decent frequency on 34 Aurora as well, even though it's all content processes (with one exception I saw).

More stats and reports at https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Alayers%3A%3AShadowLayerForwarder%3A%3AIsSameProcess%28%29
Assignee: nobody → gwright
Any chance of getting steps to reproduce? I don't have URL access on crashstats
Keywords: qawanted
(In reply to George Wright (:gw280) from comment #1)
> Any chance of getting steps to reproduce? I don't have URL access on
> crashstats

Maybe, try:

1) Open a bugzilla new bug page for Core
2) go down and expand the bug flags section, tracking-e10s should be disabled.
3) change the component to General
4) click on the tracking-e10s drop down (that should now be enabled)

*crash*
Blocks: 1067962
bp-01de2359-d909-4c20-b0b8-8f6d22140924

These steps reproduce the crash for me:
1) Open an e10s window.
2) Open an RSS or Atom feed, e.g. http://limpet.net/mbrubeck/atom.xml
3) Click the "Subscribe to this feed using" drop-down menu at the top of the page.
Crash Signature: [@ mozilla::layers::ShadowLayerForwarder::IsSameProcess()] → [@ mozilla::layers::ShadowLayerForwarder::IsSameProcess()] [@ mozilla::layers::ShadowLayerForwarder::IsSameProcess() const ]
Excellent, I can reproduce this. Thanks!
Note to self (and anyone interested): this reproduces on OS X too with mbrubeck's STR in comment 3
So here is a very rough first look at getting a XUL menu sent across the bridge to the parent process and rendered there. It took quite a while to work out how everything interacted.

I think we can probably do better here. Perhaps we can send the XUL node across wholesale and just adoptNode() it in the parent document? I already tried something along those lines but it failed, and I couldn't work out why.

I'm also seeing the popup render in the wrong place. We also appear to be missing the event coming back from the parent to the child to signal which option was selected. These should be easier to do now that I have a clearer understanding of what's going on here.

Anyway, Jim, does that look like I'm going down the right path to you? Do you have any comments about whether we can just send the XUL node across in the info object from SelectContentHelper.jsm?
Attachment #8514065 - Flags: feedback?(jmathies)
Comment on attachment 8514065 [details] [diff] [review]
0001-First-attempt-at-networking-a-XUL-menu-across-to-the.patch

(In reply to George Wright (:gw280) from comment #6)
> Created attachment 8514065 [details] [diff] [review]
> 0001-First-attempt-at-networking-a-XUL-menu-across-to-the.patch
> 
> So here is a very rough first look at getting a XUL menu sent across the
> bridge to the parent process and rendered there. It took quite a while to
> work out how everything interacted.
> 
> I think we can probably do better here. Perhaps we can send the XUL node
> across wholesale and just adoptNode() it in the parent document? I already
> tried something along those lines but it failed, and I couldn't work out why.

I don't see how this would work, you're dealing with two different proccesses here, so sending a xul node wholesale doesn't seem possible. Let's ask felipe.

Generally additional code looks good except I wonder if there's a better way to do those xul namespace checks.
Attachment #8514065 - Flags: feedback?(jmathies)
Attachment #8514065 - Flags: feedback?(felipc)
Attachment #8514065 - Flags: feedback+
(In reply to Jim Mathies [:jimm] from comment #7)
> I don't see how this would work, you're dealing with two different
> proccesses here, so sending a xul node wholesale doesn't seem possible.
> Let's ask felipe.

That makes sense. I'm not too familiar with XUL so I wasn't sure if the objects just contain data or if they have other information in them about ownership and so on that would tie it to a specific process, or if there's a method to serialise a node across in the way that would work here.
This is an incremental fix to get rid of the crash (it causes nothing to paint, though, but that's better than crashing).
Attachment #8526119 - Flags: review?(jmathies)
Test case created via support question: https://support.mozilla.org/en-US/questions/1031684

When on the xml version of a rss feed the tab will crash when clicking on any portion of the Subscribe to this option: 

Rss URL: http://rss.csmonitor.com/feeds/theculture?format=xml
Let's actually check to make sure we're in e10s mode shall we?
Attachment #8526119 - Attachment is obsolete: true
Attachment #8526119 - Flags: review?(jmathies)
Attachment #8526353 - Flags: review?(jmathies)
Which patch are you planning on landing here george?
Both. The one that sends over the popups is a work-in-progress, and the one to stop it from crashing is necessary as well I think.
Comment on attachment 8526353 [details] [diff] [review]
0001-Bug-1068626-Don-t-try-to-paint-a-popup-widget-if-we-.patch

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

::: widget/PuppetWidget.cpp
@@ +761,4 @@
>  bool
>  PuppetWidget::NeedsPaint()
>  {
> +  // e10s popups are handled by the parent process, so never should be painted here

lets augment this comment with a note about how this is a temporary work around for a bad e10s crash, and add a bug reference.

I think you might want to block on the select bug until that work is complete before continuing with this.
Attachment #8526353 - Flags: review?(jmathies) → review+
Attachment #8514065 - Flags: feedback?(felipc)
Depends on: 1053981
Carrying r+ from jimm. We discussed on IRC and decided that simply adding an NS_WARNING in there and making this a non-temporary fix was desirable.
Attachment #8526353 - Attachment is obsolete: true
Attachment #8526916 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/8709542d7b66

OK, I ran it through try to make sure it passes, so we should be good now.
Flags: needinfo?(gwright)
Closing as the crash is gone, and we've concluded the solution here is to not support XUL inside content (so there will be a followup bug to move the feed subscriber UI into the chrome)
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.