In the case of a page load error, save a second screenshot in onProgressChanged(100) to ensure that we save the substituted error page rather than the previous page that loaded. Also save the thumbnail for the page if the user creates the bookmark through the add bookmark menu or clicking the add bookmark icon on the bookmarks page.

Change-Id: Ib2b7692a45290f9ce372968e05cde8f6903e4572

6 files changed, 62 insertions, 16 deletions

diff --git a/src/com/android/browser/AddBookmarkPage.java b/src/com/android/browser/AddBookmarkPage.java
index 2a92dce..7878762 100644
--- a/src/com/android/browser/AddBookmarkPage.java
+++ b/src/com/android/browser/AddBookmarkPage.java
@@ -21,6 +21,7 @@ import android.content.ContentResolver;
 import android.content.Intent;
 import android.content.res.Resources;
 import android.database.Cursor;
+import android.graphics.Bitmap;
 import android.net.ParseException;
 import android.net.WebAddress;
 import android.os.Bundle;
@@ -46,6 +47,8 @@ public class AddBookmarkPage extends Activity {
     private boolean     mEditingExisting;
     private Bundle      mMap;
     private String      mTouchIconUrl;
+    private Bitmap      mThumbnail;
+    private String      mOriginalUrl;
 
     private View.OnClickListener mSaveBookmark = new View.OnClickListener() {
         public void onClick(View v) {
@@ -81,8 +84,9 @@ public class AddBookmarkPage extends Activity {
                 setTitle(R.string.edit_bookmark);
             }
             title = mMap.getString("title");
-            url = mMap.getString("url");
+            url = mOriginalUrl = mMap.getString("url");
             mTouchIconUrl = mMap.getString("touch_icon_url");
+            mThumbnail = (Bitmap) mMap.getParcelable("thumbnail");
         }
 
         mTitle = (EditText) findViewById(R.id.title);
@@ -154,7 +158,14 @@ public class AddBookmarkPage extends Activity {
                         getIntent().toString()).putExtras(mMap));
             } else {
                 final ContentResolver cr = getContentResolver();
-                Bookmarks.addBookmark(null, cr, url, title, true);
+
+                // Only use mThumbnail if url and mOriginalUrl are matches.
+                // Otherwise the user edited the url and the thumbnail no longer applies.
+                if (mOriginalUrl.equals(url)) {
+                    Bookmarks.addBookmark(null, cr, url, title, mThumbnail, true);
+                } else {
+                    Bookmarks.addBookmark(null, cr, url, title, null, true);
+                }
                 if (mTouchIconUrl != null) {
                     final Cursor c =
                             BrowserBookmarksAdapter.queryBookmarksForUrl(
diff --git a/src/com/android/browser/Bookmarks.java b/src/com/android/browser/Bookmarks.java
index 40aaa63..c8aaee7 100644
--- a/src/com/android/browser/Bookmarks.java
+++ b/src/com/android/browser/Bookmarks.java
@@ -21,12 +21,14 @@ import android.content.ContentUris;
 import android.content.ContentValues;
 import android.content.Context;
 import android.database.Cursor;
+import android.graphics.Bitmap;
 import android.net.Uri;
 import android.provider.Browser;
 import android.util.Log;
 import android.webkit.WebIconDatabase;
 import android.widget.Toast;
 
+import java.io.ByteArrayOutputStream;
 import java.util.Date;
 
 /**
@@ -47,13 +49,14 @@ import java.util.Date;
      *  @param cr The ContentResolver being used to add the bookmark to the db.
      *  @param url URL of the website to be bookmarked.
      *  @param name Provided name for the bookmark.
+     *  @param thumbnail A thumbnail for the bookmark.
      *  @param retainIcon Whether to retain the page's icon in the icon database.
      *          This will usually be <code>true</code> except when bookmarks are
      *          added by a settings restore agent.
      */
     /* package */ static void addBookmark(Context context,
             ContentResolver cr, String url, String name,
-            boolean retainIcon) {
+            Bitmap thumbnail, boolean retainIcon) {
         // Want to append to the beginning of the list
         long creationTime = new Date().getTime();
         // First we check to see if the user has already visited this
@@ -95,6 +98,7 @@ import java.util.Date;
             map.put(Browser.BookmarkColumns.CREATED, creationTime);
             map.put(Browser.BookmarkColumns.TITLE, name);
             map.put(Browser.BookmarkColumns.BOOKMARK, 1);
+            map.put(Browser.BookmarkColumns.THUMBNAIL, bitmapToBytes(thumbnail));
             cr.update(Browser.BOOKMARKS_URI, map,
                     "_id = " + cursor.getInt(0), null);
         } else {
@@ -125,6 +129,7 @@ import java.util.Date;
                 map.put(Browser.BookmarkColumns.CREATED, creationTime);
                 map.put(Browser.BookmarkColumns.BOOKMARK, 1);
                 map.put(Browser.BookmarkColumns.DATE, 0);
+                map.put(Browser.BookmarkColumns.THUMBNAIL, bitmapToBytes(thumbnail));
                 int visits = 0;
                 if (count > 0) {
                     // The user has already bookmarked, and possibly
@@ -199,4 +204,14 @@ import java.util.Date;
         }
         cursor.deactivate();
     }
-}
\ No newline at end of file
+
+    private static byte[] bitmapToBytes(Bitmap bm) {
+        if (bm == null) {
+            return null;
+        }
+
+        final ByteArrayOutputStream os = new ByteArrayOutputStream();
+        bm.compress(Bitmap.CompressFormat.PNG, 100, os);
+        return os.toByteArray();
+    }
+}
diff --git a/src/com/android/browser/BrowserActivity.java b/src/com/android/browser/BrowserActivity.java
index 160d6f6..444e98d 100644
--- a/src/com/android/browser/BrowserActivity.java
+++ b/src/com/android/browser/BrowserActivity.java
@@ -2350,9 +2350,10 @@ public class BrowserActivity extends Activity
         // FIXME: Would like to make sure there is actually something to
         // draw, but the API for that (WebViewCore.pictureReady()) is not
         // currently accessible here.
+
         ContentResolver cr = getContentResolver();
         final Cursor c = BrowserBookmarksAdapter.queryBookmarksForUrl(
-                cr, view.getOriginalUrl(), view.getUrl(), false);
+                cr, view.getOriginalUrl(), view.getUrl(), true);
         if (c != null) {
             boolean succeed = c.moveToFirst();
             ContentValues values = null;
@@ -2507,10 +2508,10 @@ public class BrowserActivity extends Activity
             // Reset the title and icon in case we stopped a provisional
             // load.
             resetTitleAndIcon(view);
+            updateScreenshot(view);
 
             // Update the lock icon image only once we are done loading
             updateLockIconToLatest();
-            updateScreenshot(view);
 
             // Performance probe
             if (false) {
@@ -2824,6 +2825,8 @@ public class BrowserActivity extends Activity
             Log.e(LOGTAG, "onReceivedError " + errorCode + " " + failingUrl
                     + " " + description);
 
+                mNeedExtraScreenShot = true;
+
             // We need to reset the title after an error.
             resetTitleAndRevertLockIcon();
         }
@@ -3134,9 +3137,16 @@ public class BrowserActivity extends Activity
             }
 
             if (newProgress == 100) {
-                // onProgressChanged() is called for sub-frame too while
-                // onPageFinished() is only called for the main frame. sync
-                // cookie and cache promptly here.
+                // onProgressChanged() may continue to be called after the main
+                // frame has finished loading, as any remaining sub frames
+                // continue to load. We'll only get called once though with
+                // newProgress as 100 when everything is loaded.
+                // (onPageFinished is called once when the main frame completes
+                // loading regardless of the state of any sub frames so calls
+                // to onProgressChanges may continue after onPageFinished has
+                // executed)
+
+                // sync cookies and cache promptly here.
                 CookieSyncManager.getInstance().sync();
                 if (mInLoad) {
                     mInLoad = false;
@@ -3146,6 +3156,14 @@ public class BrowserActivity extends Activity
                         hideFakeTitleBar();
                     }
                 }
