1145410 - Recently-introduced error appearing soon after starting up (& early in reftest output): "JavaScript error: resource://gre/modules/NewTabUtils.jsm, line 1255: TypeError: Links._providers.get(...) is null"

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Daniel Holbert [:dholbert]]

STR:
1. Run ./mach reftest layout/reftests/reftest-sanity/
  (or any other mach reftest command)

2. Inspect the first few lines of terminal output.

ACTUAL RESULTS: Mixed in with the first test result, you'll see something like this:

> 0:09.98 JavaScript error: resource://gre/modules/NewTabUtils.jsm, line 1255: TypeError: Links._providers.get(...) is null
> 0:09.98 JavaScript error: resource://gre/modules/NewTabUtils.jsm, line 1255: TypeError: Links._providers.get(...) is null

Looks like this was likely introduced by bug 1126184, which recently modified NewTabUtils.jsm and added a call to Links._providers.get():
http://hg.mozilla.org/mozilla-central/rev/dab6b12a0ff6#l3.247

Comment 1

[Daniel Holbert [:dholbert]]

I also hit this during a normal browser session -- it's not just a reftest-harness issue.

e.g. I just performed these STR:
 1. Start Firefox with a fresh profile, pointed at about:blank.
    Specifically, I ran this command, on linux:
      mkdir /tmp/foo; ./dist/bin/firefox -profile /tmp/foo -no-remote about:blank
 2. Type "google.com" into URL bar and hit enter.
 3. Watch your terminal while google is loading.

With those steps, I see 3 instances of this JS error go by, while Google loads.

Marina, I think this indicates that something from bug 1126184's patch isn't working as expected. Any chance you can take a look?

Comment 2

[Marina Samuel [:emtwo]]

Thanks for pointing this out, Daniel. I will investigate.

Comment 3

[Marina Samuel [:emtwo]]

Attachment: v1: Return valid results when querying the provider cache while it's empty or being populated

So it looks like there were a couple of things happening here:

1) If a user starts up a browser with their default newtab page pref to be 'blank' (aka not enabled), then the provider cache won't be populated and every time a page is loaded we look at the cache (to check if we need to update a suggested tile) and this error was thrown.

2) In the reftests, new pages were being open before the cache had finished populating.

In both cases once the cache finishes populating later on, we'll attempt to choose the suggested link again. So it's safe to treat the cache as empty even if it's mid-population.

Comment 4

[Drew Willcoxon :adw]

Comment on attachment 8580870 [details] [diff] [review]
v1: Return valid results when querying the provider cache while it's empty or being populated

Review of attachment 8580870 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, Marina.

::: toolkit/modules/NewTabUtils.jsm
@@ +1249,5 @@
>  
>    getProviderLinks: function(aProvider) {
> +    let cache = Links._providers.get(aProvider);
> +    if (cache && cache.sortedLinks) {
> +      return Links._providers.get(aProvider).sortedLinks;

return cache.get(aProvider).sortedLinks;

@@ +1257,5 @@
>  
>    isTopSiteGivenProvider: function(aSite, aProvider) {
> +    let cache = Links._providers.get(aProvider);
> +    if (cache && cache.siteMap) {
> +      return Links._providers.get(aProvider).siteMap.has(aSite);

return cache.get(aProvider).siteMap...

Comment 5

[Marina Samuel [:emtwo]]

https://hg.mozilla.org/integration/fx-team/rev/531fe47e635b

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/531fe47e635b

Comment 7

[Ed Lee :Mardak]

https://hg.mozilla.org/releases/mozilla-beta/rev/56763fc69140

Comment 8

[Cornel Ionce [:noni] [Hubs QA]]

I'm not seeing this JS error using the STR provided by Daniel in comment 2 on Ubuntu 14.04 64-bit using:
- Firefox 38 beta 2, build ID: 20150406174117
- latest Aurora, build ID: 20150408004004