Closed Bug 630549 Opened 13 years ago Closed 13 years ago

Intermittent browser_tabMatchesInAwesomebar.js | Registered open page found in autocomplete

Categories

(Firefox :: Address Bar, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 11

People

(Reporter: philor, Assigned: mak)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

http://tinderbox.mozilla.org/showlog.cgi?log=TraceMonkey/1296570566.1296572121.3504.gz
Rev3 WINNT 6.1 tracemonkey opt test mochitest-other on 2011/02/01 06:29:26
s: talos-r3-w7-044

TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/browser_tabMatchesInAwesomebar.js | Registered open page found in autocomplete.
TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/browser_tabMatchesInAwesomebar.js | Search for 'http://example.org/browser/browser/base/content/test/moz.png#tabmatch18' in open tabs.
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_tabMatchesInAwesomebar.js | Registered open page found in autocomplete.
TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/browser_tabMatchesInAwesomebar.js | Running step 3
Happened on local Linux.

TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/browser_tabMatchesInAwesomebar.js | Registered open page found in autocomplete.
TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/browser_tabMatchesInAwesomebar.js | Search for 'http://example.org/browser/browser/base/content/test/moz.png#tabmatch18' in open tabs.
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_tabMatchesInAwesomebar.js | Registered open page found in autocomplete.
TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/browser_tabMatchesInAwesomebar.js | Running step 3
TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/browser_tabMatchesInAwesomebar.js | Loading page: http://example.org/browser/browser/base/content/test/dummy_page.html#tabmatch18

$ hg tip
changeset:   62187:ca548ef9d8c6
tag:         tip
user:        Priit Laes <plaes@plaes.org>
date:        Wed Feb 09 00:02:37 2011 +0100
summary:     Bug 628222 - Add support for libnotify-0.7+. r=karlt a=joe
OS: Windows 7 → All
hm, the recent failures are strange, something must have landed around 28-29 Oct 2011 that makes autocomplete return duplicates
Since this is hitting us pretty often, I'll take a look if I can figure out something.
Assignee: nobody → mak77
no interesting changes there, apart some change that may speed up visits addition, so my theory is that we don't wait enough after addition of the last visit, changing timing just made a bug more evident.

- the switch-to-tab query returns the page thinking it's not in the database, since it's still in the process of being added.
- processRow registers the page by url, since there it has no place-id
- the visit is actually added to a searchable part of the database (likely it's still in the journal, but now the checkpoint is valid)
- the frecency query now finds the same page, but with a valid id
- processRow registers the page by id now, it won't look for the url since the page has an id
- the page ends up being twice in the results, once hashed by id and once hashed by url

This is actually a bug since, even if rarely, it may happen in real life.
- ideally the switch-to-tab query should only return pages not addable to history. While this would be easy to do checking the schema, it would also completely kill switch-to-tab in privatebrowsing mode.
- we may just always hash results by url, while doing so is a bit slower (the url string has to be hashed, while the id doesn't), it would just happen for a few results (the ones needed to reach a maxRichResults of 12, so a few more than 12). This would solve the random failure and avoid duplicates properly, but in rare cases where this bug happens a page that is being added to history may appear in the first position rather than in its proper frecency position.

I'm thinking of alternative solutions involving .canAddURI, but I'd not want to slowdown results addition just to fix an edge-case.
Attached patch patch v1.0Splinter Review
I forgot we can't hash by url due to the keywords search (they modify the url to include the search terms).
And I don't think we can take the canAddURI path since it would do too many checks on each url, being far more expensive than a couple more string hashing.

So, this is the best solution I found so far, checking the id, and if the match is negative also check the url. We will survive the rare edge-case that a user can search exactly for the page he just visited milliseconds ago, to preserve performances in the other cases.
Attachment #577075 - Flags: review?(dietrich)
Comment on attachment 577075 [details] [diff] [review]
patch v1.0

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

r=me, thanks!
Attachment #577075 - Flags: review?(dietrich) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/3062a392aa37

I think this can be closed on merge, since this was the reason for all the known recent failures, older ones may not exist anymore.
Target Milestone: --- → Firefox 11
https://hg.mozilla.org/mozilla-central/rev/3062a392aa37
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: