Bug 1270870 - Simplify guest profile management; r=snorp

Treat the guest profile as a custom profile with a special profile
directory. This lets us get rid of a lot of the guest profile handling
code. This patch does not attempt to restore to guest mode if Fennec is
killed during a guest session. A later patch will fix that behavior.

Patch renames GeckoProfile.getFromArgs to GeckoProfile.initFromArgs,
because the method now has possible side-effects (i.e. deleting stale
guest profiles). The new name also differentiates the method from the
GeckoProfile.get family of methods. initFromArgs now always returns a
profile (using the default profile if necessary), to make it easier for
its callers.
This commit is contained in:
Jim Chen 2016-05-24 01:11:16 -04:00
parent 953c2c7b71
commit b2f8c1d77f
4 changed files with 94 additions and 327 deletions

View File

@ -200,8 +200,6 @@ public class BrowserApp extends GeckoApp
private static final int TABS_ANIMATION_DURATION = 450;
public static final String GUEST_BROWSING_ARG = "--guest";
// Intent String extras used to specify custom Switchboard configurations.
private static final String INTENT_KEY_SWITCHBOARD_HOST = "switchboard-host";
@ -550,20 +548,6 @@ public class BrowserApp extends GeckoApp
final Intent intent = getIntent();
// Note that we're calling GeckoProfile.get *before GeckoApp.onCreate*.
// This means we're reliant on the logic in GeckoProfile to correctly
// look up our launch intent (via BrowserApp's Activity-ness) and pull
// out the arguments. Be careful if you change that!
final GeckoProfile p = GeckoProfile.get(this);
if (p != null && !p.inGuestMode()) {
// This is *only* valid because we never want to use the guest mode
// profile concurrently with a normal profile -- no syncing to it,
// no dual-profile usage, nothing. BrowserApp startup with a conventional
// GeckoProfile will cause the guest profile to be deleted.
GeckoProfile.maybeCleanupGuestProfile(this);
}
// This has to be prepared prior to calling GeckoApp.onCreate, because
// widget code and BrowserToolbar need it, and they're created by the
// layout, which GeckoApp takes care of.
@ -3636,23 +3620,25 @@ public class BrowserApp extends GeckoApp
}
public void showGuestModeDialog(final GuestModeDialog type) {
if ((type == GuestModeDialog.ENTERING) == getProfile().inGuestMode()) {
// Don't show enter dialog if we are already in guest mode; same with leaving.
return;
}
final Prompt ps = new Prompt(this, new Prompt.PromptCallback() {
@Override
public void onPromptFinished(String result) {
try {
int itemId = new JSONObject(result).getInt("button");
if (itemId == 0) {
String args = "";
if (type == GuestModeDialog.ENTERING) {
args = GUEST_BROWSING_ARG;
doRestart(GuestSession.GUEST_BROWSING_ARG);
} else {
GeckoProfile.leaveGuestSession(BrowserApp.this);
// Now's a good time to make sure we're not displaying the Guest Browsing notification.
GuestSession.hideNotification(BrowserApp.this);
// Now's a good time to make sure we're not displaying the
// Guest Browsing notification.
GuestSession.hideNotification(GeckoAppShell.getApplicationContext());
doRestart();
}
doRestart(args);
}
} catch (JSONException ex) {
Log.e(LOGTAG, "Exception reading guest mode prompt result", ex);

View File

@ -26,6 +26,7 @@ import org.mozilla.gecko.distribution.Distribution;
import org.mozilla.gecko.firstrun.FirstrunAnimationContainer;
import org.mozilla.gecko.mozglue.SafeIntentUtils;
import org.mozilla.gecko.preferences.DistroSharedPrefsImport;
import org.mozilla.gecko.util.FileUtils;
import org.mozilla.gecko.util.INIParser;
import org.mozilla.gecko.util.INISection;
@ -66,12 +67,10 @@ public final class GeckoProfile {
sAcceptDirectoryChanges = true;
}
// Used to "lock" the guest profile, so that we'll always restart in it
private static final String LOCK_FILE_NAME = ".active_lock";
public static final String DEFAULT_PROFILE = "default";
// Profile is using a custom directory outside of the Mozilla directory.
public static final String CUSTOM_PROFILE = "";
public static final String GUEST_PROFILE = "guest";
public static final String GUEST_PROFILE_DIR = "guest";
// Session store
private static final String SESSION_FILE = "sessionstore.js";
@ -81,11 +80,6 @@ public final class GeckoProfile {
private static final HashMap<String, GeckoProfile> sProfileCache = new HashMap<String, GeckoProfile>();
private static String sDefaultProfileName;
// Caches the guest profile dir.
private static File sGuestDir;
private static GeckoProfile sGuestProfile;
private static boolean sShouldCheckForGuestProfile = true;
private final String mName;
private final File mMozillaDir;
private final Context mApplicationContext;
@ -100,36 +94,28 @@ public final class GeckoProfile {
*/
private File mProfileDir;
// Caches whether or not a profile is "locked".
// Only used by the guest profile to determine if it should be reused or
// deleted on startup.
// These are volatile for an incremental improvement in thread safety,
// but this is not a complete solution for concurrency.
private volatile LockState mLocked = LockState.UNDEFINED;
private volatile boolean mInGuestMode;
private Boolean mInGuestMode;
// Constants to cache whether or not a profile is "locked".
private enum LockState {
LOCKED,
UNLOCKED,
UNDEFINED
};
/**
* Can return null.
*/
public static GeckoProfile getFromArgs(final Context context, final String args) {
if (GuestSession.shouldUse(context, args)) {
return GeckoProfile.getOrCreateGuestProfile(context);
public static GeckoProfile initFromArgs(final Context context, final String args) {
if (args != null && args.contains(GuestSession.GUEST_BROWSING_ARG)) {
return getGuestProfile(context);
}
if (args == null) {
return null;
// We never want to use the guest mode profile concurrently with a normal profile
// -- no syncing to it, no dual-profile usage, nothing. GeckoThread startup with
// a conventional GeckoProfile will cause the guest profile to be deleted and
// guest mode to reset.
if (getGuestDir(context).isDirectory()) {
final GeckoProfile guestProfile = getGuestProfile(context);
if (guestProfile != null) {
removeProfile(context, guestProfile);
}
}
String profileName = null;
String profilePath = null;
if (args.contains("-P")) {
if (args != null && args.contains("-P")) {
final Pattern p = Pattern.compile("(?:-P\\s*)(\\w*)(\\s*)");
final Matcher m = p.matcher(args);
if (m.find()) {
@ -137,7 +123,7 @@ public final class GeckoProfile {
}
}
if (args.contains("-profile")) {
if (args != null && args.contains("-profile")) {
final Pattern p = Pattern.compile("(?:-profile\\s*)(\\S*)(\\s*)");
final Matcher m = p.matcher(args);
if (m.find()) {
@ -146,13 +132,14 @@ public final class GeckoProfile {
}
if (profileName == null && profilePath == null) {
return null;
// Get the default profile for the Activity.
return getDefaultProfile(context);
}
return GeckoProfile.get(context, profileName, profilePath);
}
public static GeckoProfile getDefaultProfile(Context context) {
private static GeckoProfile getDefaultProfile(Context context) {
try {
return get(context, getDefaultProfileName(context));
@ -238,13 +225,7 @@ public final class GeckoProfile {
args = null;
}
final GeckoProfile fromArgs = GeckoProfile.getFromArgs(context, args);
if (fromArgs != null) {
return fromArgs;
}
// Otherwise, get the default profile for the Activity.
return getDefaultProfile(context);
return GeckoProfile.initFromArgs(context, args);
} else if (profileName == null) {
// If only profile dir was passed in, use custom (anonymous) profile.
@ -303,148 +284,25 @@ public final class GeckoProfile {
// Currently unused outside of testing.
@RobocopTarget
public static boolean removeProfile(Context context, String profileName) {
if (profileName == null) {
Log.w(LOGTAG, "Unable to remove profile: null profile name.");
return false;
}
final GeckoProfile profile = get(context, profileName);
if (profile == null) {
return false;
}
public static boolean removeProfile(final Context context, final GeckoProfile profile) {
final boolean success = profile.remove();
if (success) {
// Clear all shared prefs for the given profile.
GeckoSharedPrefs.forProfileName(context, profileName)
GeckoSharedPrefs.forProfileName(context, profile.getName())
.edit().clear().apply();
}
return success;
}
// Only public for access from tests.
private static File getGuestDir(final Context context) {
return context.getFileStreamPath(GUEST_PROFILE_DIR);
}
@RobocopTarget
public static GeckoProfile createGuestProfile(Context context) {
try {
// We need to force the creation of a new guest profile if we want it outside of the normal profile path,
// otherwise GeckoProfile.getDir will try to be smart and build it for us in the normal profiles dir.
getGuestDir(context).mkdir();
sShouldCheckForGuestProfile = true;
GeckoProfile profile = getGuestProfile(context);
// If we're creating this guest session over the keyguard, don't lock it.
// This will force the guest session to exit if the user unlocks their phone
// and starts Fennec.
profile.lock();
/*
* Now do the things that createProfileDirectory normally does --
* right now that's kicking off DB init.
*/
profile.enqueueInitialization(profile.getDir());
return profile;
} catch (Exception ex) {
Log.e(LOGTAG, "Error creating guest profile", ex);
}
return null;
}
public static void leaveGuestSession(Context context) {
GeckoProfile profile = getGuestProfile(context);
if (profile != null) {
profile.unlock();
}
}
private static File getGuestDir(Context context) {
if (sGuestDir == null) {
sGuestDir = context.getFileStreamPath("guest");
}
return sGuestDir;
}
/**
* Performs IO. Be careful of using this on the main thread.
*/
public static GeckoProfile getOrCreateGuestProfile(Context context) {
GeckoProfile p = getGuestProfile(context);
if (p == null) {
return createGuestProfile(context);
}
return p;
}
public static GeckoProfile getGuestProfile(Context context) {
if (sGuestProfile == null) {
if (sShouldCheckForGuestProfile) {
File guestDir = getGuestDir(context);
if (guestDir.exists()) {
sGuestProfile = get(context, GUEST_PROFILE, guestDir);
sGuestProfile.mInGuestMode = true;
} else {
sShouldCheckForGuestProfile = false;
}
}
}
return sGuestProfile;
}
public static boolean maybeCleanupGuestProfile(final Context context) {
final GeckoProfile profile = getGuestProfile(context);
if (profile == null) {
return false;
}
if (!profile.locked()) {
profile.mInGuestMode = false;
// If the guest dir exists, but it's unlocked, delete it
removeGuestProfile(context);
return true;
}
return false;
}
private static void removeGuestProfile(Context context) {
boolean success = false;
try {
File guestDir = getGuestDir(context);
if (guestDir.exists()) {
success = delete(guestDir);
}
} catch (Exception ex) {
Log.e(LOGTAG, "Error removing guest profile", ex);
}
if (success) {
// Clear all shared prefs for the guest profile.
GeckoSharedPrefs.forProfileName(context, GUEST_PROFILE)
.edit().clear().apply();
}
}
public static boolean delete(File file) throws IOException {
// Try to do a quick initial delete
if (file.delete())
return true;
if (file.isDirectory()) {
// If the quick delete failed and this is a dir, recursively delete the contents of the dir
String files[] = file.list();
for (String temp : files) {
File fileDelete = new File(file, temp);
delete(fileDelete);
}
}
// Even if this is a dir, it should now be empty and delete should work
return file.delete();
public static GeckoProfile getGuestProfile(final Context context) {
return get(context, CUSTOM_PROFILE, getGuestDir(context));
}
private GeckoProfile(Context context, String profileName, File profileDir, BrowserDB.Factory dbFactory) throws NoMozillaDirectoryException {
@ -472,89 +330,11 @@ public final class GeckoProfile {
return mDB;
}
// Warning, Changing the lock file state from outside apis will cause this to become out of sync
public boolean locked() {
if (mLocked != LockState.UNDEFINED) {
return mLocked == LockState.LOCKED;
}
boolean profileExists;
synchronized (this) {
profileExists = mProfileDir != null && mProfileDir.exists();
}
// Don't use getDir() as it will create a dir if none exists.
if (profileExists) {
File lockFile = new File(mProfileDir, LOCK_FILE_NAME);
boolean res = lockFile.exists();
mLocked = res ? LockState.LOCKED : LockState.UNLOCKED;
} else {
mLocked = LockState.UNLOCKED;
}
return mLocked == LockState.LOCKED;
}
public boolean lock() {
try {
// If this dir doesn't exist getDir will create it for us
final File lockFile = new File(getDir(), LOCK_FILE_NAME);
final boolean result = lockFile.createNewFile();
if (lockFile.exists()) {
mLocked = LockState.LOCKED;
} else {
mLocked = LockState.UNLOCKED;
}
return result;
} catch (IOException ex) {
Log.e(LOGTAG, "Error locking profile", ex);
}
mLocked = LockState.UNLOCKED;
return false;
}
public boolean unlock() {
final File profileDir;
synchronized (this) {
// Don't use getDir() as it will create a dir.
profileDir = mProfileDir;
}
if (profileDir == null || !profileDir.exists()) {
return true;
}
try {
final File lockFile = new File(profileDir, LOCK_FILE_NAME);
if (!lockFile.exists()) {
mLocked = LockState.UNLOCKED;
return true;
}
final boolean result = delete(lockFile);
if (result) {
mLocked = LockState.UNLOCKED;
} else {
mLocked = LockState.LOCKED;
}
return result;
} catch (IOException ex) {
Log.e(LOGTAG, "Error unlocking profile", ex);
}
mLocked = LockState.LOCKED;
return false;
}
@RobocopTarget
public boolean inGuestMode() {
return mInGuestMode;
}
private void setDir(File dir) {
if (dir != null && dir.exists() && dir.isDirectory()) {
synchronized (this) {
mProfileDir = dir;
mInGuestMode = null;
}
}
}
@ -568,6 +348,28 @@ public final class GeckoProfile {
return CUSTOM_PROFILE.equals(mName);
}
@RobocopTarget
public boolean inGuestMode() {
if (mInGuestMode != null) {
return mInGuestMode;
}
// Guest profile is just a custom profile with a special path.
if (!isCustomProfile()) {
mInGuestMode = false;
} else {
final Context context = GeckoAppShell.getApplicationContext();
try {
mInGuestMode = getDir().getCanonicalPath().equals(
getGuestDir(context).getCanonicalPath());
} catch (final IOException e) {
mInGuestMode = false;
}
}
return mInGuestMode;
}
/**
* Retrieves the directory backing the profile. This method acts
* as a lazy initializer for the GeckoProfile instance.
@ -924,16 +726,27 @@ public final class GeckoProfile {
private boolean remove() {
try {
synchronized (this) {
final File dir = getDir();
if (dir.exists()) {
delete(dir);
if (mProfileDir != null && mProfileDir.exists()) {
FileUtils.delete(mProfileDir);
}
if (isCustomProfile()) {
// Custom profiles don't have profile.ini sections that we need to remove.
return true;
}
try {
mProfileDir = findProfileDir();
} catch (NoSuchProfileException noSuchProfile) {
// If the profile doesn't exist, there's nothing left for us to do.
return false;
// If findProfileDir() succeeds, it means the profile was created
// through forceCreate(), so we set mProfileDir to null to enable
// forceCreate() to create the profile again.
findProfileDir();
mProfileDir = null;
} catch (final NoSuchProfileException e) {
// If findProfileDir() throws, it means the profile was not created
// through forceCreate(), and we have to preserve mProfileDir because
// it was given to us. In that case, there's nothing left to do here.
return true;
}
}

View File

@ -383,25 +383,16 @@ public class GeckoThread extends Thread {
private String addCustomProfileArg(String args) {
String profileArg = "";
if (mProfile != null && mProfile.inGuestMode()) {
profileArg = " -profile " + mProfile.getDir().getAbsolutePath();
// Make sure a profile exists.
final GeckoProfile profile = getProfile();
profile.getDir(); // call the lazy initializer
if (args == null || !args.contains(BrowserApp.GUEST_BROWSING_ARG)) {
profileArg += " " + BrowserApp.GUEST_BROWSING_ARG;
}
} else {
// Make sure a profile exists.
final GeckoProfile profile = getProfile();
profile.getDir(); // call the lazy initializer
// If args don't include the profile, make sure it's included.
if (args == null || !args.matches(".*\\B-(P|profile)\\s+\\S+.*")) {
if (profile.isCustomProfile()) {
profileArg = " -profile " + profile.getDir().getAbsolutePath();
} else {
profileArg = " -P " + profile.getName();
}
// If args don't include the profile, make sure it's included.
if (args == null || !args.matches(".*\\B-(P|profile)\\s+\\S+.*")) {
if (profile.isCustomProfile()) {
profileArg = " -profile " + profile.getDir().getAbsolutePath();
} else {
profileArg = " -P " + profile.getName();
}
}
@ -451,12 +442,7 @@ public class GeckoThread extends Thread {
public synchronized GeckoProfile getProfile() {
if (mProfile == null) {
final Context context = GeckoAppShell.getApplicationContext();
mProfile = GeckoProfile.getFromArgs(context, mArgs);
// fall back to default profile if we didn't load a specific one
if (mProfile == null) {
mProfile = GeckoProfile.getDefaultProfile(context);
}
mProfile = GeckoProfile.initFromArgs(context, mArgs);
}
return mProfile;
}

View File

@ -15,29 +15,11 @@ import android.view.Window;
import android.view.WindowManager;
// Utility methods for entering/exiting guest mode.
public class GuestSession {
public static final String NOTIFICATION_INTENT = "org.mozilla.gecko.GUEST_SESSION_INPROGRESS";
public final class GuestSession {
private static final String LOGTAG = "GeckoGuestSession";
/* Returns true if you should be in guest mode. This can be because a secure keyguard
* is locked, or because the user has explicitly started guest mode via a dialog. If the
* user has explicitly started Fennec in guest mode, this will return true until they
* explicitly exit it.
*/
public static boolean shouldUse(final Context context, final String args) {
// Did the command line args request guest mode?
if (args != null && args.contains(BrowserApp.GUEST_BROWSING_ARG)) {
return true;
}
// Otherwise, is there a locked guest mode profile?
final GeckoProfile profile = GeckoProfile.getGuestProfile(context);
if (profile == null) {
return false;
}
return profile.locked();
}
public static final String NOTIFICATION_INTENT = "org.mozilla.gecko.GUEST_SESSION_INPROGRESS";
public static final String GUEST_BROWSING_ARG = "--guest";
private static PendingIntent getNotificationIntent(Context context) {
Intent intent = new Intent(NOTIFICATION_INTENT);