[Offline pages] Extract internals page interaction with browser to a proxy

chrome/browser/resources/offline_pages/offline_internals_browser_proxy.js

+// Copyright 2016 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.
+
+/**
+ * @typedef {{
+ *   onlineUrl: string,
+ *   creationTime: number,
+ *   id: string,
+ *   namespace: string,
+ *   size: string,
+ *   filePath: string,
+ *   lastAccessTime: number,
+ *   accessCount: number
+ * }}
+ */
+var OfflinePage;
+
+/**
+ * @typedef {{
+ *   status: string,
+ *   onlineUrl: string,
+ *   creationTime: number,
+ *   id: string,
+ *   namespace: string,
+ *   lastAttempt: number
+ * }}
+ */
+var SavePageRequest;
+
+cr.define('offlineProxy', function() {

[dpapad, 2016/06/16 21:31:10]

Let's use the same "offlineInternals" namespace being used in
offline_internals.js.

[chili, 2016/06/16 22:31:29]

Done.

+  /** @interface */
+  function OfflineInternalsBrowserProxy() {}
+
+  OfflineInternalsBrowserProxy.prototype = {
+    /**
+     * Gets current list of stored pages.
+     * @return {!Promise} A promise firing when the list is fetched.

[dpapad, 2016/06/16 21:31:10]

!Promise<!Array<OfflinePage>>

Can we add more specific types on all returned Promises?

[chili, 2016/06/16 22:31:29]

Done.

+     */
+    onGetStoredPages: function() {},

[dpapad, 2016/06/16 21:31:10]

I don't think the 'on' prefix is appropriate here. It is usually used when a
function executes as a result of an event, for example onClick, onTap, etc. Here
you are directly requesting the stored pages, so getStoredPages() seems more
appropriate.

Same for all other methods in this interface.

[chili, 2016/06/16 22:31:29]

Done.

+
+    /**
+     * Gets current offline queue requests.
+     * @return {!Promise} A promise firing when the request queue is fetched.
+     */
+    onGetRequestQueue: function() {},
+
+    /**
+     * Deletes all the pages in stored pages.
+     * @return {!Promise} A promise firing when the pages are deleted.
+     */
+    onDeleteAllPages: function() {},
+
+    /**
+     * Deletes a set of pages from stored pages
+     * @param {!Array<string>} ids A list of page IDs to delete.
+     * @return {!Promise} A promise firing when the selected pages are deleted.
+     */
+    onDeleteSelectedPages: function(ids) {},
+  };
+
+  /**
+   * @constructor
+   * @implements {offlineProxy.OfflineInternalsBrowserProxy}
+   */
+  function OfflineInternalsBrowserProxyImpl() {}
+  cr.addSingletonGetter(OfflineInternalsBrowserProxyImpl);
+
+  OfflineInternalsBrowserProxyImpl.prototype = {
+    /** @override */
+    onGetStoredPages: function() {
+      return cr.sendWithPromise('getStoredPages');

[dpapad, 2016/06/16 21:31:10]

Following up on the previous point about naming, the convention we've been
following is to make the proxy method match the name of the string that is sent
to the C++ side, so 

getStoredPages: function() {
  return cr.sendWithPromise('getStoredPages');
};

[chili, 2016/06/16 22:31:29]

Done.

+    },
+
+    /** @override */
+    onGetRequestQueue: function() {
+      return cr.sendWithPromise('getRequestQueue');
+    },
+
+    /** @override */
+    onDeleteAllPages: function() {
+      return cr.sendWithPromise('deleteAllPages');
+    },
+
+    /** @override */
+    onDeleteSelectedPages: function(ids) {
+      return cr.sendWithPromise('deleteSelectedPages', ids);
+    }
+  };
+
+  return {
+    OfflineInternalsBrowserProxy: OfflineInternalsBrowserProxy,
+    OfflineInternalsBrowserProxyImpl: OfflineInternalsBrowserProxyImpl
+  };
+});