+                if (mNeedExtraScreenShot) {
+                    // if there was an error loading this page, capture a new
+                    // screenshot to ensure that we get the correct thumbnail
+                    // as onPageFinished may have been called before the error
+                    // page was displayed.
+                    updateScreenshot(view);
+                    mNeedExtraScreenShot = false;
+                }
             } else if (!mInLoad) {
                 // onPageFinished may have already been called but a subframe
                 // is still loading and updating the progress. Reset mInLoad
@@ -4316,6 +4334,10 @@ public class BrowserActivity extends Activity
     private boolean mPageStarted;
     private boolean mActivityInPause = true;
 
+    // If the frame fails to load, we should snap a second screenshot
+    // to ensure that we get the right thumbnail (i.e. of the error page).
+    private boolean mNeedExtraScreenShot = false;
+
     private boolean mMenuIsDown;
 
     private static boolean mInTrace;
diff --git a/src/com/android/browser/BrowserBackupAgent.java b/src/com/android/browser/BrowserBackupAgent.java
index c239b12..fe3b7ea 100644
--- a/src/com/android/browser/BrowserBackupAgent.java
+++ b/src/com/android/browser/BrowserBackupAgent.java
@@ -162,7 +162,7 @@ public class BrowserBackupAgent extends BackupAgent {
                                 // Right now we do not reconstruct the db entry in its
                                 // entirety; we just add a new bookmark with the same data
                                 Bookmarks.addBookmark(null, getContentResolver(),
-                                        mark.url, mark.title, false);
+                                        mark.url, mark.title, null, false);
                             } else {
                                 Log.v(TAG, "Skipping extant url: " + mark.url);
                             }
