From 356986740cd4bff6fa9e5c04a95636fdf264e7c5 Mon Sep 17 00:00:00 2001 From: dfalcantara Date: Tue, 13 Jan 2015 18:16:43 -0800 Subject: [PATCH] Delete Incognito profile cookies immediately after notification Immediately deletes the Incognito profile's cookies from the cookie jar as soon as Profile is notified that a profile has been deleted. The CookiesFetcher will confirm the incognito profile is missing, then schedule them for deletion. Also does general cleaning and renaming to less confusing method names. BUG=447267 Review URL: https://codereview.chromium.org/833533004 Cr-Commit-Position: refs/heads/master@{#311393} --- .../chrome/browser/cookies/CookiesFetcher.java | 113 ++++++++++----------- .../chromium/chrome/browser/profiles/Profile.java | 16 ++- chrome/browser/android/cookies/cookies_fetcher.cc | 31 +++--- chrome/browser/android/cookies/cookies_fetcher.h | 30 +++--- 4 files changed, 102 insertions(+), 88 deletions(-) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/cookies/CookiesFetcher.java b/chrome/android/java/src/org/chromium/chrome/browser/cookies/CookiesFetcher.java index 816923cbd1f8..ef545450c59c 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/cookies/CookiesFetcher.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/cookies/CookiesFetcher.java @@ -4,6 +4,7 @@ package org.chromium.chrome.browser.cookies; +import android.content.Context; import android.os.AsyncTask; import android.util.Log; @@ -28,27 +29,29 @@ import javax.crypto.CipherInputStream; import javax.crypto.CipherOutputStream; /** - * Responsible for fetching, (de)serializing and restoring cookies between the - * CookieJar and an encrypted file storage. + * Responsible for fetching, (de)serializing, and restoring cookies between the CookieJar and an + * encrypted file storage. */ public class CookiesFetcher { /** The default file name for the encrypted cookies storage. */ - public static final String DEFAULT_COOKIE_FILE_NAME = "COOKIES.DAT"; + private static final String DEFAULT_COOKIE_FILE_NAME = "COOKIES.DAT"; - public static final String TAG = "CookiesFetcher"; + /** Used for logging. */ + private static final String TAG = "CookiesFetcher"; - // To make sure the current decipher key matches the one from - // before. Otherwise, we end up restoring some garbage data on a stale - // file. - // TODO(acleung): May be use real cryptographic integrity checks on the - // whole file instead. + /** + * Used to confirm that the current cipher key matches the previously used cipher key when + * restoring data. If this value cannot be read from the file, the file is likely garbage. + * TODO(acleung): May be use real cryptographic integrity checks on the whole file, instead. + */ private static final String MAGIC_STRING = "c0Ok135"; - // Full path of the file that we are fetching / loading - // cookies from. + /** Full path of the file that we are fetching/loading cookies from. */ private final String mFileName; + /** Native-side pointer. */ private final long mNativeCookiesFetcher; + private final CleanupReference mCleanupReference; /** @@ -62,71 +65,51 @@ public class CookiesFetcher { * counter part will hold a strong reference to this Java class so the GC * would not collect it until the callback has been invoked. */ - private CookiesFetcher(String filename) { + private CookiesFetcher(Context context) { mNativeCookiesFetcher = nativeInit(); - mFileName = filename; - mCleanupReference = new CleanupReference(this, - new DestroyRunnable(mNativeCookiesFetcher)); + mFileName = context.getFileStreamPath(DEFAULT_COOKIE_FILE_NAME).getAbsolutePath(); + mCleanupReference = new CleanupReference(this, new DestroyRunnable(mNativeCookiesFetcher)); } /** - * Asynchronously fetches cookies from the incognito profile and saving - * it to file specified in the constructor. + * Asynchronously fetches cookies from the incognito profile and saves them to a file. * - * @param filename Full path of the file. + * @param context Context for accessing the file system. */ - public static void fetchFromCookieJar(String filename) { + public static void persistCookies(Context context) { try { - new CookiesFetcher(filename).fetchFromCookieJarInternal(); + new CookiesFetcher(context).persistCookiesInternal(); } catch (RuntimeException e) { e.printStackTrace(); } } - private void fetchFromCookieJarInternal() { - nativeFetchFromCookieJar(mNativeCookiesFetcher); + private void persistCookiesInternal() { + nativePersistCookies(mNativeCookiesFetcher); } /** - * Synchronously fetches cookies from the file specified and populating - * the incognito profile with it. + * If an incognito profile exists, synchronously fetch cookies from the file specified and + * populate the incognito profile with it. Otherwise deletes the file and does not restore the + * cookies. * - * @param filename Full path of the file. + * @param context Context for accessing the file system. */ - public static void restoreToCookieJar(String filename) { + public static void restoreCookies(Context context) { try { - CookiesFetcher fetcher = new CookiesFetcher(filename); - if (deleteCookieJarIfNecessary(filename)) return; - fetcher.restoreToCookieJarInternal(); + CookiesFetcher fetcher = new CookiesFetcher(context); + if (deleteCookiesIfNecessary(context)) return; + fetcher.restoreCookiesInternal(); } catch (RuntimeException e) { e.printStackTrace(); } } - /** - * Ensure the incognito cookies are deleted when the incognito profile is gone. - * - * @param filename Full path of the file. - * @return Whether or not the cookies were deleted. - */ - public static boolean deleteCookieJarIfNecessary(String filename) { - try { - CookiesFetcher fetcher = new CookiesFetcher(filename); - if (Profile.getLastUsedProfile().hasOffTheRecordProfile()) return false; - fetcher.scheduleDeleteCookiesFile(); - } catch (RuntimeException e) { - e.printStackTrace(); - return false; - } - return true; - } - - private void restoreToCookieJarInternal() { - // Use an AsyncTask to read the cookies from disk to avoid strict - // mode violations. + private void restoreCookiesInternal() { new AsyncTask>() { @Override protected List doInBackground(Void... voids) { + // Read cookies from disk on a background thread to avoid strict mode violations. ArrayList cookies = new ArrayList(); DataInputStream in = null; try { @@ -177,10 +160,9 @@ public class CookiesFetcher { @Override protected void onPostExecute(List cookies) { - // onPostExecute is back on the UI thread. We can only access - // cookies and profiles on the UI thread. + // We can only access cookies and profiles on the UI thread. for (CanonicalCookie cookie : cookies) { - nativeRestoreToCookieJar(mNativeCookiesFetcher, + nativeRestoreCookies(mNativeCookiesFetcher, cookie.getUrl(), cookie.getName(), cookie.getValue(), @@ -198,6 +180,24 @@ public class CookiesFetcher { }.executeOnExecutor(AsyncTask.SERIAL_EXECUTOR); } + /** + * Ensure the incognito cookies are deleted when the incognito profile is gone. + * + * @param context Context for accessing the file system. + * @return Whether or not the cookies were deleted. + */ + public static boolean deleteCookiesIfNecessary(Context context) { + try { + CookiesFetcher fetcher = new CookiesFetcher(context); + if (Profile.getLastUsedProfile().hasOffTheRecordProfile()) return false; + fetcher.scheduleDeleteCookiesFile(); + } catch (RuntimeException e) { + e.printStackTrace(); + return false; + } + return true; + } + @CalledByNative private CanonicalCookie createCookie(String url, String name, String value, String domain, String path, long creation, long expiration, long lastAccess, boolean secure, @@ -256,8 +256,7 @@ public class CookiesFetcher { } /** - * Delete the cookies file. This happens when we detect that all incognito - * tabs have been closed. + * Delete the cookies file. Called when we detect that all incognito tabs have been closed. */ private void scheduleDeleteCookiesFile() { new AsyncTask() { @@ -292,8 +291,8 @@ public class CookiesFetcher { private native long nativeInit(); private static native void nativeDestroy(long nativeCookiesFetcher); - private native void nativeFetchFromCookieJar(long nativeCookiesFetcher); - private native void nativeRestoreToCookieJar(long nativeCookiesFetcher, + private native void nativePersistCookies(long nativeCookiesFetcher); + private native void nativeRestoreCookies(long nativeCookiesFetcher, String url, String name, String value, String domain, String path, long creation, long expiration, long lastAccess, boolean secure, boolean httpOnly, int priority); diff --git a/chrome/android/java/src/org/chromium/chrome/browser/profiles/Profile.java b/chrome/android/java/src/org/chromium/chrome/browser/profiles/Profile.java index f0f7f8440891..908863a8a76a 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/profiles/Profile.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/profiles/Profile.java @@ -4,18 +4,27 @@ package org.chromium.chrome.browser.profiles; +import android.content.Context; + +import org.chromium.base.ApplicationStatus; import org.chromium.base.CalledByNative; import org.chromium.base.VisibleForTesting; +import org.chromium.chrome.browser.cookies.CookiesFetcher; /** * Wrapper that allows passing a Profile reference around in the Java layer. */ public class Profile { + /** Whether this wrapper corresponds to an off the record Profile. */ + private final boolean mIsOffTheRecord; + + /** Pointer to the Native-side ProfileAndroid. */ private long mNativeProfileAndroid; private Profile(long nativeProfileAndroid) { mNativeProfileAndroid = nativeProfileAndroid; + mIsOffTheRecord = nativeIsOffTheRecord(mNativeProfileAndroid); } public static Profile getLastUsedProfile() { @@ -44,7 +53,7 @@ public class Profile { } public boolean isOffTheRecord() { - return nativeIsOffTheRecord(mNativeProfileAndroid); + return mIsOffTheRecord; } /** @@ -63,6 +72,11 @@ public class Profile { @CalledByNative private void onNativeDestroyed() { mNativeProfileAndroid = 0; + + if (mIsOffTheRecord) { + Context context = ApplicationStatus.getApplicationContext(); + CookiesFetcher.deleteCookiesIfNecessary(context); + } } @CalledByNative diff --git a/chrome/browser/android/cookies/cookies_fetcher.cc b/chrome/browser/android/cookies/cookies_fetcher.cc index 0d8f7ceefbd2..16cfbaf79e37 100644 --- a/chrome/browser/android/cookies/cookies_fetcher.cc +++ b/chrome/browser/android/cookies/cookies_fetcher.cc @@ -24,7 +24,7 @@ void CookiesFetcher::Destroy(JNIEnv* env, jobject obj) { delete this; } -void CookiesFetcher::FetchFromCookieJar(JNIEnv* env, jobject obj) { +void CookiesFetcher::PersistCookies(JNIEnv* env, jobject obj) { Profile* profile = ProfileManager::GetPrimaryUserProfile(); if (!profile->HasOffTheRecordProfile()) { // There is no work to be done. We might consider calling @@ -43,11 +43,11 @@ void CookiesFetcher::FetchFromCookieJar(JNIEnv* env, jobject obj) { // The rest must be done from the IO thread. content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, - base::Bind(&CookiesFetcher::FetchFromCookieJarInternal, + base::Bind(&CookiesFetcher::PersistCookiesInternal, base::Unretained(this), getter)); } -void CookiesFetcher::FetchFromCookieJarInternal( +void CookiesFetcher::PersistCookiesInternal( net::URLRequestContextGetter* getter) { DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO)); @@ -99,18 +99,19 @@ void CookiesFetcher::OnCookiesFetchFinished(const net::CookieList& cookies) { jobject_.Reset(); } -void CookiesFetcher::RestoreToCookieJar(JNIEnv* env, jobject obj, - jstring url, - jstring name, - jstring value, - jstring domain, - jstring path, - int64 creation, - int64 expiration, - int64 last_access, - bool secure, - bool httponly, - int priority) { +void CookiesFetcher::RestoreCookies(JNIEnv* env, + jobject obj, + jstring url, + jstring name, + jstring value, + jstring domain, + jstring path, + int64 creation, + int64 expiration, + int64 last_access, + bool secure, + bool httponly, + int priority) { Profile* profile = ProfileManager::GetPrimaryUserProfile(); if (!profile->HasOffTheRecordProfile()) { return; // Don't create it. There is nothing to do. diff --git a/chrome/browser/android/cookies/cookies_fetcher.h b/chrome/browser/android/cookies/cookies_fetcher.h index 595da1949ce9..e8462cc34ed1 100644 --- a/chrome/browser/android/cookies/cookies_fetcher.h +++ b/chrome/browser/android/cookies/cookies_fetcher.h @@ -32,25 +32,25 @@ class CookiesFetcher { void OnCookiesFetchFinished(const net::CookieList& cookies); // Fetches all cookies from the cookie jar. - void FetchFromCookieJar(JNIEnv* env, jobject obj); + void PersistCookies(JNIEnv* env, jobject obj); // Saves a cookie to the cookie jar. - void RestoreToCookieJar(JNIEnv* env, - jobject obj, - jstring url, - jstring name, - jstring value, - jstring domain, - jstring path, - int64 creation, - int64 expiration, - int64 last_access, - bool secure, - bool httponly, - int priority); + void RestoreCookies(JNIEnv* env, + jobject obj, + jstring url, + jstring name, + jstring value, + jstring domain, + jstring path, + int64 creation, + int64 expiration, + int64 last_access, + bool secure, + bool httponly, + int priority); private: - void FetchFromCookieJarInternal(net::URLRequestContextGetter* getter); + void PersistCookiesInternal(net::URLRequestContextGetter* getter); void RestoreToCookieJarInternal(net::URLRequestContextGetter* getter, const net::CanonicalCookie& cookie); -- 2.11.4.GIT