Closed
Bug 232351
Opened 21 years ago
Closed 21 years ago
make a stub implementation of nsIDocumentObserver
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla1.7alpha
People
(Reporter: dbaron, Assigned: dbaron)
Details
(Whiteboard: [patch])
Attachments
(2 files)
51.59 KB,
patch
|
jst
:
review+
peterv
:
superreview+
|
Details | Diff | Splinter Review |
7.33 KB,
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
I think it might be good to have a stub implementation of nsIDocumentObserver that implementors of nsIDocumentObserver within content and layout can derive from (instead of directly from nsIDocumentObserver). It would have empty implementations (using the existing macros) of all the nsIDocumentObserver methods, but the nsISupports methods would still be pure virtual. (I'd probably want to call it nsStubDocumentObserver or nsDocumentObserverStub.) This would have two advantages: * it would reduce code size * it would hopefully reduce CPU cache misses and perhaps even missed branch prediction, since (to some degree) instead of calling a bunch of different functions when looping over observers, we'd repeatedly call the same function. Thoughts?
Comment 1•21 years ago
|
||
Sounds pretty good to me.
Assignee | ||
Comment 2•21 years ago
|
||
jst also suggested the possibility of inline virtual functions on nsIDocumentObserver, but neither of us is sure exactly what that would do. We're leaning towards nsStubDocumentObserver.
Yeah, i think a nsStubDocumentObserver would be the safest too. We should still keep the macros in nsIDocumentObserver.h though, since we have documentobservers outside of gklayout, such as transformiix and inspector.
Comment 4•21 years ago
|
||
This is a total language-geek nit: nsStubDocumentObserver is slightly verb-ier than the clearly-noun-phrase nsDocumentObserverStub, and I think for concrete class names, noun phrases win. /be
Assignee | ||
Comment 5•21 years ago
|
||
Assignee | ||
Updated•21 years ago
|
Status: NEW → ASSIGNED
Whiteboard: [patch]
Target Milestone: --- → mozilla1.7alpha
Assignee | ||
Comment 6•21 years ago
|
||
Comment on attachment 140035 [details] [diff] [review] patch I think I prefer nsStubDocumentObserver -- it's no verbier than nsIDocumentObserver itself.
Attachment #140035 -
Flags: review?(jst)
Comment 7•21 years ago
|
||
Comment on attachment 140035 [details] [diff] [review] patch r=jst
Attachment #140035 -
Flags: review?(jst) → review+
Assignee | ||
Updated•21 years ago
|
Attachment #140035 -
Flags: superreview?(peterv)
Updated•21 years ago
|
Attachment #140035 -
Flags: superreview?(peterv) → superreview+
Assignee | ||
Comment 8•21 years ago
|
||
Fix checked in to trunk, 2004-01-28 13:01/02/04 -0800.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 9•21 years ago
|
||
/me would have just called it nsDocumentObserver ;-)
Assignee | ||
Comment 10•21 years ago
|
||
Assignee | ||
Updated•21 years ago
|
Attachment #140118 -
Flags: superreview?(jst)
Attachment #140118 -
Flags: review?(jst)
Comment 11•21 years ago
|
||
Comment on attachment 140118 [details] [diff] [review] missed one r+sr=jst
Attachment #140118 -
Flags: superreview?(jst)
Attachment #140118 -
Flags: superreview+
Attachment #140118 -
Flags: review?(jst)
Attachment #140118 -
Flags: review+
Assignee | ||
Comment 12•21 years ago
|
||
Comment on attachment 140118 [details] [diff] [review] missed one Checked in to trunk, 2004-01-28 17:45 -0800.
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•