Fetch ARC Kiosk app name and icon from Android side.

chrome/browser/chromeos/app_mode/kiosk_app_data_base.cc

+// Copyright 2017 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/chromeos/app_mode/kiosk_app_data_base.h"
+
+#include <utility>
+
+#include "base/files/file_util.h"
+#include "base/task_scheduler/post_task.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "chrome/browser/browser_process.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/scoped_user_pref_update.h"
+#include "content/public/browser/browser_thread.h"
+#include "ui/gfx/codec/png_codec.h"
+
+namespace chromeos {
+
+namespace {
+
+// Keys for local state data.
+constexpr char kKeyName[] = "name";
+constexpr char kKeyIcon[] = "icon";
+
+// Icon file extension.
+const char kIconFileExtension[] = ".png";

[Luis Héctor Chávez, 2017/03/30 16:49:54]

nit: constexpr?

[Sergey Poromov, 2017/03/30 18:22:07]

Done.

+
+// Save |raw_icon| for given |app_id|.
+void SaveIconToLocalOnBlockingPool(
+    const base::FilePath& icon_path,
+    scoped_refptr<base::RefCountedString> raw_icon) {
+  DCHECK(content::BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
+
+  const base::FilePath dir = icon_path.DirName();
+  if (!base::PathExists(dir))
+    CHECK(base::CreateDirectory(dir));

[Luis Héctor Chávez, 2017/03/30 16:49:54]

don't use CHECK:
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

[Sergey Poromov, 2017/03/30 18:22:07]

Done.

+
+  CHECK_EQ(
+      static_cast<int>(raw_icon->size()),
+      base::WriteFile(icon_path, raw_icon->data().c_str(), raw_icon->size()));
+}
+
+}  // namespace
+
+// static
+const char KioskAppDataBase::kKeyApps[] = "apps";
+
+KioskAppDataBase::KioskAppDataBase(const std::string& app_id,
+                                   const AccountId& account_id)
+    : app_id_(app_id), account_id_(account_id) {}
+
+KioskAppDataBase::~KioskAppDataBase() = default;
+
+void KioskAppDataBase::SaveToDictionary(DictionaryPrefUpdate& dict_update) {

[Luis Héctor Chávez, 2017/03/30 16:49:54]

Can you add DCHECK_CURRENTLY_ON to all methods?

[Sergey Poromov, 2017/03/30 18:22:07]

Done.

+  const std::string app_key = std::string(kKeyApps) + '.' + app_id_;

[Luis Héctor Chávez, 2017/03/30 16:49:54]

It's probably better to use base::StringPrintf instead.

[Sergey Poromov, 2017/03/30 18:22:07]

I copied the code from original functions, prefer not to change it and IMHO it
doesn't look so bad here.

+  const std::string name_key = app_key + '.' + kKeyName;
+  const std::string icon_path_key = app_key + '.' + kKeyIcon;
+
+  dict_update->SetString(name_key, name_);
+  dict_update->SetString(icon_path_key, icon_path_.value());
+}
+
+bool KioskAppDataBase::LoadFromDictionary(const base::DictionaryValue* dict) {
+  const std::string app_key =
+      std::string(KioskAppDataBase::kKeyApps) + '.' + app_id_;
+  const std::string name_key = app_key + '.' + kKeyName;
+  const std::string icon_path_key = app_key + '.' + kKeyIcon;
+
+  std::string icon_path_string;
+  if (!dict->GetString(name_key, &name_) ||
+      !dict->GetString(icon_path_key, &icon_path_string)) {
+    return false;
+  }
+  icon_path_ = base::FilePath(icon_path_string);
+
+  // KioskAppIconLoader deletes itself when done.
+  (new KioskAppIconLoader(AsWeakPtr()))->Start(icon_path_);

[Luis Héctor Chávez, 2017/03/30 16:49:54]

Is this the only caller? (Seems like it)

If so, can you instead make this NOT delete itself? Instead make this class own
a std::unique_ptr<KioskAppIconLoader>. That way you don't need to have this
class support WeakPtr.

[Sergey Poromov, 2017/03/30 18:22:07]

What if KioskAppIconLoader will live longer than KioskAppDataBase ? It's
possible if the icon will be loaded slower and callback in ImageRequest will be
called after destruction of KioskAppDataBase. Anyway, it seems like at least one
WeakPtr is needed - either here, or there.

[Luis Héctor Chávez, 2017/03/30 19:57:22]

That's fine, you still _need_ the weak ptr in KioskAppIconLoader.

[Sergey Poromov, 2017/03/31 12:56:24]

Acknowledged.

+  return true;
+}
+
+void KioskAppDataBase::SaveIcon(const SkBitmap& icon,
+                                const base::FilePath& cache_dir) {
+  std::vector<unsigned char> image_data;
+  CHECK(gfx::PNGCodec::EncodeBGRASkBitmap(icon, false, &image_data));
+  scoped_refptr<base::RefCountedString> raw_icon(new base::RefCountedString);

[Luis Héctor Chávez, 2017/03/30 16:49:54]

nit: Avoid parenthesisless ctors
https://sites.google.com/a/chromium.org/dev/developers/coding-style/cpp-dos-a...
(or document that you're doing it as an optimization.

[Sergey Poromov, 2017/03/30 18:22:07]

Done.

+  raw_icon->data().assign(image_data.begin(), image_data.end());
+
+  base::FilePath icon_path =
+      cache_dir.AppendASCII(app_id_).AddExtension(kIconFileExtension);
+  content::BrowserThread::GetBlockingPool()->PostTask(
+      FROM_HERE,
+      base::Bind(&SaveIconToLocalOnBlockingPool, icon_path, raw_icon));

[Luis Héctor Chávez, 2017/03/30 16:49:54]

Why are you wrapping image_data in a scoped_refptr? Can't you just use
base::Passed(std::move(image_data)) to avoid the copy?

[Sergey Poromov, 2017/03/30 18:22:07]

Done.

+
+  icon_path_ = icon_path;
+}
+
+void KioskAppDataBase::ClearCache() {
+  PrefService* local_state = g_browser_process->local_state();
+
+  DictionaryPrefUpdate dict_update(local_state, kiosk_dictionary_name());
+
+  std::string app_key = std::string(KioskAppDataBase::kKeyApps) + '.' + app_id_;
+  dict_update->Remove(app_key, NULL);

[Luis Héctor Chávez, 2017/03/30 16:49:54]

nit: nullptr

[Sergey Poromov, 2017/03/30 18:22:07]

Done.

+
+  if (!icon_path_.empty()) {
+    base::PostTaskWithTraits(
+        FROM_HERE,
+        base::TaskTraits().MayBlock().WithPriority(
+            base::TaskPriority::BACKGROUND),
+        base::Bind(base::IgnoreResult(&base::DeleteFile), icon_path_, false));
+  }
+}
+
+}  // namespace chromeos