diff options
author | John Reck <jreck@google.com> | 2010-12-09 11:14:04 -0800 |
---|---|---|
committer | John Reck <jreck@google.com> | 2010-12-10 10:27:14 -0800 |
commit | 0ebd3ac69a76ec76d9caab65a1947f971242994e (patch) | |
tree | 81f41426123f7eb68636c53a91030460eea715f5 | |
parent | b77aa7a93d1993174a832d0f2c7b380d0e65a0a7 (diff) | |
download | packages_apps_browser-0ebd3ac69a76ec76d9caab65a1947f971242994e.zip packages_apps_browser-0ebd3ac69a76ec76d9caab65a1947f971242994e.tar.gz packages_apps_browser-0ebd3ac69a76ec76d9caab65a1947f971242994e.tar.bz2 |
Fixes history update race condition
Bug: 3270709
Since ContentResolver does not have transaction support, I moved
history updates to a controller that can serialize updates to the
history table. This prevents the race condition.
Change-Id: Ic33bedb9d6faef2393379306f8f88778d16caf24
-rw-r--r-- | src/com/android/browser/Controller.java | 62 | ||||
-rw-r--r-- | src/com/android/browser/DataController.java | 118 |
2 files changed, 122 insertions, 58 deletions
diff --git a/src/com/android/browser/Controller.java b/src/com/android/browser/Controller.java index 4b341da..0227621 100644 --- a/src/com/android/browser/Controller.java +++ b/src/com/android/browser/Controller.java @@ -195,6 +195,7 @@ public class Controller // Checks to see when the bookmarks database has changed, and updates the // Tabs' notion of whether they represent bookmarked sites. private ContentObserver mBookmarksObserver; + private DataController mDataController; private static class ClearThumbnails extends AsyncTask<File, Void, Void> { @Override @@ -213,6 +214,7 @@ public class Controller public Controller(Activity browser) { mActivity = browser; mSettings = BrowserSettings.getInstance(); + mDataController = DataController.getInstance(mActivity); mTabControl = new TabControl(this); mSettings.setController(this); @@ -843,42 +845,7 @@ public class Controller } // Update the title in the history database if not in private browsing mode if (!tab.isPrivateBrowsingEnabled()) { - new AsyncTask<Void, Void, Void>() { - @Override - protected Void doInBackground(Void... unused) { - // See if we can find the current url in our history - // database and add the new title to it. - String url = pageUrl; - if (url.startsWith("http://www.")) { - url = url.substring(11); - } else if (url.startsWith("http://")) { - url = url.substring(4); - } - // Escape wildcards for LIKE operator. - url = url.replace("\\", "\\\\").replace("%", "\\%") - .replace("_", "\\_"); - Cursor c = null; - try { - final ContentResolver cr = - getActivity().getContentResolver(); - String selection = History.URL + " LIKE ? ESCAPE '\\'"; - String [] selectionArgs = new String[] { "%" + url }; - ContentValues values = new ContentValues(); - values.put(History.TITLE, title); - cr.update(History.CONTENT_URI, values, selection, - selectionArgs); - } catch (IllegalStateException e) { - Log.e(LOGTAG, "Tab onReceived title", e); - } catch (SQLiteException ex) { - Log.e(LOGTAG, - "onReceivedTitle() caught SQLiteException: ", - ex); - } finally { - if (c != null) c.close(); - } - return null; - } - }.execute(); + mDataController.updateHistoryTitle(pageUrl, title); } } @@ -924,28 +891,7 @@ public class Controller if (url.regionMatches(true, 0, "about:", 0, 6)) { return; } - // remove "client" before updating it to the history so that it wont - // show up in the auto-complete list. - int index = url.indexOf("client=ms-"); - if (index > 0 && url.contains(".google.")) { - int end = url.indexOf('&', index); - if (end > 0) { - url = url.substring(0, index) - .concat(url.substring(end + 1)); - } else { - // the url.charAt(index-1) should be either '?' or '&' - url = url.substring(0, index-1); - } - } - final ContentResolver cr = getActivity().getContentResolver(); - final String newUrl = url; - new AsyncTask<Void, Void, Void>() { - @Override - protected Void doInBackground(Void... unused) { - Browser.updateVisitedHistory(cr, newUrl, true); - return null; - } - }.execute(); + mDataController.updateVisitedHistory(url); WebIconDatabase.getInstance().retainIconForPageUrl(url); } diff --git a/src/com/android/browser/DataController.java b/src/com/android/browser/DataController.java new file mode 100644 index 0000000..be38d70 --- /dev/null +++ b/src/com/android/browser/DataController.java @@ -0,0 +1,118 @@ +/* + * Copyright (C) 2010 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +package com.android.browser; + +import android.content.ContentResolver; +import android.content.ContentUris; +import android.content.ContentValues; +import android.content.Context; +import android.database.Cursor; +import android.os.Handler; +import android.os.HandlerThread; +import android.os.Looper; +import android.os.Message; +import android.provider.BrowserContract.History; + +public class DataController { + // Message IDs + private static final int HISTORY_UPDATE_VISITED = 100; + private static final int HISTORY_UPDATE_TITLE = 101; + private static DataController sInstance; + + private Context mContext; + private Handler mHandler; + + /* package */ static DataController getInstance(Context c) { + if (sInstance == null) { + sInstance = new DataController(c); + } + return sInstance; + } + + private DataController(Context c) { + mContext = c.getApplicationContext(); + HandlerThread thread = new HandlerThread("DataController"); + thread.setDaemon(true); + thread.start(); + mHandler = new DataControllerHandler(thread.getLooper()); + } + + public void updateVisitedHistory(String url) { + mHandler.obtainMessage(HISTORY_UPDATE_VISITED, url).sendToTarget(); + } + + public void updateHistoryTitle(String url, String title) { + mHandler.obtainMessage(HISTORY_UPDATE_TITLE, new String[] { url, title }) + .sendToTarget(); + } + + class DataControllerHandler extends Handler { + public DataControllerHandler(Looper looper) { + super(looper); + } + + @Override + public void handleMessage(Message msg) { + switch (msg.what) { + case HISTORY_UPDATE_VISITED: + doUpdateVisitedHistory((String) msg.obj); + break; + case HISTORY_UPDATE_TITLE: + String[] args = (String[]) msg.obj; + doUpdateHistoryTitle(args[0], args[1]); + break; + } + } + } + + private void doUpdateVisitedHistory(String url) { + ContentResolver cr = mContext.getContentResolver(); + Cursor c = null; + try { + c = cr.query(History.CONTENT_URI, new String[] { History._ID, History.VISITS }, + History.URL + "=?", new String[] { url }, null); + if (c.moveToFirst()) { + ContentValues values = new ContentValues(); + values.put(History.VISITS, c.getInt(1) + 1); + values.put(History.DATE_LAST_VISITED, System.currentTimeMillis()); + cr.update(ContentUris.withAppendedId(History.CONTENT_URI, c.getLong(0)), + values, null, null); + } else { + android.provider.Browser.truncateHistory(cr); + ContentValues values = new ContentValues(); + values.put(History.URL, url); + values.put(History.VISITS, 1); + values.put(History.DATE_LAST_VISITED, System.currentTimeMillis()); + values.put(History.TITLE, url); + values.put(History.DATE_CREATED, 0); + values.put(History.USER_ENTERED, 0); + cr.insert(History.CONTENT_URI, values); + } + } finally { + if (c != null) c.close(); + } + } + + private void doUpdateHistoryTitle(String url, String title) { + ContentResolver cr = mContext.getContentResolver(); + ContentValues values = new ContentValues(); + values.put(History.TITLE, title); + cr.update(History.CONTENT_URI, values, History.URL + "=?", + new String[] { url }); + } +} |