nsMicrosummaryService.js cleanup

RESOLVED FIXED in Firefox 3 beta2

Status

Firefox Graveyard
Microsummaries
RESOLVED FIXED
11 years ago
2 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

Trunk
Firefox 3 beta2

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

11 years ago
Created attachment 286810 [details] [diff] [review]
patch
Attachment #286810 - Flags: review?(mano)
Comment on attachment 286810 [details] [diff] [review]
patch

>Index: browser/components/microsummaries/src/nsMicrosummaryService.js

> function ArrayEnumerator(aItems) {
>-  this._index = 0;
>-  this._contents = [];
>   if (aItems) {
>     for (var i = 0; i < aItems.length; ++i) {
>       if (!aItems[i])
>-        aItems.splice(i, 1);      
>+        aItems.splice(i--, 1);

Hmm, this code is duplicated a lot in the tree (http://lxr.mozilla.org/seamonkey/search?string=ion%20ArrayEnumerator), I suppose we should fix them all?
(Assignee)

Comment 2

11 years ago
Oh, yes, and there's more that can and should be fixed in those implementations. I'll file a new bug.
Comment on attachment 286810 [details] [diff] [review]
patch

nit: please fix getService alignment.

r=mano.
Attachment #286810 - Flags: review?(mano) → review+
(Assignee)

Comment 4

10 years ago
Created attachment 287773 [details] [diff] [review]
patch v2
Attachment #286810 - Attachment is obsolete: true
(Assignee)

Comment 5

10 years ago
Comment on attachment 287773 [details] [diff] [review]
patch v2

there were quite a few getService calls aligned that way. I also changed two other minor things while I was at it, interdiff should be helpful here.
Attachment #287773 - Flags: review?(mano)
Comment on attachment 287773 [details] [diff] [review]
patch v2

Just land v1, sorry for my fonts-issue again ;)
Attachment #287773 - Flags: review?(mano)
(Assignee)

Comment 7

10 years ago
Created attachment 288386 [details] [diff] [review]
patch v2

The second patch didn't just change the indention, it also contained a fix for the getPref function where getPrefType was called twice for int prefs.
Attachment #287773 - Attachment is obsolete: true
Attachment #288386 - Flags: review?(mano)
(Assignee)

Comment 8

10 years ago
Created attachment 288387 [details] [diff] [review]
patch v2

wrong patch ...
Attachment #288386 - Attachment is obsolete: true
Attachment #288387 - Flags: review?(mano)
Attachment #288386 - Flags: review?(mano)
(Assignee)

Updated

10 years ago
Attachment #288387 - Flags: approval1.9?

Comment 10

10 years ago
What's the goal of the cleanup?   Are we good with ensuring this doesn't call leaks?
(Assignee)

Comment 11

10 years ago
This is mostly code shifting (e.g. inlining of internal methods and getters that have been used only once), small performance optimizations (e.g. in getPref and LOG), and it fixes the wrongly implemented ArrayEnumerator constructor.

Comment 12

10 years ago
Comment on attachment 288387 [details] [diff] [review]
patch v2

k - thnx for the explaination..
Attachment #288387 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Checking in browser/components/microsummaries/src/nsMicrosummaryService.js;
/cvsroot/mozilla/browser/components/microsummaries/src/nsMicrosummaryService.js,v  <--  nsMicrosummaryService.js
new revision: 1.81; previous revision: 1.80
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M10
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.