401865 - nsMicrosummaryService.js cleanup

Status: RESOLVED FIXED

(Firefox Graveyard :: Microsummaries, defect)

Description

[Dão Gottwald [:dao]]

Attachment: patch

Comment 1

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

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?

Comment 2

[Dão Gottwald [:dao]]

Oh, yes, and there's more that can and should be fixed in those implementations. I'll file a new bug.

Comment 3

[Asaf Romano (gone)]

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

nit: please fix getService alignment.

r=mano.

Comment 4

[Dão Gottwald [:dao]]

Attachment: patch v2

Comment 5

[Dão Gottwald [:dao]]

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.

Comment 6

[Asaf Romano (gone)]

Comment on attachment 287773 [details] [diff] [review]
patch v2

Just land v1, sorry for my fonts-issue again ;)

Comment 7

[Dão Gottwald [:dao]]

Attachment: 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.

Comment 8

[Dão Gottwald [:dao]]

Attachment: patch v2

wrong patch ...

Comment 9

[Asaf Romano (gone)]

Comment on attachment 288387 [details] [diff] [review]
patch v2

r=mano

Comment 10

[Mike Schroepfer]

What's the goal of the cleanup?   Are we good with ensuring this doesn't call leaks?

Comment 11

[Dão Gottwald [:dao]]

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

[Mike Schroepfer]

Comment on attachment 288387 [details] [diff] [review]
patch v2

k - thnx for the explaination..

Comment 13

[Reed Loden [:reed]]

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