layout module double-init

RESOLVED FIXED

Status

()

Core
SVG
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: tor, Assigned: tor)

Tracking

({assertion, fixed1.8})

Trunk
x86
Windows XP
assertion, fixed1.8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

12 years ago
The change for bug 294519 causes us to assert because the checking for the
renderer causes layout to be initialized again.  This patch attempts to fix this
by unconditionally registering the types, and unregistering as soon as
SVGEnabled() gets called.
(Assignee)

Comment 1

12 years ago
Created attachment 195301 [details] [diff] [review]
avoid double-init
Assignee: general → tor
Status: NEW → ASSIGNED
Attachment #195301 - Flags: review?(dbaron)
So what calls SVGEnabled soon enough that this matters when it needs to?
(Assignee)

Comment 3

12 years ago
nsContentDLF::CreateInstance, a bit down the line from
nsDocShell::CreateContentViewer.
Attachment #195301 - Flags: review?(dbaron) → review+
(Assignee)

Updated

12 years ago
Attachment #195301 - Flags: superreview?(bzbarsky)
Comment on attachment 195301 [details] [diff] [review]
avoid double-init

No, that's too late.  By the time we're in CreateInstance, we've already
dispatched the SVG to the docshell because it claimed it could handle it.  So
that means no helper app dialog if you have no svg plugin installed...
Attachment #195301 - Flags: superreview?(bzbarsky) → superreview-
Note that what I suggested in the last sentence of bug 294519 comment 11 *may*
work, although I didn't trace through all the native component loader and
generic factory code (and it doesn't seem to be well documented).
(Assignee)

Comment 6

12 years ago
I do get a helper dialog box if the pref is off, even if a SVG file is my start
page.

RegisterDocumentFactories only gets called when the layout module is first
registered, it seems.  At least, a breakpoint in there didn't trigger when
starting up normally.
> I do get a helper dialog box if the pref is off

Probably because CreateInstance got called for a XUL window before then.  But in
an embedding build you can't depend on that.
(In reply to comment #6)
> I do get a helper dialog box if the pref is off, even if a SVG file is my
> start page.

In an app without XUL UI?

> RegisterDocumentFactories only gets called when the layout module is first
> registered, it seems.  At least, a breakpoint in there didn't trigger when
> starting up normally.

That suggests we should always call RegisterSVG or UnregisterSVG at startup --
or that we should always register and then find another way to return an error
later (and handle that error correctly) so that things work as they should.
(Assignee)

Comment 9

12 years ago
(In reply to comment #8)
> (In reply to comment #6)
> > I do get a helper dialog box if the pref is off, even if a SVG file is my
> > start page.
> 
> In an app without XUL UI?

No.

> > RegisterDocumentFactories only gets called when the layout module is first
> > registered, it seems.  At least, a breakpoint in there didn't trigger when
> > starting up normally.
> 
> That suggests we should always call RegisterSVG or UnregisterSVG at startup --
> or that we should always register and then find another way to return an error
> later (and handle that error correctly) so that things work as they should.

At what point are things set up enough that we can safely look for renderers
without causing layout to reinitialize?
(Assignee)

Comment 10

12 years ago
Created attachment 195898 [details] [diff] [review]
simpler test for just gdi+, which is all the previous code was really looking fo
Attachment #195301 - Attachment is obsolete: true
Attachment #195898 - Flags: review?(dbaron)
###!!! ASSERTION: module already initialized: '!gInitialized', file
m:/mozilla/layout/build/nsLayoutModule.cpp, line 261
Keywords: assertion
Comment on attachment 195898 [details] [diff] [review]
simpler test for just gdi+, which is all the previous code was really looking fo

r=dbaron.  (Although perhaps it's worth adding back the logic for the dialog
that was removed...)
Attachment #195898 - Flags: review?(dbaron) → review+
... which would require a boolean parameter to SVGEnabled (whether to show the
dialog) and a separate variable to cache for the GDIplus case.
(Assignee)

Comment 14

12 years ago
Comment on attachment 195898 [details] [diff] [review]
simpler test for just gdi+, which is all the previous code was really looking fo

I don't think we can restore the dialog logic, because part of getting all SVG
disable modes acting the same was to remove svg from the list of mimetypes we
handle internally, so we never know the user tried to load an SVG file.
Attachment #195898 - Flags: superreview?(bzbarsky)
Attachment #195898 - Flags: superreview?(bzbarsky) → superreview+
(Assignee)

Comment 15

12 years ago
Checked in on trunk.
(Assignee)

Updated

12 years ago
Attachment #195898 - Flags: approval1.8b5?

Updated

12 years ago
Attachment #195898 - Flags: approval1.8b5? → approval1.8b5+
(Assignee)

Comment 16

12 years ago
Checked in on branch.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.