Closed Bug 594897 Opened 12 years ago Closed 12 years ago

Check for an update and launch it if available on Fennec restart

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(fennec2.0b1+)

VERIFIED FIXED
Tracking Status
fennec 2.0b1+ ---

People

(Reporter: alexp, Assigned: alexp)

References

Details

Attachments

(1 file, 3 obsolete files)

When the update service downloads an update it restarts Fennec.
We have to check for a downloaded update APK and launch it.
Assignee: nobody → alexp
Status: NEW → ASSIGNED
Summary: Check for and update and launch it if available on Fennec restart → Check for an update and launch it if available on Fennec restart
Attached patch Fix (obsolete) — Splinter Review
User may restart Fennec manually, so check for updates in GeckoApp.onCreate() on a normal start.
Attachment #473671 - Flags: review?(blassey.bugs)
Blocks: 587384
Comment on attachment 473671 [details] [diff] [review]
Fix

>+            // Update the status file
>+            String status;
>+            if (succeeded)
>+                status = "succeeded\n";
>+            else
>+                status = "failed\n";

failed should be followed by ": x" where x is the error.
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#1211

If you need to add a new error for Android due to the existing ones not being adequate please do so.
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#117

Here is where they are copied from
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/readstrings/errors.h#42
tracking-fennec: --- → ?
Flags: in-testsuite?
Flags: in-litmus?
Comment on attachment 473671 [details] [diff] [review]
Fix

