Closed
Bug 513469
Opened 15 years ago
Closed 15 years ago
Don't use nsIBrowserInstance in Firefox
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta1-fixed |
People
(Reporter: asqueella, Assigned: asqueella)
References
Details
(Keywords: perf)
Attachments
(1 file)
(deleted),
patch
|
Gavin
:
review+
beltzner
:
approval1.9.2+
|
Details | Diff | Splinter Review |
From the now-closed bug 46200:
------- Comment #102 From Nickolay_Ponomarev 2009-07-18 12:09:10 PDT
So...
http://mxr.mozilla.org/mozilla-central/search?string=nsibrowserinstance and
http://mxr.mozilla.org/mozilla-central/search?string=component/browser/instance
only list the interface/component used in (Firefox's) browser.js
In browser.js the component is created only once and stored in a global
variable appCore. http://mxr.mozilla.org/mozilla-central/search?string=appCore
shows this variable is only used in browser.js:
# line 924 -- appCore.startPageCycler();
-- #ifdef'ed ENABLE_PAGE_CYCLER -- I don't see this being defined anywhere.
Plus anyway nsBrowserInstance::StartPageCycler is a no-op.
# line 1107 -- appCore =
Components.classes["@mozilla.org/appshell/component/browser/instance;1"]
# line 1145 -- appCore.setWebShellWindow(window);
-- startup initialization. The component stores references to the chrome
|window| and the content window, but doesn't appear to do anything else useful.
Also has some debug code, which doesn't appear useful.
# line 1433 -- if (appCore)
# line 1434 -- appCore.close();
-- on shutdown. Doesn't do anything useful (sets a flag, which doesn't seem to
affect anything).
Looks like it can be killed. Anything I missed?
------
It should be possible to remove the component and the interface both from Firefox and mozilla-central, but removing it from Firefox requires packaging changes which tend to cause pain and suffering, and removing it from mozilla-central requires fixing up comm-central, which still uses the component.
Removing references to the component from browser.js is easy though, so let's do it first.
Assignee | ||
Comment 1•15 years ago
|
||
Attachment #397453 -
Flags: review?(gavin.sharp)
Updated•15 years ago
|
Attachment #397453 -
Flags: review?(gavin.sharp) → review+
Comment 2•15 years ago
|
||
Comment on attachment 397453 [details] [diff] [review]
remove nsIBrowserInstance usage from browser.js
Might even be a nice Ts win!
Updated•15 years ago
|
Keywords: checkin-needed
Updated•15 years ago
|
Keywords: checkin-needed → 4xp
Assignee | ||
Comment 3•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Attachment #397453 -
Flags: approval1.9.2?
Comment 4•15 years ago
|
||
Comment on attachment 397453 [details] [diff] [review]
remove nsIBrowserInstance usage from browser.js
a192=beltzner (did it turn out to be a Ts win? if so, can someone whiteboard this with [ts]?)
Attachment #397453 -
Flags: approval1.9.2? → approval1.9.2+
Comment 5•15 years ago
|
||
status1.9.2:
--- → beta1-fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•