[Offline Pages]Allow multiple urls to be requested in internal page.

chrome/browser/ui/webui/offline_internals_ui.cc

 // 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.
 
 #include "chrome/browser/ui/webui/offline_internals_ui.h"
 
 #include <stdint.h>
 #include <stdlib.h>
+#include <sstream>

[michaelpg, 2016/08/14 21:31:20]

alphabetize

[romax, 2016/08/19 18:53:35]

removed c++ changes and moved to JS side.

+#include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/guid.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/android/offline_pages/offline_page_model_factory.h"
 #include "chrome/browser/android/offline_pages/request_coordinator_factory.h"
@@ skipping 293 matching lines @@
   result.AppendStrings(logs);
 
   ResolveJavascriptCallback(*callback_id, result);
 }
 
 void OfflineInternalsUIMessageHandler::HandleAddToRequestQueue(
     const base::ListValue* args) {
   const base::Value* callback_id;
   CHECK(args->Get(0, &callback_id));
 
-  std::string url;
-  CHECK(args->GetString(1, &url));
+  std::string request_urls;
+  CHECK(args->GetString(1, &request_urls));
 
   // To be visible in Downloads UI, these items need a well-formed GUID
   // and AsyncNamespace in their ClientId.
   std::ostringstream id_stream;
   id_stream << base::GenerateGUID();
 
-  ResolveJavascriptCallback(
-      *callback_id,
-      base::FundamentalValue(
-          request_coordinator_->SavePageLater(
-              GURL(url),
-              offline_pages::ClientId(offline_pages::kAsyncNamespace,
-                                      id_stream.str()),
-              true)));
+  std::vector<std::string> url_list;
+  std::stringstream ss(request_urls);

[michaelpg, 2016/08/14 21:31:19]

istringstream?

[romax, 2016/08/19 18:53:35]

Done.

+  std::string url;
+  while (getline(ss, url, ',') && (!url.empty())) {

[michaelpg, 2016/08/14 21:31:20]

nit: parens not needed around !url.empty()

[romax, 2016/08/19 18:53:35]

Done.

+    url_list.push_back(url);
+  }
+
+  for (auto url : url_list) {
+    ResolveJavascriptCallback(

[michaelpg, 2016/08/14 21:31:20]

it doesn't make sense to repeatedly "resolve" a promise -- perhaps the callback
should be called once, representing whether *all* requests succeeded?

[chili, 2016/08/15 18:27:08]

+1.  I believe ResolveJavascriptCallback might also error out if the callback id
is resolved multiple times

You'd need to change the method signature of the js side to accept multiple
status values though
Alternatively (may be slower), you can do the url parsing on js side and call
into here per url.  It may make error messages easier to render.

[romax, 2016/08/19 18:53:35]

I think i'm going with parsing the urls in JS side... that seems more reasonable
to me..

+        *callback_id,
+        base::FundamentalValue(
+            request_coordinator_->SavePageLater(
+                GURL(url),
+                offline_pages::ClientId(offline_pages::kAsyncNamespace,
+                                        id_stream.str()),
+                true)));
+  }
 }
 
 void OfflineInternalsUIMessageHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "deleteAllPages",
       base::Bind(&OfflineInternalsUIMessageHandler::HandleDeleteAllPages,
                  weak_ptr_factory_.GetWeakPtr()));
   web_ui()->RegisterMessageCallback(
       "deleteSelectedPages",
       base::Bind(&OfflineInternalsUIMessageHandler::HandleDeleteSelectedPages,
@@ skipping 56 matching lines @@
   html_source->AddResourcePath("offline_internals_browser_proxy.js",
                                IDR_OFFLINE_INTERNALS_BROWSER_PROXY_JS);
   html_source->SetDefaultResource(IDR_OFFLINE_INTERNALS_HTML);
 
   content::WebUIDataSource::Add(Profile::FromWebUI(web_ui), html_source);
 
   web_ui->AddMessageHandler(new OfflineInternalsUIMessageHandler());
 }
 
 OfflineInternalsUI::~OfflineInternalsUI() {}