Closed Bug 970300 Opened 6 years ago Closed 6 years ago

(Synth APK) - Checking webapps for updates always downloads the webapp even if there is no update available

Categories

(Firefox for Android :: Web Apps (PWAs), defect, P1, blocker)

ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 30
Tracking Status
firefox29 --- fixed
firefox30 --- verified

People

(Reporter: cos_flaviu, Assigned: myk)

References

Details

(Keywords: regression)

Attachments

(2 files)

Environment: 
Device: Google Nexus 7 (Android 4.4.2);
Build: Nightly 30.0a1 (2014-02-10);

Steps to reproduce:
1. Install a webapp (e.g. wikipedia);
2. Go to about:apps;
3. Tap on 'Check for Updates' button;

Expected result:
A proper message is displayed saying that the webapp is to date. 

Actual result:
The same version of the webapp is downloaded again even if there is no update available.
This is on purpose for testing while the dev server is in place. 

+// The URL of the service that checks for updates.
+// This currently points to the development server.
+// To test updates, set this to http://apk-update-checker.paas.allizom.org,
+// which is a test server that always reports all apps as having updates.
+pref("browser.webapps.updateCheckUrl", "http://dapk.net/app_updates");
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
(In reply to Aaron Train [:aaronmt] from comment #1)
> This is on purpose for testing while the dev server is in place. 

Erm, I'm not sure that's correct.


> +// The URL of the service that checks for updates.
> +// This currently points to the development server.
> +// To test updates, set this to http://apk-update-checker.paas.allizom.org,
> +// which is a test server that always reports all apps as having updates.
> +pref("browser.webapps.updateCheckUrl", "http://dapk.net/app_updates");

http://apk-update-checker.paas.allizom.org always reports all apps as having updates, but http://dapk.net/app_updates doesn't, as far as I know.

Perhaps ozten can confirm.
Flags: needinfo?(ozten.bugs)
Hm, comment seems backwards if that's the case?
I'll look into this, I'm not seeing the result I expected.

For reference:

curl -v -H "Content-Type: application/json" -X POST -d '{"installed": {"http://app.hackerwebapp.com/manifest.webapp":1392008314677, "http://www.audio29.com/news29.webapp":1391899603217}}' http://dapk.net/app_updates

should return no manifest urls, because those are the current version numbers for those apps.
Flags: needinfo?(ozten.bugs)
(In reply to Aaron Train [:aaronmt] from comment #3)
> Hm, comment seems backwards if that's the case?

Hmm, I'm unsure how so, but then I wrote the comment, so it's probably hard for me to see how it can be misinterpreted.  Can you suggest an edit?
Fixed, I had a merge issue, code is on master now. Deployed to dapk.net.
Re-opening as this not invalid; was fixed (comment #6), but I can still reproduce (huh?).

Environment: Nightly (02/10), I have Soundcloud as my sole application installed from Marketplace. I manually tap 'Check for Updates' and it's finding an update (default preferences in browser).
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Per bug fix in Bug#966562

Can you try again against http://dapk.net/application.apk (the default) the fix is not on stage yet.
Flags: needinfo?(flaviu.cos)
Can not reproduce the bug in build 30.0a1 (2014-02-20);
Tapping on "Check for Updates" button doesn't do any visible action.
Can not verify if the "Check for updates" functionality actually works because I can not install older versions of the available webapps.
Flags: needinfo?(flaviu.cos)
It seems that the connection to server is never established:

02-26 15:04:22.741: I/Gecko(15205): checkForUpdates
02-26 15:04:28.085: I/IdleService(15205): Get idle time: time since reset 5336 msec
02-26 15:04:28.085: I/IdleService(15205): Idle timer callback: current idle time 5336 msec
02-26 15:04:28.085: I/IdleService(15205): next timeout 9663 msec from now
02-26 15:04:28.085: I/IdleService(15205): SetTimerExpiryIfBefore: next timeout 9662 msec from now
02-26 15:04:28.085: I/IdleService(15205): reset timer expiry to 9672 msec from now
02-26 15:04:37.757: I/IdleService(15205): Get idle time: time since reset 15010 msec
02-26 15:04:37.757: I/IdleService(15205): Idle timer callback: current idle time 15010 msec
02-26 15:04:37.757: I/IdleService(15205): next timeout 4294967279990 msec from now
02-26 15:04:37.757: I/IdleService(15205): SetTimerExpiryIfBefore: next timeout 4294967279989 msec from now
02-26 15:04:37.757: I/IdleService(15205): reset timer expiry to 4294967279999 msec from now
02-26 15:04:37.757: I/IdleService(15205): Idle timer callback: tell observer 6d009350 user is idle
02-26 15:09:09.734: I/Gecko(15205): checkForUpdates
02-26 15:09:09.734: I/Gecko(15205): already checking for updates
Priority: -- → P1
Depends on: 982182
Severity: normal → blocker
The "already checking for updates" issue is probably bug 982182.  In my limited testing, this bug still seems reproducible, but I'll check again tomorrow and see if I can figure out whether it's a client or a server issue.
Assignee: nobody → myk
Status: REOPENED → ASSIGNED
Ranker was mentioned on the Triage as an example.

The manifest includes a redirect
http://m.ranker.com/manifest.webapp is a 302 to
http://www.ranker.com/manifest.webapp

On dapk.net the version of the APK is 1394650432

    curl -v -H "Content-Type: application/json" -X POST -d '{ "installed":{"http://m.ranker.com/manifest.webapp":1394650432}}' http://dapk.net/app_updates
    {"outdated":[]}

Couple places to look for bugs

1) Client sends wrong version #
2) Client uses re-directed url instead of original manifest url
This is a client-side bug.  There are two issues:

1. webapp.EventListener double-stringifies the hash of APK versions it returns.

2. WebappManager.checkForUpdates references the old "packageName" property instead of the new "apkPackageName" property to which it was changed in bug 958356 (presumably because I rolled the patch before the name change but landed it after the name change).

(Which goes to show the limits of testing against a testing server that always tells you there's an update: you don't end up testing what happens when there isn't an update.)

This patch fixes both issues.
Attachment #8390658 - Flags: review?(mark.finkle)
Blocking on regressing bug 958356 for posterity, even though it regressed a patch in progress rather than a committed change.

Blocking on regressing bug 946344 because the double-stringifying issue looks like more fallout from removing GeckoEventResponder: we used to set mCurrentResponse to a stringified representation of the "versions" array (i.e. the return value of getApkVersions); now we stuff "versions" into an object and pass it to EventDispatcher.setResponse, which stringifies the whole object; but we never removed the toString() call on the array.
Blocks: 958356, 946344
Keywords: regression
Attachment #8390658 - Flags: review?(mark.finkle) → review+
This is the Aurora version of the patch.  It only contains the property name fix, because the change that broke stringification didn't land on Aurora.

[Approval Request Comment]

Bug caused by (feature/regressing bug #): 
  bug 958356

User impact if declined: 
  Users will be prompted at least once per day to update all their apps.

Testing completed (on m-c, etc.): 
  I've tested this on a personal build, and it's inbound, but it hasn't merged
  to Central yet.

Risk to taking this patch (and alternatives if risky): 
  The risk is low, as the fix is small and obvious.

String or IDL/UUID changes made by this patch:
  None.
Attachment #8390702 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/471e30a53c71
Status: ASSIGNED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Attachment #8390702 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.