799433 - Removal of PageThumbsCache causes Firefox to keep loading 'about:newtab' forever after removing of page history

Status: RESOLVED WONTFIX

(Firefox :: New Tab Page, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

This strange behavior has been discovered by our Mozmill tests a while back. In most of the cases it cannot easily be reproduced. We only see this on machines which are running our tests for more than 2 or so weeks. A reboot makes the problem go away. We were able to create a minimized testcase for Mozmill which is attached to bug 790191, which I have used to do the regression test.

As it has been shown bug 762094 ([Page Thumbnails] remove file cache fallback) is the cause of this problem. 

The test itself is doing only some simple tasks whereby the last opening of about:newtab stalls. I can't reproduce it manually so far. But will try to do so.


var browserHistory = Cc["@mozilla.org/browser/nav-history-service;1"].
                     getService(Ci.nsIBrowserHistory);

function setupModule() {
  controller = mozmill.getBrowserController();
}

function testHang() {
  controller.open("about:newtab");
  controller.waitForPageLoad();

  browserHistory.removeAllPages();
  controller.sleep(2000);

  controller.open("about:newtab");
  controller.waitForPageLoad();
}

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Ok, tried manually and it can be reproduced.

Steps:
1. Create a fresh profile
2. Open some pages so images are in the cache
3. Open about:newtab
4. Open the error console and execute the following command:

> Components.classes["@mozilla.org/browser/nav-history-service;1"].getService(Components.interfaces.nsIBrowserHistory).removeAllPages()

5. Enter about:newtab in the locationbar again and press Enter

After step 5 Firefox is not able to load the newtab page.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

I can now even reproduce it locally with a freshly booted machine and Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:18.0) Gecko/18.0 Firefox/18.0 (20121007030539)

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

As mentioned before this is causing massive failures in our mozmill tests. So I raise the severity to major and hope we can get it fixed soon.

Comment 4

[u279076]

If this is a regression, can you find a regression range, Henrik?

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #4)
> If this is a regression, can you find a regression range, Henrik?

Please read my initial description:

As it has been shown bug 762094 ([Page Thumbnails] remove file cache fallback) is the cause of this problem.

Comment 6

[bhavana bajaj [:bajaj]]

Passing on this to you Tim, as it is a suspected regression from bug 762094 .Please feel free to reassign if needed.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Tim, we really would appreciate a feedback from you. This broken behavior is spamming us with a dozen of test failures per day, which we kinda would like to see gone. Thanks!

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

I have no idea what's causing this. It's all handled by the file channel and I don't know what's the expected behavior when a file is missing. The implementation is in PageThumbsProtocol.js - all it does is create a new file channel and return it.

It would be great if we find someone to investigate the root cause of this. I don't have much time to do it as I'll certainly not be working on Firefox Desktop until the end of the year.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Thanks Tim. bhavana, is there someone else we could have for the investigation and a possible fix?

Comment 10

[Alex Keybl [:akeybl]]

Felipe - can you look into this and let us know if we should consider a forward fix, wontfix, or back out bug 762094? Tim doesn't appear to have time to look at this right now.

Comment 11

[:Felipe Gomes (needinfo for replies!)]

Yep, looking at it

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Felipe, do you have an update for us?

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

(In reply to Henrik Skupin (:whimboo) from comment #12)
> Felipe, do you have an update for us?

Mind giving us an estimate for a fix?

Comment 14

[:Felipe Gomes (needinfo for replies!)]

Henrik: I've investigated this for a bit and I've got a lead to the problem, but not enough info yet to be sure. The Social API work going on on beta has blocked me into looking into this but I believe I'll be able to get back to this again on Wednesday.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

This is now in beta. Can we get an update on what we want to do? Is it so hard to get the causing patch backed out?

Comment 16

[:Felipe Gomes (needinfo for replies!)]

(In reply to Henrik Skupin (:whimboo) from comment #15)
> This is now in beta. Can we get an update on what we want to do? Is it so
> hard to get the causing patch backed out?

I've discussed this in the past weeks with Gavin and I'm now working on a patch to fix the problem. We can't simply backout bug 762094 because it didn't cause the bug, it just exposed a previous problem (actually 2 probs).

Does the QA tests that are broken by this also need to have it fixed on beta/aurora? This is a minor bug for users so we talked about untracking this for the branches and only fixing it on m-c. But one of the patches to fix it should be hopefully simple enough that it could get approval for the branches if QA needs it.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

All of our tests have a workaround for now. Means we have disabled the usage of about:newtab and replaced it with about:blank. It's not ideal if we can't land it on aurora, beta, but if those backports would cause a lot of work, I would be fine with your suggestion. I do not speak for the QA feature owner. Probably Anthony can give his statement at least.

Comment 18

[u279076]

I'm fine with your suggestion as well, Felipe. If we start to hear noise on Input/SUMO about similar issues on Beta then we might want to consider uplifting for Firefox 18, assuming it's a low-risk fix. It's just unfortunate that we can't use the New Tab page in our automation right now.

Comment 19

[Alex Keybl [:akeybl]]

Given the minor impact to users, and the QA workaround, no need to track for release.

Comment 20

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

From my discussion with Felipe, IIRC the problem was two-fold:
1) we don't lose all in-memory references to the now-removed files when clearing history (this should be solvable with a browser:purge-session-history observer in PageThumbs.jsm?)
2) the PageThumbsProtocol.js protocol handler somehow doesn't fail right when the file it wants to point to is now missing (which causes the "spinning forever"). Need to look into why that happens (probably in a followup).

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Removing blocking whiteboard entry because it's an annoyance but not a blocker anymore.

Comment 22

[Matthew N. [:MattN]]

Mass-move to Firefox::New Tab Page.

Filter on new-tab-page-component.

Comment 23

[Jessilyn Davis]

Hello!

This bug has been closed due to inactivity and/or the potential for this bug to no longer be an issue with the new Discovery Stream-powered New Tab experience.

Please help us triage by reopening if this issue still persists and should be addressed.

Thanks!