History visit counts are all 1 after migration

RESOLVED FIXED in Firefox 15

Status

()

Firefox for Android
Data Providers
--
minor
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gcp, Assigned: gcp)

Tracking

({regression})

15 Branch
Firefox 16
ARM
Android
regression
Points:
---

Firefox Tracking Flags

(firefox15+ fixed, firefox16+ fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
Regression from bug 710330: after running Profile Migration the visit counts in the history database are all equal to 1.
(Assignee)

Updated

5 years ago
Assignee: nobody → gpascutto
(Assignee)

Updated

5 years ago
status-firefox15: --- → affected
tracking-firefox15: --- → ?
(Assignee)

Comment 1

5 years ago
Created attachment 641367 [details] [diff] [review]
Patch 1. Fix visit count adjusetment
Attachment #641367 - Flags: review?(margaret.leibovic)
(Assignee)

Comment 2

5 years ago
Created attachment 641368 [details] [diff] [review]
Patch 2. Add test for visit counts

Also rename test* to runTest* so Robocop doesn't accidentally pick them up.
Attachment #641368 - Flags: review?(margaret.leibovic)
(Assignee)

Updated

5 years ago
Attachment #641368 - Attachment is patch: true

Updated

5 years ago
Attachment #641367 - Flags: review?(margaret.leibovic) → review+

Comment 3

5 years ago
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? :)
Attachment #641368 - Flags: review?(margaret.leibovic) → review+
Since bug 710330 was landed in 16 I've marked 16 affected as well, please correct if I'm wrong.
status-firefox16: --- → affected
tracking-firefox15: ? → +
tracking-firefox16: --- → +
Keywords: regression
(Assignee)

Comment 5

5 years ago
Pushed with review comments addressed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3de0ba7c2eb4
https://hg.mozilla.org/integration/mozilla-inbound/rev/b878869b3d81
https://hg.mozilla.org/mozilla-central/rev/3de0ba7c2eb4
https://hg.mozilla.org/mozilla-central/rev/b878869b3d81
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
status-firefox16: affected → fixed
Target Milestone: --- → Firefox 16
[Triage Comment]
This looks like a viable candidate for beta uplift, please nominate if that's correct.
(Assignee)

Comment 8

5 years ago
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
Attachment #641367 - Flags: approval-mozilla-beta?

Updated

5 years ago
Attachment #641367 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/1e208424d68e
status-firefox15: affected → fixed
You need to log in before you can comment on or make changes to this bug.