webapps: introduce helper class to store extended set of data

chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java

Index: chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
new file mode 100644
index 0000000000000000000000000000000000000000..4f2c91c6730b7f9eadaffd79f3d4fa7edac10ee8
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
@@ -0,0 +1,157 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.webapps;
+
+import android.content.Context;
+import android.content.SharedPreferences;
+import android.graphics.Bitmap;
+import android.graphics.BitmapFactory;
+import android.os.AsyncTask;
+import android.text.TextUtils;
+import android.util.Base64;
+
+import java.io.ByteArrayOutputStream;
+
+/**
+ * This is a class used to store data about an installed webapp.
+ */
+public class WebappDataStorage {
+
+    private static final String KEY_SPLASH_ICON = "splash_icon";
+    private static final String KEY_LAST_ACCESSED = "last_accessed";

[mlamouri (slow - plz ping), 2015/08/26 13:41:04]

nit: last_used. "used" is a more common term. For example LRU/MRU.

[Lalit Maganti, 2015/08/26 21:20:41]

Done.

+
+    private final SharedPreferences mPreferences;
+
+    private WebappDataStorage(Context context, String webappId) {

[mlamouri (slow - plz ping), 2015/08/26 13:41:04]

style: unless the coding style says otherwise, I would move the private methods
after the public ones.

[Lalit Maganti, 2015/08/26 21:20:41]

Moved to be after the public static methods similar to WebappInfo.

+        mPreferences = context.getSharedPreferences("webapp_" + webappId,
+                Context.MODE_PRIVATE);
+    }
+
+    /**
+     * Opens an instance of WebappDataStorage for the webapp specified.
+     * Implementation detail: This WebappDataStorage object returned utilizes

[gone, 2015/08/26 21:03:09]

nits:

(1) Don't bother saying "Implementation detail:".  Callers of the function won't
care and it's unnecessary verbosity.

(2) The comment about using SharedPreference affects everything about the class,
so move it up to the class comment.

[Lalit Maganti, 2015/08/26 21:20:41]

(1) Ack.

(2) Done.

+     * SharedPreferences to persist data.
+     * @param context  The context to open the SharedPreferences.
+     * @param webappId The ID of the webapp which is being opened.
+     */
+    public static WebappDataStorage open(Context context, String webappId) {
+        WebappDataStorage storage = new WebappDataStorage(
+                context.getApplicationContext(), webappId);
+        storage.updateLastAccessedTime();
+        return storage;
+    }
+
+    /**
+     * Retrieve the time which this WebappDataStorage was last accessed (i.e. last opened using
+     * the open function) asynchrously.

[gone, 2015/08/26 21:03:09]

nit:
Be explicit, especially since this is a comment and they quickly drift away from
the function names during a refactoring.

the open function -> {@ref WebappDataStorage#open(Context, String)}

[gone, 2015/08/26 21:04:57]

er @link

[Lalit Maganti, 2015/08/26 21:20:41]

Done.

+     * @param context  The context to read the SharedPreferences file.
+     * @param webappId The ID of the webapp the accessed time is being read for.
+     * @param callback Called when the last accessed time has been retrieved.
+     */
+    public static WebappDataStorage getLastAccessedTime(Context context, String webappId,
+            final FetchCallback<String> callback) {
+        return new WebappDataStorage(context.getApplicationContext(), webappId);
+    }
+
+    /*
+     * Retrieve the splashcreen image associated with with the current webapp

[gone, 2015/08/26 21:03:09]

nit: If it's not too late, we should standardize on "splash screen".  That's the
correct term, and we've already got too many other screwed up single-word names
in these classes ("webapps", "homescreen").  This should be handled before it
becomes more onerous to fix later.

[Lalit Maganti, 2015/08/26 21:20:41]

Changed but see comment below about camel casing.

+     * asynchrously.
+     * @param callback Called when the splashscreen image has been retrieved.
+     *                 May be null if no image was found.
+     */
+    public void getSplashImage(final FetchCallback<Bitmap> callback) {
+        new BitmapFetchTask(KEY_SPLASH_ICON, callback).execute();
+    }
+
+    /*
+     * Update the information associated with the webapp with the specified
+     * data.
+     * @param splashscreenImage The image which should be shown on the splashcreen of the webapp.

[gone, 2015/08/26 21:03:09]

nit: splashcreen?

[Lalit Maganti, 2015/08/26 21:20:41]

Fixed.

+     */
+    public void update(final Bitmap splashscreenImage) {

[gone, 2015/08/26 21:03:09]

update() is vague.  Use updateSplashscreenImage() or updateSplashScreenImage()
since there's only one thing.  Change the function name when you actually have a
need to add more data instead of just a vague idea of future class usage.

[Lalit Maganti, 2015/08/26 21:20:41]

Changed to updateSplashscreenImage. However, if we are switching to "splash
screen" the second way makes more sense but would be inconsistent with the rest
of the file. Do you want me to change the camel casing everywhere?

[gone, 2015/08/26 21:56:14]

Ideally, yes.  That's one reason why we got stuck calling "web apps" "webapps"
-- we used Webapp instead of WebApp for everything.  It's also the same reason
why the code has "infobar" and "InfoBar" all over the place.  That can be done
as a cleanup CL to avoid bloating this CL up more, but it should be done sooner
rather than later.

[Lalit Maganti, 2015/08/26 22:33:53]

It's OK as this is the first patch that actually uses "splashscreen". I've
updated this one and will update the dependent ones as well to match.

+        new UpdateTask(splashscreenImage).execute();
+    }
+
+    private void getLastAccessedTime(final FetchCallback<String> callback) {
+        new AsyncTask<Void, Void, String>() {
+            @Override
+            protected final String doInBackground(Void... nothing) {
+                return mPreferences.getString(KEY_LAST_ACCESSED, null);
+            }
+
+            @Override
+            protected final void onPostExecute(String result) {
+                callback.run(result);
+            }
+        }.execute();
+    }
+
+    private WebappDataStorage updateLastAccessedTime() {
+        new AsyncTask<Void, Void, Void>() {
+            @Override
+            protected final Void doInBackground(Void... nothing) {
+                mPreferences.edit()
+                    .putLong(KEY_LAST_ACCESSED, System.currentTimeMillis())
+                    .commit();
+                return null;
+            }
+        }.execute();
+        return this;
+    }
+
+    /**
+     * Callback class used when items are requested from the storage.
+     */
+    public interface FetchCallback<T> {
+        public void run(T readObject);
+    }
+
+    private final class BitmapFetchTask extends AsyncTask<Void, Void, Bitmap> {
+
+        private final String mKey;
+
+        private final FetchCallback<Bitmap> mCallback;
+
+        public BitmapFetchTask(String key, FetchCallback<Bitmap> callback) {
+            mKey = key;
+            mCallback = callback;
+        }
+
+        @Override
+        protected final Bitmap doInBackground(Void... nothing) {
+            String icon = mPreferences.getString(mKey, null);
+            if (TextUtils.isEmpty(icon)) return null;
+
+            byte[] decoded = Base64.decode(icon, Base64.DEFAULT);
+            return BitmapFactory.decodeByteArray(decoded, 0, decoded.length);
+        }
+
+        @Override
+        protected final void onPostExecute(Bitmap result) {
+            mCallback.run(result);
+        }
+    }
+
+    private final class UpdateTask extends AsyncTask<Void, Void, Void> {
+
+        private final Bitmap mSplashImage;
+
+        public UpdateTask(Bitmap splashImage) {
+            mSplashImage = splashImage;
+        }
+
+        @Override
+        protected Void doInBackground(Void... nothing) {
+            ByteArrayOutputStream output = new ByteArrayOutputStream();
+            mSplashImage.compress(Bitmap.CompressFormat.PNG, 100, output);
+            String splashImageString = Base64.encodeToString(
+                    output.toByteArray(), Base64.DEFAULT);
+            mPreferences.edit()
+                .putString(KEY_SPLASH_ICON, splashImageString)
+                .commit();
+            return null;
+        }
+    }
+}