Once bug 972341 we can rewrite the about:newtab impressions telemetry probe as done in attachment 8375574 [details] [diff] [review] from bug 971171.

Marco, can you please add this to the current iteration? Thanks!

Added to Iteration 32.2

Comment on attachment 8425082 [details] [diff] [review] 0001-Bug-973532-Let-the-preloader-s-docShells-be-inactive.patch Review of attachment 8425082 [details] [diff] [review]: ----------------------------------------------------------------- Nice. Either here or in a new bug, we should make a small search-related change. We should either move gSearch.setUpInitialState's body to gSearch.init and call gSearch.init from gPage.onPageVisible, or we can still have gSearch.init and setUpInitialState, but init would not call setUpInitialState, and gPage would call init and setUpInitialState as this patch does. The reason is that currently, allowBackgroundCaptures isn't properly set on pages that bypass the preloader, so as a hedge, gSearch.init requests the initial search state, but so does setUpInitialState, which gPage calls from its mutation observer. For pages in the preloader, gSearch.init's request does nothing since content.js isn't loaded. Now that we're correctly checking the page's visibility, gSearch can request its initial state only once, when the page truly becomes visible. I can file a new bug if you don't want this to block this one. ::: browser/base/content/newtab/page.js @@ +198,4 @@ > } > + }, > + > + onPageVisible: function () { Nit: onPageFirstVisible?

(In reply to Drew Willcoxon :adw from comment #4) > Either here or in a new bug, we should make a small search-related change. > We should either move gSearch.setUpInitialState's body to gSearch.init and > call gSearch.init from gPage.onPageVisible, or we can still have > gSearch.init and setUpInitialState, but init would not call > setUpInitialState, and gPage would call init and setUpInitialState as this > patch does. I went with the second suggestion as gGrid calls gSearch.setWidth() which fails if gSearch.init() hasn't been called before.

I almost forgot that now that we record NEWTAB_PAGE_SHOWN always when the newtab page is shown, we can remove the special code we added for when about:newtab is the homepage. Otherwise we currently that twice.

gfritzsche is on PTO until June

Assigning to Kamil since this is related to Telemetry.

Went through verification using the following build: - http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-05-27-03-02-02-mozilla-central/ Went through all of the cases listed below and ensured that they are all being counted under about:telemetry: * NEWTAB_PAGE_DIRECTORY_AFFILIATE_SHOWN * NEWTAB_PAGE_DIRECTORY_ORGANIC_SHOWN * NEWTAB_PAGE_DIRECTORY_SPONSORED_SHOWN * NEWTAB_PAGE_SHOWN - loading about:newtab from the location bar - loading about:newtab via the "CTRL + T" shortcut - loading about:newtab via "+" next to the tabs - loading about:newtab from the menu bar - setting about:newtab as the home page and opening fx - while about:newtab is set as the home page, select the "Home" button - Opening a new window while about:newtab is set as the home page - Opening a new window (e10s) while about:newtab is set as the home page - loading about:newtab via bookmark - Ensured when "browser.newtabpage.enabled;false", about:newtab is not being counted Possible issues found: - set fx to restore from last session, open two about:newtab and close/re-open fx (about:newtab not being counted) - open a few about:newtab and close/re-open fx and select "Restore Previous Session" (about:newtab not being counted) - about:newtab is still being counted under "Private Browsing", not 100% sure if telemetry data should be logged while the user is in a private session Before completing verification, please let me know if the above three issues are already known or do they warrant a separate bug?

(In reply to Kamil Jozwiak [:kjozwiak] from comment #13) > - set fx to restore from last session, open two about:newtab and > close/re-open fx (about:newtab not being counted) > - open a few about:newtab and close/re-open fx and select "Restore Previous > Session" (about:newtab not being counted) I assume that you didn't actually click-to-select to non-selected tabs after restarting? We restore tabs lazily which means they are loaded when selected. > - about:newtab is still being counted under "Private Browsing", not 100% > sure if telemetry data should be logged while the user is in a private > session I don't see a problem with this one if you navigate to about:newtab directly. We don't reveal anything interesting.

(In reply to Tim Taubert [:ttaubert] from comment #14) > I assume that you didn't actually click-to-select to non-selected tabs after > restarting? We restore tabs lazily which means they are loaded when selected. Yup, that's correct. Went through the following: - selected "Restore Previous Session" and clicked on each about:newtab, ensured they're being counted - selected "Show windows from last time" and clicked on each about:newtab, ensured they're being counted - ensured that restored about:newtab(s) are counted once when clicked (going back and forth shouldn't count as multiple visits) > I don't see a problem with this one if you navigate to about:newtab > directly. We don't reveal anything interesting. Agreed. Wasn't 100% certain so asked just in case :) Thanks Tim, much appreciated.

Thank you for verifying!