>@@ -439,9 +441,81 @@ abstract public class GeckoApp
>             Log.i("GeckoAppJava", e.toString());
>         }
>         System.exit(0);
>     }
> 
>     public void handleNotification(String action, String alertName, String alertCookie) {
>         GeckoAppShell.handleNotification(action, alertName, alertCookie);
>     }
>+
>+    private void checkAndLaunchUpdate() {
>+        Log.i("GeckoAppJava", "Checking for an update");
>+
>+        boolean succeeded = false;
>+
>+        String updateDir = "/data/data/org.mozilla." + getAppName() + "/updates/0/";
>+        File updateFile = new File(updateDir + "update.apk");
>+
>+        if (updateFile.exists()) {
if (!updateFile.exists()) return;

>+            Log.i("GeckoAppJava", "Update is available!");
>+
>+            // Launch APK
>+            File downloadDir = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS);
>+            File updateFileToRun = new File(downloadDir, getAppName() + "-update.apk");
>+            try {
>+                if (moveFile(updateFile, updateFileToRun)) {
>+                    String amCmd = "/system/bin/am start -a android.intent.action.VIEW " +
>+                                   "-n com.android.packageinstaller/.PackageInstallerActivity -d file://" +
>+                                   updateFileToRun.getPath();
>+                    Log.i("GeckoAppJava", amCmd);
>+                    Runtime.getRuntime().exec(amCmd);
>+                    succeeded = true;
>+                } else {
>+                    Log.i("GeckoAppJava", "Cannot move the update file!");
>+                }
>+            } catch (Exception e) {
>+                Log.i("GeckoAppJava", e.toString());
>+            }
>+
>+            // Update the status file
>+            String status;
String status = stats ? "succeeded\n" : "failed\n";

>+            if (succeeded)
>+                status = "succeeded\n";
>+            else
>+                status = "failed\n";
>+
>+            File statusFile = new File(updateDir + "update.status");
>+            OutputStream outStream;
>+            try {
>+                byte[] buf = status.getBytes("UTF-8");
>+                outStream = new FileOutputStream(statusFile);
>+                outStream.write(buf, 0, buf.length);
>+                outStream.close();
>+            } catch (Exception e) {
>+                Log.i("GeckoAppJava", e.toString());
>+            }
>+        }
>+
>+        if (succeeded)
>+            System.exit(0);
>+    }
>+
>+    private static boolean moveFile(File fromFile, File toFile) {
Try using File.renameTo

>+        try {
>+            FileInputStream inStream = new FileInputStream(fromFile);
>+            FileOutputStream outStream = new FileOutputStream(toFile);
>+            byte[] buf = new byte[8192];
>+            int read = 0;
>+            while ((read = inStream.read(buf)) > 0) {
>+                outStream.write(buf, 0, read);
>+            }
>+            inStream.close();
>+            outStream.close();
>+
>+            fromFile.delete();
>+        } catch (Exception e) {
>+            Log.i("GeckoAppJava", e.toString());
>+            return false;
>+        }
>+        return true;
>+    }
> }
(In reply to comment #3)
> >+        if (updateFile.exists()) {
> if (!updateFile.exists()) return;

Right.
Originally I had some additional logic at the end of the function.

> String status = stats ? "succeeded\n" : "failed\n";

Hmm, now a comment for the failure code I'm adding will look ambiguous.

> >+    private static boolean moveFile(File fromFile, File toFile) {
> Try using File.renameTo

Unfortunately renameTo doesn't work across filesystems.
Attachment #473671 - Flags: review?(blassey.bugs)
Attached patch Fix v2 (obsolete) — Splinter Review
Implemented review comments.
Attachment #473671 - Attachment is obsolete: true
Attachment #474827 - Flags: review?(blassey.bugs)
Attached patch Fix v2 (real) (obsolete) — Splinter Review
Attachment #474827 - Attachment is obsolete: true
Attachment #474862 - Flags: review?(blassey.bugs)
Attachment #474827 - Flags: review?(blassey.bugs)
(In reply to comment #2)
> Comment on attachment 473671 [details] [diff] [review]
> Fix
> 
> >+            // Update the status file
> >+            String status;
> >+            if (succeeded)
> >+                status = "succeeded\n";
> >+            else
> >+                status = "failed\n";
> 
> failed should be followed by ": x" where x is the error.
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#1211
Rob, is this supposed to be the error text or the error code?
Comment on attachment 474862 [details] [diff] [review]
Fix v2 (real)


>+        if (updateFile.exists()) {
>+            Log.i("GeckoAppJava", "Update is available!");

as mwu suggested, return early here

>+            // Update the status file
>+            String status = statusCode == 0 ? "succeeded\n" : "failed: "+ statusCode + "\n";

I believe this is supposed to be an error string, not an error code. Rob, can you comment?
Comment on attachment 474862 [details] [diff] [review]
Fix v2 (real)


>+    private static boolean moveFile(File fromFile, File toFile) {
>+        try {
>+            FileInputStream inStream = new FileInputStream(fromFile);
>+            FileOutputStream outStream = new FileOutputStream(toFile);
>+            byte[] buf = new byte[8192];
>+            int read = 0;
>+            while ((read = inStream.read(buf)) > 0) {
>+                outStream.write(buf, 0, read);
>+            }
>+            inStream.close();
>+            outStream.close();
>+
>+            fromFile.delete();
>+        } catch (Exception e) {
>+            Log.i("GeckoAppJava", e.toString());
>+            return false;
>+        }
>+        return true;
>+    }
> }

This would be better if you created two FileChannel's and used transferFrom().

Additionally you could special case moving on the same file system and use renameTo(). Perhaps try that first and if it fails use the FileChannels.
Attachment #474862 - Flags: review?(blassey.bugs) → review-
(In reply to comment #8)
> >+            // Update the status file
> >+            String status = statusCode == 0 ? "succeeded\n" : "failed: "+ statusCode + "\n";
> 
> I believe this is supposed to be an error string, not an error code. Rob, can
> you comment?

It's obvious from here: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#1213:

        update.errorCode = parseInt(ary[1])
It would be best to avoid moving files across filesystems altogether. The SD card isn't necessarily fast or even there.
(In reply to comment #10)
> (In reply to comment #8)
> > >+            // Update the status file
> > >+            String status = statusCode == 0 ? "succeeded\n" : "failed: "+ statusCode + "\n";
> > 
> > I believe this is supposed to be an error string, not an error code. Rob, can
> > you comment?
> 
> It's obvious from here:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#1213:
> 
>         update.errorCode = parseInt(ary[1])


or here http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/updater.cpp#1393 (more obvious to the js-illiterate among us)
(In reply to comment #11)
> It would be best to avoid moving files across filesystems altogether. The SD
> card isn't necessarily fast or even there.

I guess we will figure this out later as a follow-up. The best way would be to make it possible for the installer to access the APK right from our updates dir.
Attached patch Fix v3Splinter Review
- Missed review comment.
- Use FileChannel.transferTo to move the file.
Attachment #474862 - Attachment is obsolete: true
Attachment #474951 - Flags: review?(blassey.bugs)
Attachment #474951 - Flags: review?(blassey.bugs) → review+
tracking-fennec: ? → 2.0b1+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Filed bug 597599 because if the updater fails, i.e, 'connection timed out' - there is no way to check for updates again
Verified:

Mozilla/5.0 (Android; Linux armv71; rv2.0b7pre) Gecko/20101007 Firefox/4.0b7pre Fennec/4.0b2pre

(technically 20101006 build)
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus?(nhirata.bugzilla)
You need to log in before you can comment on or make changes to this bug.