156043 - Moz cores when printing [@ nsRenderingContextGTK::Init]

Status: RESOLVED FIXED

(Core :: Printing: Output, defect, P3)

Description

[Blu3]

Linux Jul5 #8
Loaded webpage listed above.  Browsed around in it (developing the website),
returned to the main page, hit the print icon and the [Print] button.  Moz
immediate cored.

Talkback ID TB8054258H

Comment 1

[Blu3]

Finishing summary ;)

Comment 2

[Olivier Cahagne]

which build ID ?

Comment 3

[Blu3]

Linux Jul5 #8

Translated, that would be 2002070508 on Linux.

Comment 4

[adam dudek]

WFM Build #0000000000

Core didnt dump for me.

Comment 5

[Boris Zbarsky [:bzbarsky]]

From the talkback:

nsRenderingContextGTK::Init()
NewOffscreenContext()
nsViewManager::CreateBlendingBuffers()
nsViewManager::RenderViews()
nsViewManager::Display()
nsSimplePageSequenceFrame::PrintNextPage()
DocumentViewerImpl::PrintPage()
nsPagePrintTimer::Notify()
nsTimerImpl::Fire()
handleTimerEvent()
PL_HandleEvent()
PL_ProcessPendingEvents()
nsEventQueueImpl::ProcessPendingEvents()
event_processor_callback()
our_gdk_io_invoke()
libglib-1.2.so.0 + 0x108ae (0x403a98ae)
libglib-1.2.so.0 + 0x120bb (0x403ab0bb)
libglib-1.2.so.0 + 0x12596 (0x403ab596)
libglib-1.2.so.0 + 0x12834 (0x403ab834)
libgtk-1.2.so.0 + 0x9b54f (0x402bf54f)
nsAppShell::Run()
nsAppShellService::Run()
main1()
main()
libc.so.6 + 0x1c9e3 (0x404d89e3) 

Comment 6

[Roland Mainz]

*** Bug 151389 has been marked as a duplicate of this bug. ***

Comment 7

[rods (gone)]

I tried this tody with a recent build from last week and it worked fine.

Comment 8

[Roland Mainz]

rods wrote:
> I tried this tody with a recent build from last week and it worked fine

