Update WebAPK if homescreen icon changed.

chrome/browser/android/webapk/manifest_upgrade_detector_fetcher.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/android/webapk/manifest_upgrade_detector_fetcher.h"
 
 #include <jni.h>

[dominickn, 2016/08/16 00:12:45]

Nit: #include <vector>

[pkotwicz, 2016/08/16 19:08:15]

Done.

 
 #include "base/android/jni_string.h"
+#include "chrome/browser/android/shortcut_helper.h"

[dominickn, 2016/08/16 00:12:45]

Nit: I couldn't see where ShortcutHelper is being used in this file now - remove
the include?

[pkotwicz, 2016/08/16 19:08:15]

We use ShortcutHelper to get the homescreen icon size in lines 108 and 110

 #include "chrome/browser/android/shortcut_info.h"
+#include "chrome/browser/installable/installable_manager.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/manifest.h"
 #include "jni/ManifestUpgradeDetectorFetcher_jni.h"
+#include "third_party/smhasher/src/MurmurHash2.h"
+#include "ui/gfx/android/java_bitmap.h"
+#include "ui/gfx/codec/png_codec.h"
 #include "url/gurl.h"
 
 using base::android::JavaParamRef;
 using base::android::ScopedJavaLocalRef;
 
 namespace {
 
+// The seed to use for the murmur2 hash.
+const uint64_t kMurmur2HashSeed = 0;
+
+// Computes a murmur2 hash of |bitmap|'s PNG encoded bytes.
+uint64_t ComputeBitmapHash(const SkBitmap& bitmap) {
+  std::vector<unsigned char> png_bytes;
+  gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, false, &png_bytes);
+  return MurmurHash64B(&png_bytes.front(), png_bytes.size(), kMurmur2HashSeed);
+}
+
 // Returns whether the given |url| is within the scope of the |scope| url.
 bool IsInScope(const GURL& url, const GURL& scope) {
   return base::StartsWith(url.spec(), scope.spec(),
                          base::CompareCase::SENSITIVE);
 }
 
 }  // anonymous namespace
 
 jlong Initialize(JNIEnv* env,
                  const JavaParamRef<jobject>& obj,
@@ skipping 49 matching lines @@
 }
 
 void ManifestUpgradeDetectorFetcher::DidFinishLoad(
     content::RenderFrameHost* render_frame_host,
     const GURL& validated_url) {
   if (render_frame_host->GetParent())
     return;
   if (!IsInScope(validated_url, scope_))
     return;
 
-  web_contents()->GetManifest(
-      base::Bind(&ManifestUpgradeDetectorFetcher::OnDidGetManifest,
+  InstallableParams params;

[dominickn, 2016/08/16 00:12:45]

Optional nit: put the construction of params in a function in the anonymous
namespace.

+  params.ideal_icon_size_in_dp =

[dominickn, 2016/08/16 00:12:45]

Optional nit: make ideal_icon_size_in_dp and minimum_icon_size_in_dp members of
this class, and pass them in from Java via
Initialize(ShortcutHelper.getIdealHomescreenIconSizeInDp(),
ShortcutHelper.getMinimumHomescreenIconSizeInDp()).

+      ShortcutHelper::GetIdealHomescreenIconSizeInDp();
+  params.minimum_icon_size_in_dp =
+      ShortcutHelper::GetMinimumHomescreenIconSizeInDp();
+  params.fetch_valid_icon = true;
+  InstallableManager::CreateForWebContents(web_contents());
+  InstallableManager* installable_manager =
+      InstallableManager::FromWebContents(web_contents());
+  installable_manager->GetData(
+      params,
+      base::Bind(&ManifestUpgradeDetectorFetcher::OnDidGetInstallableData,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
-void ManifestUpgradeDetectorFetcher::OnDidGetManifest(
-    const GURL& manifest_url,
-    const content::Manifest& manifest) {
+void ManifestUpgradeDetectorFetcher::OnDidGetInstallableData(
+    const InstallableData& data) {
   // If the manifest is empty, it means the current WebContents doesn't
   // associate with a Web Manifest. In such case, we ignore the empty manifest
   // and continue observing the WebContents's loading until we find a page that
   // links to the Web Manifest that we are looking for.
   // If the manifest URL is different from the current one, we will continue
   // observing too. It is based on our assumption that it is invalid for
   // web developers to change the Web Manifest location. When it does
   // change, we will treat the new Web Manifest as the one of another WebAPK.
-  if (manifest.IsEmpty() || web_manifest_url_ != manifest_url)
+  if (data.manifest.IsEmpty() || web_manifest_url_ != data.manifest_url)
     return;
 
   ShortcutInfo info(GURL::EmptyGURL());
-  info.UpdateFromManifest(manifest);
-  info.manifest_url = manifest_url;
-
-  OnDataAvailable(info);
+  info.UpdateFromManifest(data.manifest);
+  info.manifest_url = data.manifest_url;
+  OnDataAvailable(info, (data.icon ? *data.icon : SkBitmap()));
 }
 
-void ManifestUpgradeDetectorFetcher::OnDataAvailable(const ShortcutInfo& info) {
+void ManifestUpgradeDetectorFetcher::OnDataAvailable(
+    const ShortcutInfo& info,
+    const SkBitmap& icon_bitmap) {
   JNIEnv* env = base::android::AttachCurrentThread();
 
   ScopedJavaLocalRef<jstring> java_url =
       base::android::ConvertUTF8ToJavaString(env, info.url.spec());
   ScopedJavaLocalRef<jstring> java_scope =
       base::android::ConvertUTF8ToJavaString(env, info.scope.spec());
   ScopedJavaLocalRef<jstring> java_name =
       base::android::ConvertUTF16ToJavaString(env, info.name);
   ScopedJavaLocalRef<jstring> java_short_name =
       base::android::ConvertUTF16ToJavaString(env, info.short_name);
+  ScopedJavaLocalRef<jstring> java_icon_url =
+      base::android::ConvertUTF8ToJavaString(env, info.icon_url.spec());
+  ScopedJavaLocalRef<jobject> java_bitmap;
+  uint64_t icon_murmur2_hash = 0;
+  if (icon_bitmap.getSize()) {

[dominickn, 2016/08/16 00:12:44]

Perhaps call !icon_bitmap.drawsNothing() ? That is consistent with app banners
and should subsume the getSize check.

[pkotwicz, 2016/08/16 19:08:15]

Yes, I need to do this. I prefer to do this in a separate CL though. I have
filed http://crbug.com/636934 with respect to this.

+    java_bitmap = gfx::ConvertToJavaBitmap(&icon_bitmap);
+    // TODO(pkotwicz): Get hash of untransformed icon's bytes (with no
+    // encoding/decoding).
+    icon_murmur2_hash = ComputeBitmapHash(icon_bitmap);
+  }
 
   Java_ManifestUpgradeDetectorFetcher_onDataAvailable(
       env, java_ref_.obj(),
       java_url.obj(),
       java_scope.obj(),
       java_name.obj(),
       java_short_name.obj(),
+      java_icon_url.obj(),
+      icon_murmur2_hash,
+      java_bitmap.obj(),
       info.display,
       info.orientation,
       info.theme_color,
       info.background_color);
 }