Closed Bug 615519 Opened 14 years ago Closed 13 years ago

profile data should be stored on sd card when application is moved to sd card

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(fennec2.0+)

VERIFIED FIXED
Tracking Status
fennec 2.0+ ---

People

(Reporter: dietrich, Assigned: blassey)

References

Details

Attachments

(1 file, 4 obsolete files)

currently there's 37mb of fennec data on my primary storage, by far the phattest app data according to DiskUsage app.

this should be kept on the sd card.

what is this data? cache? user data? is there a way to clear the cache?
(In reply to comment #0)
> currently there's 37mb of fennec data on my primary storage, by far the
> phattest app data according to DiskUsage app.
> 
> this should be kept on the sd card.

Android should allow this using:  Settings > Applications > Fennec > Move to SD Card

Might only be for newer versions of Android

> what is this data?

Your profile

> cache? user data?

No cache for Fennec. Most likely it is the places DB.

> is there a way to clear the cache?

You can clear your profile using the Settings app as well _or_ using Fennec. In Fennec, use Preferences > Clear private data
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WORKSFORME
Re-opening.

(In reply to comment #1)
> (In reply to comment #0)
> > currently there's 37mb of fennec data on my primary storage, by far the
> > phattest app data according to DiskUsage app.
> > 
> > this should be kept on the sd card.
> 
> Android should allow this using:  Settings > Applications > Fennec > Move to SD
> Card
> 
> Might only be for newer versions of Android

Yes, for 2.2 and up IIRC.

I'd already done this. DiskUsage shows that Fennec is 128k in the Apps2SD folder on the card. Fennec is 31.6mb on internal storage.

DiskUsage also says whether the file is "data" or "apk". It says "apk" for the tiny bit on SD card, and "data" for the stuff on internal storage. This confused me.

I opened the application properties for Fennec and it says close the same numbers.

> > is there a way to clear the cache?
> 
> You can clear your profile using the Settings app as well _or_ using Fennec. In
> Fennec, use Preferences > Clear private data

Uhh, that would be bad. If CPD means the same thing as on desktop Firefox, It'd blow away a bunch of stuff I want. This is very very different than clearing the browser cache.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
(In reply to comment #2)
> Re-opening.

OK, we can investigate.

> > > is there a way to clear the cache?
> > 
> > You can clear your profile using the Settings app as well _or_ using Fennec. In
> > Fennec, use Preferences > Clear private data
> 
> Uhh, that would be bad. If CPD means the same thing as on desktop Firefox, It'd
> blow away a bunch of stuff I want. This is very very different than clearing
> the browser cache.

Fennec has zero browser cache. The profile size is entirely driven by data in places and cookies (a little) databases.

CPD means the same thing as desktop Firefox, so be wary.
Talked to mbrubeck about this on IRC a bit, and apparently it's by-design for security reasons that the user profile isn't stored on the SD card, since the SD card isn't protected by the Android system, meaning other applications can read/write into that space.

While I understand that instinct, and further understand that the newer generations of Android phones are not as limited as the Nexus One and its contemporaries in terms of the internal/SD memory split, I can also tell you that as a Nexus One owner, this really does prevent me from using Fennec as intended. My choices are:

 - Fennec + Sync and a limited number of other applications
 - Fennec w/o Sync and my applications
 - No Fennec and my applications

Possible remedies are:

 - move the profile to the SD card for all phones, in a "salt"ed directory to obfuscate it a bit (note: we have the same security risk on desktop, here)
 - move the profile to the SD card for certain models of phones
 - add an option to trim the local history/bookmark DB to a certain size (right now my DB is large, I suspect, because it's Syncing with my desktop DB)

Or, of course, we could ignore this problem entirely. That's an option, but we should understand the cost of that option. Do we know what the spectrum of internal memory sizes of phones that meet Fennec's minimum requirements?
Nominating for product management decision, and to ensure the costs of the security-based decision here is weighed against the user experience.
tracking-fennec: --- → ?
Summary: data should be stored on sd card → profile data should be stored on sd card when application is moved to sd card
tracking-fennec: ? → 2.0+
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → blassey.bugs
Status: REOPENED → NEW
Attachment #509983 - Flags: review?(doug.turner)
Comment on attachment 509983 [details] [diff] [review]
patch

Since the profiles.ini is stored in the profile folder itself, I guess it would be moved to the new location. All profiles should be stored relative to the root profile folder, so the "IsRelative" flag in the profiles.ini file will not need to be updated. Any absolute profile paths should "just work".
Attached patch patch (obsolete) — Splinter Review
cleaned up a few things
Attachment #509983 - Attachment is obsolete: true
Attachment #510313 - Flags: review?(doug.turner)
Attachment #509983 - Flags: review?(doug.turner)
Attached patch patch (obsolete) — Splinter Review
preserve last modified date
Attachment #510313 - Attachment is obsolete: true
Attachment #510328 - Flags: review?(doug.turner)
Attachment #510313 - Flags: review?(doug.turner)
Comment on attachment 510328 [details] [diff] [review]
patch


> import java.nio.*;
> import java.lang.reflect.*;
> import java.text.*;
>+import java.nio.channels.*;

Nit:  Move this new import under the import for java.nio.*
> 
> import android.os.*;
> import android.app.*;
>@@ -117,15 +118,93 @@ class GeckoAppShell
>         return sFreeSpace;
>     }
> 
>+    static boolean moveFile(File inFile, File outFile)
>+    {
>+        Log.i("GeckoAppShell", "moving " + inFile + " to " + outFile);
>+        if (inFile.renameTo(outFile))
>+            return true;

i think you want a try around this -- as well as the return check.  Mostly worried about a security exception.

http://developer.android.com/reference/java/io/File.html#renameTo%28java.io.File%29


>+        try {
>+            long lastModified = inFile.lastModified();
>+            outFile.createNewFile();
>+            // so copy it instead
>+            FileChannel inChannel = new FileInputStream(inFile).getChannel();
>+            FileChannel outChannel = new FileOutputStream(outFile).getChannel();
>+            long size = inChannel.size();
>+            long transferred = inChannel.transferTo(0, size, outChannel);
>+            inChannel.close();
>+            outChannel.close();
>+            outFile.setLastModified(lastModified);
>+            
>+            if (transferred == size)
>+                inFile.delete();
>+        } catch (Exception e) {
>+            Log.e("GeckoAppShell",
>+                  "exception while moving file: ", e);
>+            return false;
>+        }
>+        return true;
>+    }

If anything after createNewFile() throws, you will want to delete outFile.

You will probably want to test for the file type (is the source a directory), that you aren't copy to the same location (think about symlinks).

if transferred != size, you will want to also return false.

you probably also want to fflush().

>+
>+    static boolean moveDir(File from, File to) {
>+        to.mkdirs();
>+        if (from.renameTo(to))
>+            return true;

same comment about a security try/catch above.


>+        File[] files = from.listFiles();
>+        boolean retVal = true;
>+        if (files != null) {

nit: early return, save us from intending.

>+            Iterator fileIterator = Arrays.asList(files).iterator();
>+            while (fileIterator.hasNext()) {
>+                File file = (File)fileIterator.next();
>+                File dest = new File(to, file.getName());
>+                if (file.isDirectory())
>+                    retVal = moveDir(file, dest) ? retVal : false;
>+                else
>+                    retVal = moveFile(file, dest) ? retVal : false;
>+            }
>+        }

you need a try around this to catch and cleanup any problem when attempting to move.



>+        GeckoApp geckoApp = GeckoApp.mAppContext;
>+        String homeDir;
>+        if (geckoApp.getApplication().getPackageResourcePath().startsWith("/data")) {
>+            File home = geckoApp.getFilesDir();
>+            homeDir = home.getPath();
>+            // handle the application being moved to phone from sdcard
>+            File profileDir = new File(homeDir, "mozilla");
>+            File oldHome = new File("/data/data/" + 
>+                        GeckoApp.mAppContext.getPackageName() + "/mozilla");
>+            if (oldHome.exists())
>+                moveDir(oldHome, profileDir);
> 
>-                
>-        Intent i = GeckoApp.mAppContext.getIntent();
>+            File extHome =  geckoApp.getExternalFilesDir(null);
>+            File extProf = new File (extHome, "mozilla");
>+            if (extHome.exists())
>+                moveDir(extProf, profileDir);
>+        } else {
>+            File home = geckoApp.getExternalFilesDir(null);
>+            homeDir = home.getPath();
>+            // handle the application being moved to phone from sdcard
>+            File profileDir = new File(homeDir, "mozilla");
>+            File oldHome = new File("/data/data/" + 
>+                        GeckoApp.mAppContext.getPackageName() + "/mozilla");
>+            if (oldHome.exists())
>+                moveDir(oldHome, profileDir);
>+
>+            File intHome =  geckoApp.getFilesDir();
>+            File intProf = new File(intHome, "mozilla");
>+            if (intHome.exists())
>+                moveDir(intProf, profileDir);
>+        }
>+        GeckoAppShell.putenv("HOME=" + homeDir);
>+        Intent i = geckoApp.getIntent();
>         String env = i.getStringExtra("env0");
>         Log.i("GeckoApp", "env0: "+ env);
>         for (int c = 1; env != null; c++) {


There are a couple places where you're hard coding "mozilla".  Should we take this time to make this configurable?
Attachment #510328 - Flags: review?(doug.turner) → review-
Attached patch patch (obsolete) — Splinter Review
Attachment #510328 - Attachment is obsolete: true
Attachment #510476 - Flags: review?(doug.turner)
Attached patch patchSplinter Review
Attachment #510476 - Attachment is obsolete: true
Attachment #510478 - Flags: review?(doug.turner)
Attachment #510476 - Flags: review?(doug.turner)
Do we need to worry about making the profile "move" be atomic? An error or failure in the middle would leave a profile ripped across two locations.
Attachment #510478 - Flags: review?(doug.turner) → review+
pushed http://hg.mozilla.org/mozilla-central/rev/70f37cb8fdde
Status: NEW → RESOLVED
Closed: 14 years ago13 years ago
Resolution: --- → FIXED
Depends on: 632908
Depends on: 634530
Before the move to SD Card, I have:
Total: 21.62MB
Application: 13.79MB
Data: 7.83MB

After the move to SD Card, I have:
Total: 160KB
Application: 128KB
Data: 32.00KB

This is on the Motorola Droid, using yesterday's trunk build of Fennec.

Does this data make sense? How can Fennec only occupy 160KB on the SD Card?
Aaron explained to me that that is the data that remains on the phone, after almost everything has moved to the SD car. So marking verified fixed then.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.