[Android WebAPK] Make webapk_installer.cc return proto as base64 string

chrome/browser/android/webapk/webapk_installer.cc

Index: chrome/browser/android/webapk/webapk_installer.cc
diff --git a/chrome/browser/android/webapk/webapk_installer.cc b/chrome/browser/android/webapk/webapk_installer.cc
index 663093dd268ea2aad1fd27e9021fe38aae605a07..0e148d1739bd3702423e309df6834259a6e789d3 100644
--- a/chrome/browser/android/webapk/webapk_installer.cc
+++ b/chrome/browser/android/webapk/webapk_installer.cc
@@ -5,11 +5,13 @@
 #include "chrome/browser/android/webapk/webapk_installer.h"
 
 #include <utility>
+#include <vector>
 
 #include "base/android/build_info.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/path_utils.h"
+#include "base/base64.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
@@ -111,7 +113,7 @@ void SetImageData(webapk::Image* image, const SkBitmap& icon) {
 
 // Populates webapk::WebApk and returns it.
 // Must be called on a worker thread because it encodes an SkBitmap.
-std::unique_ptr<std::vector<uint8_t>> BuildProtoInBackground(
+std::unique_ptr<std::string> BuildProtoInBackground(
     const ShortcutInfo& shortcut_info,
     const SkBitmap& primary_icon,
     const SkBitmap& badge_icon,
@@ -174,12 +176,12 @@ std::unique_ptr<std::vector<uint8_t>> BuildProtoInBackground(
     image->set_hash(entry.second);
   }
 
-  size_t serialized_size = webapk->ByteSize();
-  std::unique_ptr<std::vector<uint8_t>> serialized_proto =
-      base::MakeUnique<std::vector<uint8_t>>();
-  serialized_proto->resize(serialized_size);
-  webapk->SerializeToArray(serialized_proto->data(), serialized_size);
-  return serialized_proto;
+  std::string serialized_proto;

[dominickn, 2017/06/20 01:07:21]

Base64Encode can be run in place, which will save a string allocation. Can you
do:

auto serialized_proto = base::MakeUnique<std::string>();
webapk->SerializeToString(&serialized_proto.get());
 base::Base64Encode(serialized_proto.get(), serialized_proto.get());

[pkotwicz, 2017/06/20 22:18:26]

I can do this change. However, this does not decrease the number of string
allocations since base::Base64Encode() does a copy internally

[dominickn, 2017/06/21 04:13:49]

I meant that you'll save one of |serialized_proto| and |base64_serialized_proto|
from being allocated when they're default-constructed (we only use one instead
of both). I'm not overly fussed though.

+  webapk->SerializeToString(&serialized_proto);
+  std::unique_ptr<std::string> base64_serialized_proto =
+      base::MakeUnique<std::string>();
+  base::Base64Encode(serialized_proto, base64_serialized_proto.get());
+  return base64_serialized_proto;
 }
 
 // Returns task runner for running background tasks.
@@ -210,13 +212,12 @@ void WebApkInstaller::InstallAsync(content::BrowserContext* context,
 }
 
 // static
-void WebApkInstaller::UpdateAsync(
-    content::BrowserContext* context,
-    const std::string& webapk_package,
-    const GURL& start_url,
-    const base::string16& short_name,
-    std::unique_ptr<std::vector<uint8_t>> serialized_proto,
-    const FinishCallback& finish_callback) {
+void WebApkInstaller::UpdateAsync(content::BrowserContext* context,
+                                  const std::string& webapk_package,
+                                  const GURL& start_url,
+                                  const base::string16& short_name,
+                                  std::unique_ptr<std::string> serialized_proto,
+                                  const FinishCallback& finish_callback) {
   // The installer will delete itself when it is done.
   WebApkInstaller* installer = new WebApkInstaller(context);
   installer->UpdateAsync(webapk_package, start_url, short_name,
@@ -238,7 +239,7 @@ void WebApkInstaller::UpdateAsyncForTesting(
     const std::string& webapk_package,
     const GURL& start_url,
     const base::string16& short_name,
-    std::unique_ptr<std::vector<uint8_t>> serialized_proto,
+    std::unique_ptr<std::string> serialized_proto,
     const FinishCallback& finish_callback) {
   installer->UpdateAsync(webapk_package, start_url, short_name,
                          std::move(serialized_proto), finish_callback);
@@ -264,8 +265,7 @@ void WebApkInstaller::BuildProto(
     const std::string& version,
     const std::map<std::string, std::string>& icon_url_to_murmur2_hash,
     bool is_manifest_stale,
-    const base::Callback<void(std::unique_ptr<std::vector<uint8_t>>)>&
-        callback) {
+    const base::Callback<void(std::unique_ptr<std::string>)>& callback) {
   base::PostTaskAndReplyWithResult(
       GetBackgroundTaskRunner().get(), FROM_HERE,
       base::Bind(&BuildProtoInBackground, shortcut_info, primary_icon,
@@ -367,12 +367,11 @@ void WebApkInstaller::InstallAsync(const ShortcutInfo& shortcut_info,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
-void WebApkInstaller::UpdateAsync(
-    const std::string& webapk_package,
-    const GURL& start_url,
-    const base::string16& short_name,
-    std::unique_ptr<std::vector<uint8_t>> serialized_proto,
-    const FinishCallback& finish_callback) {
+void WebApkInstaller::UpdateAsync(const std::string& webapk_package,
+                                  const GURL& start_url,
+                                  const base::string16& short_name,
+                                  std::unique_ptr<std::string> serialized_proto,
+                                  const FinishCallback& finish_callback) {
   webapk_package_ = webapk_package;
   start_url_ = start_url;
   short_name_ = short_name;
@@ -477,19 +476,19 @@ void WebApkInstaller::OnGotBadgeIconMurmur2Hash(
 }
 
 void WebApkInstaller::SendRequest(
-    std::unique_ptr<std::vector<uint8_t>> serialized_proto) {
+    std::unique_ptr<std::string> serialized_proto) {
   timer_.Start(
       FROM_HERE, base::TimeDelta::FromMilliseconds(webapk_server_timeout_ms_),
       base::Bind(&WebApkInstaller::OnResult, weak_ptr_factory_.GetWeakPtr(),
                  WebApkInstallResult::FAILURE));
 
-  std::string serialized_proto_string(serialized_proto->begin(),
-                                      serialized_proto->end());
+  std::string decoded_proto;
+  base::Base64Decode(*serialized_proto, &decoded_proto);
 
   url_fetcher_ =
       net::URLFetcher::Create(server_url_, net::URLFetcher::POST, this);
   url_fetcher_->SetRequestContext(request_context_getter_);
-  url_fetcher_->SetUploadData(kProtoMimeType, serialized_proto_string);
+  url_fetcher_->SetUploadData(kProtoMimeType, decoded_proto);
   url_fetcher_->SetLoadFlags(
       net::LOAD_DISABLE_CACHE | net::LOAD_DO_NOT_SEND_COOKIES |
       net::LOAD_DO_NOT_SAVE_COOKIES | net::LOAD_DO_NOT_SEND_AUTH_DATA);