Closed Bug 307547 Opened 19 years ago Closed 19 years ago

layout module double-init

Categories

(Core :: SVG, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: tor, Assigned: tor)

Details

(Keywords: assertion, fixed1.8)

Attachments

(1 file, 1 obsolete file)

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.
Attached patch avoid double-init (obsolete) — Splinter Review
Assignee: general → tor
Status: NEW → ASSIGNED
Attachment #195301 - Flags: review?(dbaron)
So what calls SVGEnabled soon enough that this matters when it needs to?
nsContentDLF::CreateInstance, a bit down the line from
nsDocShell::CreateContentViewer.
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).
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.
(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?
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.
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+
Checked in on trunk.
Attachment #195898 - Flags: approval1.8b5?
Attachment #195898 - Flags: approval1.8b5? → approval1.8b5+
Checked in on branch.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: