Pull HostListDirectoryGrabber out to a separate class.

remoting/android/java/src/org/chromium/chromoting/HostListLoader.java

Index: remoting/android/java/src/org/chromium/chromoting/HostListLoader.java
diff --git a/remoting/android/java/src/org/chromium/chromoting/HostListLoader.java b/remoting/android/java/src/org/chromium/chromoting/HostListLoader.java
new file mode 100644
index 0000000000000000000000000000000000000000..4e5ab24744fc2f68fcc8762f1c6f96fe4ac5b465
--- /dev/null
+++ b/remoting/android/java/src/org/chromium/chromoting/HostListLoader.java
@@ -0,0 +1,180 @@
+// Copyright 2014 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.chromoting;
+
+import android.os.Handler;
+import android.os.HandlerThread;
+import android.os.Looper;
+import android.util.Log;
+
+import org.chromium.chromoting.jni.JniInterface;
+import org.json.JSONArray;
+import org.json.JSONException;
+import org.json.JSONObject;
+
+import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Scanner;
+
+/** Helper for fetching the host list. */
+public class HostListLoader {
+    public enum Error {
+        AUTH_FAILED,
+        NETWORK_ERROR,
+        SERVICE_UNAVAILABLE,
+        UNEXPECTED_RESPONSE,
+        UNKNOWN,
+    }
+
+    /** Callback for receiving the host list, or getting notified of an error. */
+    public interface Callback {
+        void onHostListReceived(Host[] hosts);
+        void onError(Error error);
+    }
+
+    /** Path from which to download a user's host list JSON object. */
+    private static final String HOST_LIST_PATH =
+            "https://www.googleapis.com/chromoting/v1/@me/hosts?key=";
+
+    /** Callback handler to be used for network operations. */
+    private Handler mNetworkThread;
+
+    /** Handler for main thread. */
+    private Handler mMainThread;
+
+    public HostListLoader() {
+        // Thread responsible for downloading the host list.
+        HandlerThread thread = new HandlerThread("network");
+        thread.start();
+        mNetworkThread = new Handler(thread.getLooper());
+
+        mMainThread = new Handler(Looper.getMainLooper());
+    }
+
+    /**
+      * Causes the host list to be fetched on a background thread. This should be called on the
+      * main thread, and callbacks will also be invoked on the main thread. On success,
+      * callback.onHostListReceived() will be called, otherwise callback.onError() will be called
+      * with an error-code describing the failure.
+      */
+    public void retrieveHostList(String authToken, Callback callback) {
+        final String authTokenFinal = authToken;
+        final Callback callbackFinal = callback;
+        mNetworkThread.post(new Runnable() {
+            @Override
+            public void run() {
+                doRetrieveHostList(authTokenFinal, callbackFinal);
+            }
+        });
+    }
+
+    private void doRetrieveHostList(String authToken, Callback callback) {
+        HttpURLConnection link = null;
+        try {
+            link = (HttpURLConnection)
+                    new URL(HOST_LIST_PATH + JniInterface.nativeGetApiKey()).openConnection();
+            link.addRequestProperty("client_id", JniInterface.nativeGetClientId());
+            link.addRequestProperty("client_secret", JniInterface.nativeGetClientSecret());
+            link.setRequestProperty("Authorization", "OAuth " + authToken);
+
+            // Listen for the server to respond.
+            int status = link.getResponseCode();
+            switch (status) {
+                case HttpURLConnection.HTTP_OK:  // 200
+                    break;
+                case HttpURLConnection.HTTP_UNAUTHORIZED:  // 401
+                    postError(callback, Error.AUTH_FAILED);
+                    return;
+                case HttpURLConnection.HTTP_BAD_GATEWAY:  // 502
+                case HttpURLConnection.HTTP_UNAVAILABLE:  // 503
+                    postError(callback, Error.SERVICE_UNAVAILABLE);
+                    return;
+                default:
+                    postError(callback, Error.UNKNOWN);
+                    return;
+            }
+
+            StringBuilder response = new StringBuilder();
+            Scanner incoming = new Scanner(link.getInputStream());
+            Log.i("auth", "Successfully authenticated to directory server");
+            while (incoming.hasNext()) {
+                response.append(incoming.nextLine());
+            }
+            incoming.close();
+
+            // Interpret what the directory server told us.

[Sergey Ulanov, 2014/02/14 08:55:20]

suggest rewording: "Parse directory response"

[Lambros, 2014/02/15 01:40:21]

Done.

+            JSONObject data = new JSONObject(String.valueOf(response)).getJSONObject("data");
+            JSONArray hostsJson = data.getJSONArray("items");
+            Log.i("hostlist", "Received host listing from directory server");
+
+            int index = 0;
+            ArrayList<Host> hostList = new ArrayList<Host>();
+            while (!hostsJson.isNull(index)) {
+                JSONObject hostJson = hostsJson.getJSONObject(index);
+                Host host = new Host();
+                host.name = hostJson.getString("hostName");
+                host.id = hostJson.getString("hostId");
+                host.jabberId = hostJson.getString("jabberId");

[Sergey Ulanov, 2014/02/14 08:55:20]

jabberId may not be set when the host was just registered. We shouldn't be
treating such cases as an error. Use optString() here.

I think ideally only hostName and hostId should be mandatory and this code
should handle gracefully the case when the other fields are missing.

[Lambros, 2014/02/15 01:40:21]

Done.

+                host.publicKey = hostJson.getString("publicKey");
+                host.isOnline = hostJson.getString("status").equals("ONLINE");
+                hostList.add(host);
+                ++index;
+            }
+
+            sortHosts(hostList);

[Sergey Ulanov, 2014/02/14 08:55:20]

Sorting should be UI feature. Move this to HostListAdapter?

[Sergey Ulanov, 2014/02/14 08:55:20]

This and code below it doesn't need to be in try-catch

[Lambros, 2014/02/15 01:40:21]

This was already decided in the CL here:
https://codereview.chromium.org/148313004
Jamie said there was only ever going to be one reasonable way to sort the list,
so we might as well do it just once at loading-time.

[Lambros, 2014/02/15 01:40:21]

Done.

+
+            final Callback callbackFinal = callback;
+            final Host[] hosts = hostList.toArray(new Host[hostList.size()]);
+            mMainThread.post(new Runnable() {
+                @Override
+                public void run() {
+                    callbackFinal.onHostListReceived(hosts);
+                }
+            });
+        } catch (MalformedURLException ex) {
+            // This should never happen.
+            Log.e("hostlist", "Unexpected error while fetching host list: " + ex);

[Sergey Ulanov, 2014/02/14 08:55:20]

Throw RuntimeException?

[Lambros, 2014/02/15 01:40:21]

Done.

+        } catch (JSONException ex) {

[Sergey Ulanov, 2014/02/14 08:55:20]

I suggest having two separate try-catch block. One for the HTTP request, and one
to parse the response. That way you wouldn't need to mix exception handling. 

[Lambros, 2014/02/15 01:40:21]

Done.

+            postError(callback, Error.UNEXPECTED_RESPONSE);
+        } catch (IOException ex) {
+            postError(callback, Error.NETWORK_ERROR);
+        } finally {
+            if (link != null) {
+                link.disconnect();
+            }
+        }
+    }
+
+    /** Posts error to callback on main thread. */
+    private void postError(Callback callback, Error error) {
+        final Callback callbackFinal = callback;
+        final Error errorFinal = error;
+        mMainThread.post(new Runnable() {
+            @Override
+            public void run() {
+                callbackFinal.onError(errorFinal);
+            }
+        });
+    }
+
+    private static void sortHosts(ArrayList<Host> hosts) {
+        Comparator<Host> compareHosts = new Comparator<Host>() {

[Sergey Ulanov, 2014/02/14 08:55:20]

nit: hostComparator

[Lambros, 2014/02/15 01:40:21]

Done.

+            public int compare(Host a, Host b) {
+                if (a.isOnline != b.isOnline) {
+                    return a.isOnline ? -1 : 1;
+                }
+                String aName = a.name.toUpperCase();
+                String bName = b.name.toUpperCase();
+                return aName.compareTo(bName);
+            }
+        };
+        Collections.sort(hosts, compareHosts);
+    }
+}