567121 - History is not cleared immediately after selecting Clear in Options->Preferences

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[ahoza]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
Build Identifier: Mozilla/5.0(X11;U;Linux armv7l; Nokia N900; en-US;rv:1.9.2.5pre) Gecko/20100520 Namoroka/3.6.5pre Fennec 1.1b2pre 

Previously visited sites are still available after clearing private data.

Reproducible: Always

Steps to Reproduce:
1. Clear private data and restart browser
2. Visit www.google.com and then mozilla.org
3. Go to Options->Preferences and tap on Clear button in "Clear private data section"
4. Go back to the awesome bar.
5. Start typing "goo"

Actual Results:  
At step 4, previously visited sites are also available in the list. When starting typing "goo", empty set is displayed and if the bar is cleared those sites are not available anymore in the list.

Expected Results:  
When performing step 4 only bookmarked pages are displayed (no history).
Empty result set is displayed after step 5.

The following bug was filed describing the same behavior: https://bugzilla.mozilla.org/show_bug.cgi?id=547993

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

The problem with this bug is the AutoCompleteCache is being lazily initialized.

I can't reproduce this bug as stated in comment 0. As soon as I open the awesome bar the AutoCompleteCache is initialized and clearing the cache works.

But if I open Fennec and go directly to clear the cache (don't open the awesomebar) then the cache is not cleared.

Bug 547993 never saw this issue because we always opened the awesome bar when Fennec loaded, therby initing the AutoCompleteCache.

I don't think this is a big deal, now that I see the problem. We could always init the AutoCompleteCache during startup, but that might hurt Ts.

We could init the AutoCompleteCache in our delayed startup code.

Thoughts?

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Let me just point out that the session history is _always_ clear as soon as you press the button. What you see in the awesombar is just some left over items stuck in the AutoComplete cache, waiting for the cache to be cleared.

The next time you open the awesomebar, or even clear the text in the bar and start typing again, the cache will be cleared to reflect the history.

Comment 3

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Mozilla/5.0 (Android; Linux armv71; rv2.0b13pre) Gecko/20110310 Firefox/4.0b13pre Fennec/4.0b6pre
Device: Droid 2 
OS: Android 2.2

After I hit the clear button in today's build (with or without sync) I get an error in console:

Error: [Exception... "JavaScript component does not have a method named: "observe" when calling method: [nsIObserver::observe]" nsresult:"0x80570030 (NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED)" location: "JSframe :: chrome://broser/content/sanitize.js :: <TOP_LEVEL> :: line 195" data:no ]
Source File: chrome://browser/content/sanitize.js

Comment 4

[Wesley Johnston (:wesj)]

Attachment: Patch?

Is there some reason we can't just do this?

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 522530 [details] [diff] [review]
Patch?

Deleting the file and setting the cache to null doesn't directly do anything. Setting the cache to null will cause this code path to be skipped though:

http://mxr.mozilla.org/mobile-browser/source/components/AutoCompleteCache.js#299

And we will force a fetch here:
http://mxr.mozilla.org/mobile-browser/source/components/AutoCompleteCache.js#313

If that is the desired outcome anyway, let's just change the existing AutoCompleteUtils.update() call to a AutoCompleteUtils.fetch(AutoCompleteUtils.query) call:
http://mxr.mozilla.org/mobile-browser/source/components/AutoCompleteCache.js#347

r- if we can drop the file exists/remove code and just call fetch directly

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Patch

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 522664 [details] [diff] [review]
Patch

I assume this actually works? Any ideas for testing this?

Comment 8

[Wesley Johnston (:wesj)]

Yeah works fine.

I tested two ways, by looking at the cache file immediately after I clicked "Clear" in prefs, and by opening the urlbar to make sure that the old results weren't shown (there is a brief flash while we reset the autocomplete items, we could reset that as well).

We can do the same thing in tests I assume. i.e. Look at the file to make sure it is updated, and open the urlbar to make sure it has updated. Working on those....

Comment 9

[Wesley Johnston (:wesj)]

Pushed: http://hg.mozilla.org/mobile-browser/rev/6b9a963ef505

I'll file a follow up to write some tests for sanitize (already in progress in my queue).

Comment 10

[Anna (Waverley)]

I can still reproduce this on:

Build ID: Mozilla /5.0 (Android;Linux armv7l;rv:2.2a1pre) Gecko/20110406 Firefox/4.2a1pre Fennec /4.1a1pre 

Device: Motorola Droid 2 (Android 2.

Opening the bug...

Comment 11

[Wesley Johnston (:wesj)]

Attachment: Patch v2

Talked to sdwilsh about this today. The problem is that the PlacesAutocompleteService (PACs) holds a separate readonly connection to the places database than the normal history service. When the history service clears the history, PACs may not read from that exact same database.

Instead, he said to follow what they do in tests which need to be sure history has been cleared. These watch for a notification from places:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/browser/browser_visituri_nohistory.js#46

This just updates our AutoCompleteCache service to use this same notification. I looked into changing other areas, but unless a service really needs to know when history has been cleared, and not just that the user has requested history be cleared, we should be fine. I didn't see any other services that really met that condition for me.

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 533354 [details] [diff] [review]
Patch v2

Our tests still pass?

Comment 13

[Wesley Johnston (:wesj)]

Tests pass as well with as without for me. Pushed:
http://hg.mozilla.org/mozilla-central/rev/6f5cb026f6d8

Comment 14

[Anna (Waverley)]

VERIFIED FIXED on:

Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a2) Gecko/20110705 Firefox/6.0a2 Fennec/6.0a2 

Device: HTC Desire Z (Android 2.2)