705610 - setting network.dns.disablePrefetch to true prevents history from being recorded

Status: VERIFIED FIXED

(Camino Graveyard :: General, defect)

Description

[philippe (part-time)]

STR
1. in about:config, set network.dns.disablePrefetch to true (false = default)
2. visit some webpages

A.R. those visits are not recorded in browsing history (history manager is blank, URL autocomplete doesn't know about those recent visits)

The back button still works though.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

This was reported via the forum; philippe and I both can repro.

In my debug build, I was initially seeing a bunch of unusual nsContentUtils warnings, but 1) I can't reproduce them and 2) they were for methods regarding things like DTD loading and processing, so exceedingly unlikely to be related to prefetch.

So I have no idea why this is failing and Gecko's not providing any useful logspam info to help :(

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Some things we should check here:

1) Does the same thing happen in Firefox 36.25?
2) Is history actually getting stored in places.sqlite and just not making 
   it to our UI layer, or is history just failing entirely?

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Since users are starting to see the prefetch+routers hangs (bug 475603, bug 706969), we need to either figure this out or back out prefetch on our docshells for 2.1.1 :(

Comment 4

[Chris Lawson (gone)]

(In reply to Smokey Ardisson (back-ish; no bugmail - do not email) from comment #2)
> Some things we should check here:
> 
> 1) Does the same thing happen in Firefox 36.25?

I assume you meant 3.6.24, and it doesn't even appear to have the pref in about:config by default (yay for honoring prefs that Firefox doesn't define in all.js).

I added it. Firefox 3.6.24's History menu shows each successive URL that I visit regardless of the state of that pref. I tried visiting several pages with it set to FALSE, then visited several more with it set to TRUE, and nothing changed. I'm not sure how to test whether or not it's actually doing anything, though.

> 2) Is history actually getting stored in places.sqlite and just not making 
>    it to our UI layer, or is history just failing entirely?

I set the pref to TRUE, reproduced the bug, and observed the last-modified date on places.sqlite did not change.

I then set the pref back to FALSE and visited two more sites. The last-modified date then changed.

SQLite Database Browser and my 154-MB places.sqlite file are not friends, so I can't say for certain whether the last-modified date test is 100% valid, but I'd bet it is.

Comment 5

[Chris Lawson (gone)]

I just repeated the test in comment 4 with a clean profile, and I can confirm that no modifications to places.sqlite are occurring while prefetch is disabled. Re-enabling it restores the ability to record history, and inspecting the file with SQLite Database Browser shows no evidence that anything happened at all during the period during which prefetch was disabled.

Comment 6

[philippe (part-time)]

(In reply to comment #2)
> Some things we should check here:
> 
> 1) Does the same thing happen in Firefox 36.25?

After adding (!) the pref set to true in Fx 3.6.24 on a default profile, I loaded several pages; all of them are recorded in the FX' history. I restarted for good measure and verified that both the old history and newly added pages were recorded.

> 2) Is history actually getting stored in places.sqlite

Tested with a default profile; set the pref to true and visited several pages. I then opened the places.sqlite file with SQLite Database Browser and found the moz_places table to be empty with the exception of cb.o/welcome/ which was recorded before I flipped the pref.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Last-resort patch: back out bug 493474

Here's the patch of last resort, which just backs out bug 493474.

In my debug build, I still get history, and per the frozen embedding API, prefetch should *not* be enabled, so no one should hang.

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Fix

I started putting in logging statements and tracing the nsIWebBrowserSetup process, and when I got to nsWebBrowser.cpp, I noticed something interesting: the property for enabling global history was immediately after the property for dns prefetch.  Then I stared really hard and noticed there was a missing 'break;' after the prefetch case.

I then discovered that this bug was actually fixed for Gecko 2.0 and up in bug 576518, as an incidental part of some IPC/e10s changes; this patch for 1.9.2 is just the missing 'break;' that will let Camino and other nsIWebBrowserSetup turn off prefetch without turning off global history ;-)

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 578805 [details] [diff] [review]
Fix

Nice catch.  r=me

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 578805 [details] [diff] [review]
Fix

Requesting approval for 1.9.2.25; this patch fixes a bug that prevents (embedding) consumers of nsIWebBrowserSetup such as Camino from disabling prefetch without also disabling global history as an unfortunate side effect.

(Because of bug 475603, it's important to let users of broken routers turn off prefetch, and these broken routers are very common, e.g. AT&T distributes 2Wire routers that trigger bug 475603, and those routers have no firmware updates available.)

This bug has already been fixed in Gecko 2.0 and up, so the fix has baked on the trunk for a year and then some.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 578805 [details] [diff] [review]
Fix

approved for 1.9.2.25, a=dveditz

Comment 12

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

https://hg.mozilla.org/releases/mozilla-1.9.2/rev/2965c8f56a84

Comment 13

[philippe (part-time)]

Verified fixed with Camino Version 2.1.1pre (1.9.2.25pre 20111206001458) by loading a few pages on a default profile, then flipping the pref to true, surfing some more. All pages are recorded in history.