diff --git a/src/com/android/browser/BrowserBookmarksAdapter.java b/src/com/android/browser/BrowserBookmarksAdapter.java
index 6b464cc..bbecab9 100644
--- a/src/com/android/browser/BrowserBookmarksAdapter.java
+++ b/src/com/android/browser/BrowserBookmarksAdapter.java
@@ -437,13 +437,11 @@ class BrowserBookmarksAdapter extends BaseAdapter {
             mCursor.moveToPosition(position - mExtraOffset);
             tv.setText(mCursor.getString(
                     Browser.HISTORY_PROJECTION_TITLE_INDEX));
-            byte[] data = mCursor.getBlob(
-                    Browser.HISTORY_PROJECTION_THUMBNAIL_INDEX);
-            if (data == null) {
+            Bitmap thumbnail = getBitmap(Browser.HISTORY_PROJECTION_THUMBNAIL_INDEX, position);
+            if (thumbnail == null) {
                 thumb.setImageResource(R.drawable.ic_launcher_shortcut_browser_bookmark);
             } else {
-                thumb.setImageBitmap(
-                        BitmapFactory.decodeByteArray(data, 0, data.length));
+                thumb.setImageBitmap(thumbnail);
             }
 
             return convertView;
diff --git a/src/com/android/browser/HistoryItem.java b/src/com/android/browser/HistoryItem.java
index 9d6dec7..51cb026 100644
--- a/src/com/android/browser/HistoryItem.java
+++ b/src/com/android/browser/HistoryItem.java
@@ -46,7 +46,7 @@ import android.widget.TextView;
                     boolean isChecked) {
                 if (isChecked) {
                     Bookmarks.addBookmark(mContext,
-                            mContext.getContentResolver(), mUrl, getName(), true);
+                            mContext.getContentResolver(), mUrl, getName(), null, true);
                 } else {
                     Bookmarks.removeFromBookmarks(mContext,
                             mContext.getContentResolver(), mUrl, getName());