765211 - [New Tab Page] AllPages.unregister() can possibly remove wrong pages

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Attachment: trivial patch

When given a page that isn't in the list of registered pages, NewTabUtils.allPages.unregister() will still remove a page because it doesn't check if indexOf() > -1.

Comment 1

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

Comment on attachment 633489 [details] [diff] [review]
trivial patch

How'd you find this? Presumably in practice unregister never gets called for pages that haven't been registered?

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #1)
> How'd you find this? Presumably in practice unregister never gets called for
> pages that haven't been registered?

Right, while working on bug 753448 I noticed that the preloaded about:newtab instances were out of sync because they weren't tracked anymore. This happens because of how we use registerCleanupFunction() here to make sure the dummy page gets removed when timing out:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/newtab/head.js#312

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/78bae12834f6

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/78bae12834f6