Introduce ManifestUpgradeDetector for WebAPK to detect web manifest changes.

chrome/browser/android/webapps/manifest_upgrade_detector.cc

Index: chrome/browser/android/webapps/manifest_upgrade_detector.cc
diff --git a/chrome/browser/android/webapps/manifest_upgrade_detector.cc b/chrome/browser/android/webapps/manifest_upgrade_detector.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9c109bbe8d8bfded07d67940fc1c493d0be5dccb
--- /dev/null
+++ b/chrome/browser/android/webapps/manifest_upgrade_detector.cc
@@ -0,0 +1,137 @@
+// 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/webapps/manifest_upgrade_detector.h"
+
+#include <jni.h>
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_array.h"
+#include "base/android/jni_string.h"
+#include "chrome/browser/android/shortcut_info.h"
+#include "chrome/browser/banners/app_banner_manager.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/common/manifest.h"
+#include "jni/ManifestUpgradeDetector_jni.h"
+#include "url/gurl.h"
+
+using content::Manifest;
+
+jlong Initialize(JNIEnv* env,
+                 const JavaParamRef<jobject>& obj,
+                 const JavaParamRef<jobject>& java_web_contents,
+                 const JavaParamRef<jstring>& java_web_manifest_url) {
+  content::WebContents* web_contents =
+      content::WebContents::FromJavaWebContents(java_web_contents);
+  GURL web_manifest_url(base::android::ConvertJavaStringToUTF16(
+      env, java_web_manifest_url));
+  ManifestUpgradeDetector* manifest_upgrade_detector =
+      new ManifestUpgradeDetector(env, obj, web_contents, web_manifest_url);
+  return reinterpret_cast<intptr_t>(manifest_upgrade_detector);
+}
+
+ManifestUpgradeDetector::ManifestUpgradeDetector(
+    JNIEnv* env,
+    jobject obj,
+    content::WebContents* web_contents,
+    const GURL& web_manifest_url)
+    : content::WebContentsObserver(web_contents),
+      start_(false),
+      web_manifest_url_(web_manifest_url),
+      weak_ptr_factory_(this) {
+  java_ref_.Reset(env, obj);
+}
+
+ManifestUpgradeDetector::~ManifestUpgradeDetector() {
+}
+
+// static
+bool ManifestUpgradeDetector::Register(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+void ManifestUpgradeDetector::ReplaceWebContents(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& obj,
+    const JavaParamRef<jobject>& jweb_contents) {
+  content::WebContents* web_contents =
+      content::WebContents::FromJavaWebContents(jweb_contents);
+  content::WebContentsObserver::Observe(web_contents);
+}
+
+void ManifestUpgradeDetector::DidFinishLoad(
+    content::RenderFrameHost* render_frame_host,
+    const GURL& validated_url) {
+  if (render_frame_host->GetParent())
+    return;
+  if (start_) {
+    content::WebContents* web_contents =
+        content::WebContents::FromRenderFrameHost(render_frame_host);

[pkotwicz, 2016/07/13 01:08:20]

Nit: You can get the WebContents via WebContentsObserver::web_contents()

[Xi Han, 2016/07/13 19:25:14]

Thanks!

+    web_contents->GetManifest(
+        base::Bind(&ManifestUpgradeDetector::OnDidGetManifest,
+                  weak_ptr_factory_.GetWeakPtr()));
+  }
+}

[pkotwicz, 2016/07/13 01:08:20]

You should override WebContentsObserver::DidNavigateMainFrame() and fail if the
user navigates outside of the WebAPK's scope (It is possible to navigate outside
of the WebAPK scope without bouncing out of the WebAPK)

I do not think that it is an issue; is there any weirdness which occurs if the
WebContents::GetManifest() request is sent prior to a navigation and the
response is received after a navigation?

[Xi Han, 2016/07/13 19:25:14]

Add the scope to check whether the url loaded is within the WebAPK's scope. As
discussed offline, we still use the DidFinishLoad() for now.

+
+void ManifestUpgradeDetector::OnDidGetManifest(
+    const GURL& manifest_url,
+    const content::Manifest& manifest) {
+  if (manifest.IsEmpty()
+      ||!banners::AppBannerManager::URLsAreForTheSamePage(

[pkotwicz, 2016/07/13 01:08:20]

Check:
web_manifest_url_ != manifest_url
instead

banners::AppBannerManager::URLsAreForTheSamePage() checks whether the URLs match
except for their fragments. Fragment navigations do not make sense in the
context of Web App Manifests

[Xi Han, 2016/07/13 19:25:14]

It makes sense, updated.

+          web_manifest_url_, manifest_url)) {
+    return;
+  }
+
+  start_ = false;
+
+  ShortcutInfo info(GURL::EmptyGURL());
+  info.UpdateFromManifest(manifest);
+  info.manifest_url = manifest_url;
+

[pkotwicz, 2016/07/13 01:08:20]

Ignore icon URLs for now too :)

[Xi Han, 2016/07/13 19:25:14]

We could have the icon_urls, since they are obtained in the manifest directly,
without any additional fetches.

[pkotwicz, 2016/07/13 21:40:38]

I am suggesting skipping the icons for now because I am not yet convinced that
we will make use of multiple icon URLs in ManifestUpgradeDetector

We need to see how confusing things are once we introduce the bitmap checking
code

[Xi Han, 2016/07/15 19:30:01]

No matter how we treat them later, these urls are something that we can get
directly from Web Manifest. Their changes most likely mean the images are
updated. I still prefer to keep them for now. 

+  std::vector<std::string> icon_urls;
+  for (const auto& icon : manifest.icons)
+    icon_urls.push_back(icon.src.spec());
+
+  OnDataAvailable(info, icon_urls);
+}
+
+void ManifestUpgradeDetector::OnDataAvailable(
+    const ShortcutInfo& info,
+    const std::vector<std::string>& icons) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+
+  ScopedJavaLocalRef<jstring> java_url =
+      base::android::ConvertUTF8ToJavaString(env, info.url.spec());
+  ScopedJavaLocalRef<jstring> java_user_title =
+      base::android::ConvertUTF16ToJavaString(env, info.user_title);

[pkotwicz, 2016/07/13 01:08:20]

|java_user_title| is not used?

[Xi Han, 2016/07/13 19:25:14]

Removed.

+  ScopedJavaLocalRef<jstring> java_name =
+      base::android::ConvertUTF16ToJavaString(env, info.name);
+  ScopedJavaLocalRef<jstring> java_short_name =
+      base::android::ConvertUTF16ToJavaString(env, info.short_name);
+  ScopedJavaLocalRef<jobjectArray> java_icons =
+            base::android::ToJavaArrayOfStrings(env, icons);
+
+  Java_ManifestUpgradeDetector_onDataAvailable(
+    env, java_ref_.obj(),
+    java_url.obj(),
+    java_name.obj(),
+    java_short_name.obj(),
+    java_icons.obj(),
+    info.display,
+    info.orientation,
+    info.theme_color,
+    info.background_color);
+}
+
+void ManifestUpgradeDetector::Destroy(JNIEnv* env,
+                                      const JavaParamRef<jobject>& obj) {
+  delete this;
+}
+
+void ManifestUpgradeDetector::Start(JNIEnv* env,
+                                      const JavaParamRef<jobject>& obj) {
+  start_ = true;
+}