613480 - Check for a NULL docShell in browser constructor

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

In Firefox Mobile, where multi-process web content is used, the browser.docShell is NULL. The browser XBL constructor uses the docShell without checking it first and causes errors when run in Firefox Mobile.

This patch just adds a simple check to the constructor.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

In addition to errors reported by QA and users, this bug will keep the Mobile browser-chrome tests from ever being green, since the word "Error" is found in the browser-chrome output.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

When is the docShell initialized?

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #2)
> When is the docShell initialized?

For remote browsers? It's never available in the chrome (parent) process. Only local browsers have a docShell available to the browser binding.

For remote browsers the docShell is available to the content process scripts.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Does my answer make sense? Let me know if you need any more explanation.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

A browser binding whose .docShell is always null wouldn't be very useful, so I assumed that patch was working around the fact that the docShell is null when the constructor runs, but non-null at some later point. If you're saying instead that this.docShell is always null, then I don't understand how you wouldn't be running into a bunch of other problems...

Comment 6

[:Gavin Sharp [email: [e-mail]]]

So as explained on IRC you don't run into other problems because you're using bindings that override the base browser binding and re-implement most of its functionality. So it's just the constructor (which you can't prevent from running) that poses a problem.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 491826 [details] [diff] [review]
patch

Removes extraneous errors on console, which get reported as bugs. Also needed for Fennec to pass browser-chrome tests

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mozilla-central/rev/b5610f4fa3a8

Comment 9

[Dão Gottwald [:dao]]

bugspam