1277330 - Importing Android Browser history multiple times keeps increasing visit counts

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: Data Providers, defect, P5)

Description

[:Grisha Kruglov]

If history is imported multiple times from the Android Browser, we'll see that History.VISITS count keeps going up. That is because we unconditionally increase it by number of visits in the Android Browser history, regardless of the fact that we might already have these visits in the Visits table, and import might not have actually done anything.

Once Bug 1274029 lands, this bug will also affect History.LOCAL_VISITS aggregate as well, in the same way.

Note that Visits table behaves correctly, in that no duplicates are inserted.

Impact: if user imports their history multiple times, counts will keep going up - especially local_visits aggregate (past Bug 1274029), and their top sites will start bubbling up imported items.

Approach #1: ideally, we want to look at how many visits we actually inserted into the visits table, and increase our counters by that number. Due to how import is done (via batch operations that get built up and then flushed), we'll first need to refactor import in order to obtain "number of visits inserted". There isn't too much to refactor, but this code isn't covered by unit tests...

Approach #2: remember when was the last time we imported, and only insert/update counts if Android Browser has items which are newer. This could be pretty brittle, since our wall time might (read: will) change.

Approach #3: recalculate our visit counts based on the visit table after we import. This will ensure that History.LOCAL_VISITS aggregate is correct (since we can treat visits table as the source of truth) and ensure that top sites list is unaffected, but might be problematic for History.VISITS.

Approach #4: ignore this problem and mark it as a "known issue"; Android Browser is dead anyway past API21, and import is disabled on newer devices.

I'm personally leaning towards #3 + rethink the need for History.VISITS that we keep up manually - could it be just a sum of LOCAL_VISITS+REMOTE_VISITS which is calculated in the Combined view? Do we need it at all, now that it's not used for Top Sites? See Bug 1277329.

Comment 1

[Richard Newman [:rnewman]]

I'm strongly in favor of #4: WONTFIX this, tell the three users who'll try this "don't do that". I bet you can think of 10 bugs right now that are more valuable to work on…

Comment 2

[:Grisha Kruglov]

(In reply to Richard Newman [:rnewman] from comment #1)
> I'm strongly in favor of #4: WONTFIX this, tell the three users who'll try
> this "don't do that". I bet you can think of 10 bugs right now that are more
> valuable to work on…

Indeed. But now we have this documented :-)