The example page (http://www.mozilla.org/projects/mathml/demo/basics.xhtml) from
bug 151389 (which is marked a DUPlicate of this bug) still crashes my
2002-07-18-08-trunk GTK+ non-debug build on Solaris/SPARC like this:
-- snip --
(dbx) where
current thread: t@1
=>[1] nsRenderingContextGTK::Init(0xf75e40, 0x7044a8, 0x0, 0x0, 0x0, 0x0), at
0xfce9efd0
  [2] NewOffscreenContext(0x7044a8, 0x0, 0xfc32f110, 0xd7a04c, 0x2665, 0x0), at
0xfc315104
  [3] nsViewManager::CreateBlendingBuffers(0xd79f80, 0xf74910, 0x0, 0xfc32f110,
0x80000000, 0xfc32dd54), at 0xfc31546c
  [4] nsViewManager::RenderViews(0xd79f80, 0x0, 0xf74910, 0xffbeea7c, 0xd79fc4,
0x1), at 0xfc314a4c
  [5] nsViewManager::Display(0xd79f80, 0xf64568, 0x0, 0x0, 0xffbeeb3c,
0xfb8d83c0), at 0xfc3192b0
  [6] nsSimplePageSequenceFrame::PrintNextPage(0xd79060, 0xd0bf30, 0x0,
0xfc2d34d0, 0xffbeeb64, 0xffbeeb6c), at 0xfc06e774
  [7] DocumentViewerImpl::PrintPage(0x6fafa8, 0xd0bf30, 0x1, 0x7d20f8,
0xffbeec64, 0x0), at 0xfc8187ac
  [8] nsPagePrintTimer::Notify(0xd24da0, 0x5bfde8, 0x20d4f8, 0x1, 0x1,
0xd0bf30), at 0xfc82c444
  [9] nsTimerImpl::Fire(0x5bfde8, 0x5bfde8, 0xac897d6b, 0xac8976f6, 0xff1dac14,
0xff1cf034), at 0xff15491c
  [10] handleTimerEvent(0x7af400, 0x400, 0x7a688, 0xff00391c, 0x0, 0xff1cf034),
at 0xff154a90
  [11] PL_HandleEvent(0x7af400, 0xff1549a0, 0x96c64, 0xff023734, 0x177f0,
0x7af400), at 0xff14e598
  [12] PL_ProcessPendingEvents(0x96c60, 0x1, 0x1, 0x0, 0x0, 0x89ea8), at
0xff14e4b0
  [13] nsEventQueueImpl::ProcessPendingEvents(0xdfa00, 0x3, 0x7f778, 0xfe8b34a8,
0x45fc4, 0xff1cf034), at 0xff14f8f4
  [14] our_gdk_io_invoke(0x2c5bf0, 0x1, 0x1c48f8, 0xfffffff8, 0x0, 0xfd8401cc),
at 0xfd83fdf8
  [15] g_io_unix_dispatch(0x2c2238, 0xffbef058, 0x1c48f8, 0x0, 0x0, 0xffbeefc0),
at 0xfe8b2dc8
  [16] g_main_dispatch(0xffbef058, 0x11cd30, 0x1, 0x1c48f8, 0xfe95155b, 0x378),
at 0xfe8b6dc8
  [17] g_main_iterate(0x1, 0x1, 0x5, 0xff3e4270, 0xfd82ea10, 0x19), at
0xfe8b7bcc
  [18] g_main_run(0x2c0cc0, 0x2c0cc0, 0x96db0, 0xfd840560, 0x3ddfc, 0x0), at
0xfe8b7f64
  [19] gtk_main(0x0, 0xff1dab00, 0xdfa00, 0x80000000, 0x111030, 0x0), at
0xfead60a0
  [20] nsAppShell::Run(0x111028, 0x21bee0, 0xfd8405f4, 0x19c68, 0x2e4f8, 0x0),
at 0xfd840634
  [21] main1(0x2, 0xfd8c38e8, 0xfd8c25a0, 0xffbef25c, 0xffbef270, 0xffbef2a4),
at 0x19c68
  [22] main(0x2, 0xffbef3a4, 0xffbef3b0, 0x66c00, 0x0, 0x684b0), at 0x1a678
-- snip --

Comment 9

[rods (gone)]

reopening...

Comment 10

[Roland Mainz]

Attachment: Reduced testcase, usage of '[class="background"]' seems to cause the crash here...

It looks that the usage of this CSS statement is causing this crash:
-- snip --
[class="background"] {
  background-image: url(http://www.mozilla.org/images/mozilla-banner.gif);
  border: 1px solid blue;
  -moz-opacity: 0.3;
}
-- snip --

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

To fix this we need to be able to create the correct offscreen rendering context
for any given device context. nsIDeviceContext does not currently provide a way
to do this. I have asked gisburn to come up with a patch for that.

Comment 12

[Roland Mainz]

Robert O'Callahan wrote:
> I have asked gisburn to come up with a patch for that.

... but I can do it only for GTK+/Xlib/Xprint/PostScript-modules.
Someone else has to care about Win32/Mac/BeOS... ;-(

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You should be able to clone the current behavior in NewOffscreenContext for
those other platforms.

Comment 14

[Roland Mainz]

Taking myself...

Comment 15

[Roland Mainz]

Attachment: Patch for 2002-07-18-08-trunk

Patch to fix the issue via implementing a new method |NS_IMETHOD 
CreateRenderingContext(nsDrawingSurface aSurface, nsIRenderingContext
*&aContext)| and using it in |NewOffscreenContext()|.
GTK+/Xlib implement the method, PostScript and Xprint return
|NS_ERROR_NOT_IMPLEMENTED| (Xprint can implement it, but this was not done yet
to keep the module functionality in sync with the PostScript module)

Comments:
- We should implement something like
|nsIDeviceContext::GetDeviceFeatureFlags()| to test if the device supports
features like "offscreen surfaces", "native widgets", "alpha blending" etc.
- The use of |do_CreateInstance(kRenderingContextCID, &rv)| in
nsDeviceContext.cpp is braindead and should be replaced with something like
|nsIDeviceContext::CreateRenderingContextInstance()| (which is implemented per
device context implementation and an equivalent to |new
nsRenderingContextXXX;|). The advantage would be that we could avoid overriding
tons of other methods for print device contexts and that we can bypass the
component manager - which is very very expensive.

Comment 16

[Roland Mainz]

Attachment: Better patch for 2002-07-18-08-trunk which uses the new |nsIDeviceContext::CreateRenderingContextInstance()|

Alternate patch which implements and uses
|nsIDeviceContext::CreateRenderingContextInstance()|.

I think the way to get the way of getting |nsIRenderingContext| objects via the
component manager should be removed in the future since it is a very good way
to shoot printing people into the feet (I'll file a bug for that once this one
has passed the r=/sr=/a=-cycle).

IMHO |nsIRenderingContext| objects must _always_ be obtained from a
|nsIDeviceContext| object to ensure that we are using the right
|nsIRenderingContext| on that device (we had much "fun" in the past (and we
still have bugs in SVG code due this xx@@@!!!) with such issues - and killing
the "get-|nsIRenderingContext|-from-component-manager"-path will automagically
kill these issues, too).

Comment 17

[Roland Mainz]

Requesting r=/sr= ...

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 92508 [details] [diff] [review]
Better patch for 2002-07-18-08-trunk which uses the new |nsIDeviceContext::CreateRenderingContextInstance()|

Instead of adding the new InitRenderingContext method to nsIDeviceContext, I
think we should remove both of the InitRenderingContext methods from the
interface.

Other than that I'm happy with the patch.

Comment 19

[Roland Mainz]

Attachment: New patch for 2002-07-18-08-trunk per roc+moz's review comments

Comment 20

[Roland Mainz]

Robert O'Callahan wrote:
> Instead of adding the new InitRenderingContext method to nsIDeviceContext, I
> think we should remove both of the InitRenderingContext methods from the
> interface.

I made the |InitRenderingContext| a private method and added a comment about
their usage.

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 92639 [details] [diff] [review]
New patch for 2002-07-18-08-trunk per roc+moz's review comments

r=roc+moz

Comment 22

[Roland Mainz]

Requesting sr= ...

Comment 23

[Alec Flett]

ok, the only issue I had is that in nsDeviceContext.h, line 148, you have to
change the "nsresult" to "NS_IMETHOD" because nsIDeviceContext declares that
method to be NS_IMETHOD. without this windows busts.

Comment 24

[Roland Mainz]

Alec Flett wrote:
> ok, the only issue I had is that in nsDeviceContext.h, line 148, you have to
> change the "nsresult" to "NS_IMETHOD" because nsIDeviceContext declares that
> method to be NS_IMETHOD. without this windows busts.

Actually |InitRenderingContext| should be removed from |nsIDeviceContext| ...
new patch in a few hours...

Comment 25

[Roland Mainz]

Attachment: New patch for 2002-07-27-08-trunk (same as previous one but fixes Win32 build bustage (per alecf's comments))

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 93207 [details] [diff] [review]
New patch for 2002-07-27-08-trunk (same as previous one but fixes Win32 build bustage (per alecf's comments))

r=roc+moz

Comment 27

[Scott Collins]

Comment on attachment 93207 [details] [diff] [review]
New patch for 2002-07-27-08-trunk (same as previous one but fixes Win32 build bustage (per alecf's comments))

sr=scc

Your patch looks reasonable.  The only thing of concern is a fact about this
class that seems to be true even before your patch, and that is that it uses a
lot of overloading of already |virtual| member functions, i.e., for
|CreateRenderingContext|.  This can lead to trouble when not all the overloaded
names are also overridden.  Be warned for the future, and try to avoid this in
future classes when possible.

Comment 28

[Scott Collins]

Comment on attachment 93207 [details] [diff] [review]
New patch for 2002-07-27-08-trunk (same as previous one but fixes Win32 build bustage (per alecf's comments))

should have put a checkmark in the box with my previous comment.

Comment 29

[Roland Mainz]

Patch checked-in into "trunk"
(http://bonsai.mozilla.org/cvsquery.cgi?module=MozillaTinderboxAll&branch=HEAD&cvsroot=/cvsroot&date=explicit&mindate=1028603760&maxdate=1028604420&who=bzbarsky%25mit.edu)
...

Comment 30

[Roland Mainz]

Marking bug as FIXED and requesting approval for the 1.0.1-branch...

Comment 31

[Roland Mainz]

Filed spin-off bug 161364 ("Print preview crashes when page has transparent
content") to catch a similar issue in print preview...

Comment 32

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 93207 [details] [diff] [review]
New patch for 2002-07-27-08-trunk (same as previous one but fixes Win32 build bustage (per alecf's comments))

Approved for 1.1 branch.  Please commit asap

Comment 33

[Boris Zbarsky [:bzbarsky]]

fixed on 1.1 branch

Comment 34

[Roland Mainz]

Filed bug 162024 ("RFE: Remove NS_RENDERING_CONTEXT_CID and related code") as
the follow-up to remove the ability to create rendering context objects without
having a matching and valid |nsIDeviceContext| for it.