842607 - Error: TelemetryStopwatch: key "FX_PAGE_LOAD_MS" was already initialized when opening new tab with newtab preloading enabled

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Jim Jeffery not reading bug-mail 1/2/11]

STR:
1. Open new tab
2. Press 'down arrow' 
3. select any site from the list
4. Note error is produced in the Error console

Tue Feb 19 2013 11:39:50
Error: TelemetryStopwatch: key "FX_PAGE_LOAD_MS" was already initialized
Source file: resource://gre/modules/TelemetryStopwatch.jsm
Line: 53
                     aHistogram + "\" was already initialized");


Tested on Win7 x64 m-c latest hourly build cset:
https://hg.mozilla.org/mozilla-central/rev/a0c57dffd179

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

I'm pretty sure this is caused by newtab page preloading. I don't see the error with 'browser.newtab.preload' set to 'false'. This also makes a lot of sense because the progress listeners doesn't really know we stole it's loading docShell and replaced it with one that's already loaded.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

FTR, you don't need to press the down arrow. Just open a new tab (preloaded) and enter some URL.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Not sure what to do about this. The only events getting fired when swapping docShells are pageshow and pagehide. Should we introduce another event like "TabSwapped" or the like that indicates the tab's content has changed? We could also just call Stopwatch.cancel() before we call start to make sure it doesn't cause any errors. It's supposed to be a perf measurement after all.

Comment 4

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

I think we had another bug somewhere that discussed adding a TabMoved notification, let me see if I can find it.

Comment 5

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

bug 723951

Comment 6

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

Could we just avoid starting the timer in tabbrowser if aRequest.URI.spec is one that we preload (i.e. about:newtab)?

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

It's not that easy unfortunately. Depending on whether a preloaded newtab page in the background is ready we may swap or not.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

FWIW, I didn't see this error for quite a while.

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Ok, this was fixed by bug 875509, great. Previously we would start loading about:newtab and swap afterwards if there is a preloaded one ready in the background. Now, we let the BrowserNewTabPagePreloader handle loading *after* (and only if) the docShells were swapped.