Last Comment Bug 586055 - Eliminate users of browsers.item
: Eliminate users of browsers.item
Status: VERIFIED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a3
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
http://mxr.mozilla.org/comm-central/s...
: 585444 585446 (view as bug list)
Depends on:
Blocks: 585511
  Show dependency treegraph
 
Reported: 2010-08-10 13:07 PDT by Robert Kaiser (not working on stability any more)
Modified: 2010-08-16 13:50 PDT (History)
3 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
a3+


Attachments
(Av1) Copy Firefox usage of "Array.some()" [Checkin: Comment 7] (974 bytes, patch)
2010-08-11 10:50 PDT, Serge Gautherie (:sgautherie)
kairo: review+
kairo: approval‑seamonkey2.1a3+
Details | Diff | Review

Description Robert Kaiser (not working on stability any more) 2010-08-10 13:07:27 PDT
In bug 585511, I eliminated browser.item, but we have a few callers left that should really die:
robert@robert:/mnt/mozilla/hg/comm-central> grep -rin "browsers.item" suite/
suite/browser/navigator.js:552:    for (var i = 0; browsers.item(i); i++)
suite/common/bindings/notification.xml:50:                this._activeBrowser = browsers.item(0);
suite/common/bindings/notification.xml:53:                  if (browsers.item(i).docShell) {
suite/common/bindings/notification.xml:54:                    this._activeBrowser = browsers.item(i);
Comment 1 neil@parkwaycc.co.uk 2010-08-10 13:17:52 PDT
(In reply to comment #0)
The notification.xml hits are false positives.
Comment 2 Serge Gautherie (:sgautherie) 2010-08-11 08:14:38 PDT
(In reply to comment #0)
> suite/browser/navigator.js:552:    for (var i = 0; browsers.item(i); i++)

{
372   isTabContentWindow: function isTabContentWindow(aWindow) {
373     var browsers = gBrowser.browsers;
374     for (var i = 0; browsers.item(i); i++)
375       if (browsers[i].contentWindow == aWindow)
376         return true;
377     return false;
378   }
}

This code should be replaceable using "Array.some()", right?
Comment 3 Robert Kaiser (not working on stability any more) 2010-08-11 08:39:15 PDT
Serge, feel free to do a patch using this, I would have done it with something I better understand myself, but your idea might be better overall. ;-)
Comment 4 Robert Kaiser (not working on stability any more) 2010-08-11 08:40:12 PDT
Oh, if you intend to do this, please do it within the next few hours though, as this is something that should go in for the already-frozen 2.1a3, btw.
Comment 5 Serge Gautherie (:sgautherie) 2010-08-11 10:50:51 PDT
Created attachment 464867 [details] [diff] [review]
(Av1) Copy Firefox usage of "Array.some()"
[Checkin: Comment 7]

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#4669
already has the intended code ;->

Requesting review from Callek, as in bug 585511.
Comment 6 Robert Kaiser (not working on stability any more) 2010-08-12 07:07:29 PDT
Comment on attachment 464867 [details] [diff] [review]
(Av1) Copy Firefox usage of "Array.some()"
[Checkin: Comment 7]

Stealing this one. Please get it landed ASAP, this is probably the reason why our alive-tests on leak test boxes are hanging and need to be manually stopped. We also shouldn't ship an alpha with this.

Thanks for looking into that!
Comment 7 Serge Gautherie (:sgautherie) 2010-08-12 11:51:20 PDT
Comment on attachment 464867 [details] [diff] [review]
(Av1) Copy Firefox usage of "Array.some()"
[Checkin: Comment 7]

http://hg.mozilla.org/comm-central/rev/55314c861a36
Comment 8 Robert Kaiser (not working on stability any more) 2010-08-12 12:02:13 PDT
status-seamonkey2.1 is only for wanted nominations right now, as we haven't branched, no need to set it to any value, target milestone does fine atm. :)
Comment 9 Robert Kaiser (not working on stability any more) 2010-08-12 16:03:19 PDT
*** Bug 585444 has been marked as a duplicate of this bug. ***
Comment 10 Robert Kaiser (not working on stability any more) 2010-08-12 16:03:25 PDT
*** Bug 585446 has been marked as a duplicate of this bug. ***
Comment 11 Serge Gautherie (:sgautherie) 2010-08-12 17:18:26 PDT
(In reply to comment #6)
> this is probably the reason why
> our alive-tests on leak test boxes are hanging and need to be manually stopped.

Indeed, this checkin fixed
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1281643601.1281645719.10743.gz
Linux comm-central-trunk debug test mochitests-1/5 on 2010/08/12 13:06:41

23733 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug426646.html | Test timed out.
TEST-UNEXPECTED-FAIL | /tests/content/html/document/test/test_bug391777.html | application timed out after 330 seconds with no output
PROCESS-CRASH | /tests/content/html/document/test/test_bug391777.html | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | missing output line for total leaks!
}
on all 3 platforms :-)

(In reply to comment #8)
> status-seamonkey2.1 is only for wanted nominations right now, as we haven't
> branched

Ah, I'm used to getting unsure/wrong in this specific situation ;-<
Comment 12 Serge Gautherie (:sgautherie) 2010-08-12 17:19:14 PDT
V.Fixed, per the duplicates, then.

Note You need to log in before you can comment on or make changes to this bug.