772676 - History visit counts are all 1 after migration

Status: RESOLVED FIXED

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

Description

[Gian-Carlo Pascutto [:gcp]]

Regression from bug 710330: after running Profile Migration the visit counts in the history database are all equal to 1.

Comment 1

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. Fix visit count adjusetment

Comment 2

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 2. Add test for visit counts

Also rename test* to runTest* so Robocop doesn't accidentally pick them up.

Comment 3

[:Margaret Leibovic]

Comment on attachment 641368 [details] [diff] [review]
Patch 2. Add test for visit counts

>diff --git a/mobile/android/base/tests/testMigration.java.in b/mobile/android/base/tests/testMigration.java.in

>         try {
>             Class browserContract =
>                 mClassLoader.loadClass("org.mozilla.gecko.db.BrowserContract");
>             Class browserContractControl =
>                 mClassLoader.loadClass("org.mozilla.gecko.db.BrowserContract$Control");
>+            Class browserContractHistory =
>+                mClassLoader.loadClass("org.mozilla.gecko.db.BrowserContract$History");
>+            Class browserContractUrl =
>+                mClassLoader.loadClass("org.mozilla.gecko.db.BrowserContract$URLColumns");
>+            Class browserContractHistoryColumns =
>+                mClassLoader.loadClass("org.mozilla.gecko.db.BrowserContract$HistoryColumns");
>+
>             controlUri = (Uri)browserContractControl.getField("CONTENT_URI").get(null);
>+            historyUri = (Uri)browserContractHistory.getField("CONTENT_URI").get(null);
>+            urlField = (String)browserContractUrl.getField("URL").get(null);
>+            visitsField = (String)browserContractHistoryColumns.getField("VISITS").get(null);
>             String profilePath = (String)browserContract.getField("PARAM_PROFILE_PATH").get(null);
>             Uri.Builder builder = controlUri.buildUpon();
>             controlUri = builder.build();
>             ensureHistory =
>                 (String)browserContractControl.getField("ENSURE_HISTORY_MIGRATED").get(null);
>             ensureBookmarks =
>                 (String)browserContractControl.getField("ENSURE_BOOKMARKS_MIGRATED").get(null);
>         } catch (Exception ex) {

Nit: The error message that follows in here only talks about controlUri, but now there could be other things causing an exception here.

>+        // Check whether visits counts are as expected
>+        c = mResolver.query(historyUri,
>+                            new String[] { visitsField },
>+                            urlField  + " = ?",
>+                            new String[] { "http://www.reddit.com/" },
>+                            null);
>+        mAsserter.is(c.moveToFirst(), true, "Expected URL found");
>+        int visits = c.getInt(0);
>+        c.close();
>+
>+        mAsserter.is(visits, 4, "Visit count of " + visits + " equals expected 4");

Where does this URL/visit count come from? The test profile you're using for migration? Could you just add a comment about why this is expected? :)

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

Since bug 710330 was landed in 16 I've marked 16 affected as well, please correct if I'm wrong.

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Pushed with review comments addressed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3de0ba7c2eb4
https://hg.mozilla.org/integration/mozilla-inbound/rev/b878869b3d81

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3de0ba7c2eb4
https://hg.mozilla.org/mozilla-central/rev/b878869b3d81

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

[Triage Comment]
This looks like a viable candidate for beta uplift, please nominate if that's correct.

Comment 8

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 641367 [details] [diff] [review]
Patch 1. Fix visit count adjusetment

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 710330
User impact if declined: Awesomebar will be slightly less awesome
Testing completed (on m-c, etc.): Landed on m-c a while ago, with added tests
Risk to taking this patch (and alternatives if risky): Almost zero

Comment 9

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1e208424d68e