Closed
Bug 485782
Opened 15 years ago
Closed 15 years ago
get rid of nsSupportsArray
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: Swatinem, Assigned: Swatinem)
Details
Attachments
(1 file, 3 obsolete files)
15.58 KB,
patch
|
Swatinem
:
review+
Swatinem
:
superreview+
|
Details | Diff | Splinter Review |
None of the users relied on the scriptability anyway so I rewrote them to use TArray+COMPtr instead. I wonder if there are more DeCOM opportunities there. SupportsArray has a internal storage for 8 Elements. I'm not quite sure what impact that will have.
Attachment #369879 -
Flags: superreview?(benjamin)
Attachment #369879 -
Flags: review?(jonas)
Attachment #369879 -
Flags: review?(jonas) → review+
All these uses look like they should be fine with no longer having internal storage.
Comment 2•15 years ago
|
||
I believe the decision from md.platform was to use nsCOMArray, not nsTArray<nsCOMPtr<nsIInterface> >, at least until codesize effects were thoroughly researched.
Updated•15 years ago
|
Attachment #369879 -
Flags: superreview?(benjamin) → superreview-
Assignee | ||
Comment 3•15 years ago
|
||
This time using nsCOMArray.
Attachment #369879 -
Attachment is obsolete: true
Attachment #372198 -
Flags: superreview?(benjamin)
Attachment #372198 -
Flags: review+
Assignee | ||
Comment 4•15 years ago
|
||
COMArrays indizes are signed. This patch takes more care of that to avoid some un/signed comparison warnings.
Attachment #372198 -
Attachment is obsolete: true
Attachment #372199 -
Flags: superreview?(benjamin)
Attachment #372199 -
Flags: review+
Attachment #372198 -
Flags: superreview?(benjamin)
Updated•15 years ago
|
Attachment #372199 -
Flags: superreview?(benjamin) → superreview+
Comment 5•15 years ago
|
||
Comment on attachment 372199 [details] [diff] [review] get rid of nsSupportsArray, v3 >diff --git a/xpcom/base/nsConsoleService.cpp b/xpcom/base/nsConsoleService.cpp > static PRBool snapshot_enum_func(nsHashKey *key, void *data, void* closure) > { >- nsISupportsArray *array = (nsISupportsArray *)closure; >+ nsCOMArray<nsIConsoleListener> *array = >+ (nsCOMArray<nsIConsoleListener> *)closure; use reinterpret_cast<nsCOMArray<nsIConsoleListener> *> instead of the C-style cast. >diff --git a/xpcom/reflect/xptinfo/src/xptiInterfaceInfoManager.cpp b/xpcom/reflect/xptinfo/src/xptiInterfaceInfoManager.cpp > PRBool xptiAdditionalManagersEnumerator::AppendElement(nsIInterfaceInfoManager* element) > { >- if(!mArray.AppendElement(static_cast<nsISupports*>(element))) >+ if(!mArray.AppendObject(static_cast<nsISupports*>(element))) you shouldn't need the static_cast here, I think: use .AppendObject(element) r=me with nits picked
Assignee | ||
Comment 6•15 years ago
|
||
Patch as pushed with nits fixed: http://hg.mozilla.org/mozilla-central/rev/d12b4a785ecd
Attachment #372199 -
Attachment is obsolete: true
Attachment #372563 -
Flags: superreview+
Attachment #372563 -
Flags: review+
Assignee | ||
Updated•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 7•15 years ago
|
||
This appears to have caused a Txul win: http://graphs.mozilla.org/#show=787113,787087,787109&sel=1239523742,1239869342 Also a Tp3 win: http://graphs.mozilla.org/#show=395020,395048,395008&sel=1239523742,1239869342
Comment 8•15 years ago
|
||
Is this about actually removing the nsISupportsArray interface entirely?
Well, we want to do that yes, but this patch didn't fully do that.
Assignee | ||
Comment 10•15 years ago
|
||
This bug was about removing users of the implementation class. Removing the Interface and its users is far more complicated and not yet possible at all due to frozen interfaces using nsISupportsArray.
You need to log in
before you can comment on or make changes to this bug.
Description
•