Update WebAPKs even if the WebAPK start URL has no Web Manifest

chrome/browser/android/webapk/webapk_update_data_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/webapk_update_data_fetcher.h"
 
 #include <jni.h>
 #include <vector>
 
 #include "base/android/jni_array.h"
@@ skipping 37 matching lines @@
 }
 
 WebApkUpdateDataFetcher::WebApkUpdateDataFetcher(
     JNIEnv* env,
     jobject obj,
     const GURL& scope,
     const GURL& web_manifest_url)
     : content::WebContentsObserver(nullptr),
       scope_(scope),
       web_manifest_url_(web_manifest_url),
+      is_initial_fetch_(false),
       info_(GURL()),
       weak_ptr_factory_(this) {
   java_ref_.Reset(env, obj);
 }
 
 WebApkUpdateDataFetcher::~WebApkUpdateDataFetcher() {
 }
 
 // static
 bool WebApkUpdateDataFetcher::Register(JNIEnv* env) {
@@ skipping 27 matching lines @@
   FetchInstallableData();
 }
 
 void WebApkUpdateDataFetcher::FetchInstallableData() {
   GURL url = web_contents()->GetLastCommittedURL();
 
   // DidStopLoading() can be called multiple times for a single URL. Only fetch
   // installable data the first time.
   if (url == last_fetched_url_)
     return;
+  is_initial_fetch_ = last_fetched_url_.is_empty();
   last_fetched_url_ = url;
 
-  if (!IsInScope(url, scope_))
+  if (!IsInScope(url, scope_)) {
+    OnWebManifestNotWebApkCompatible();
     return;
+  }
 
   InstallableParams params;
   params.ideal_icon_size_in_dp =
       ShortcutHelper::GetIdealHomescreenIconSizeInDp();
   params.minimum_icon_size_in_dp =
       ShortcutHelper::GetMinimumHomescreenIconSizeInDp();
   params.check_installable = true;
   params.fetch_valid_icon = true;
   InstallableManager::CreateForWebContents(web_contents());
   InstallableManager* installable_manager =
       InstallableManager::FromWebContents(web_contents());
   installable_manager->GetData(
       params,
       base::Bind(&WebApkUpdateDataFetcher::OnDidGetInstallableData,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void WebApkUpdateDataFetcher::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 (data.manifest.IsEmpty() || web_manifest_url_ != data.manifest_url)
-    return;
-
-  // TODO(pkotwicz): Tell Java side that the Web Manifest was fetched but the
-  // Web Manifest is not WebAPK-compatible. (http://crbug.com/639536)
-  if (data.error_code != NO_ERROR_DETECTED ||
-      !AreWebManifestUrlsWebApkCompatible(data.manifest)) {
+  if (data.manifest.IsEmpty() || web_manifest_url_ != data.manifest_url) {
+    OnWebManifestNotWebApkCompatible();
     return;
   }
 
+  if (data.error_code != NO_ERROR_DETECTED ||

[Xi Han, 2016/12/20 20:53:23]

I believe Peter splited them as two checks because the comment above doesn't
apply to the second check.

[dominickn, 2016/12/21 02:14:32]

The comment can be very easily changed to incorporate this as well. It's
preferable to having repeated code.

"Determine whether or not the manifest is WebAPK-compatible. There are 3 cases:
1. the site isn't installable.
2. the URLs in the manifest expose passwords
3. there is no manifest or the manifest is different to the one we're expecting
(in which case, we continue observing this WebContents as we may find the
manifest on a different page)"

[Xi Han, 2016/12/21 22:25:11]

All right, merge them together. I re-order these checks to put the easiest
first.

+      !AreWebManifestUrlsWebApkCompatible(data.manifest)) {
+    OnWebManifestNotWebApkCompatible();
+    return;
+  }
+
   info_.UpdateFromManifest(data.manifest);
   info_.manifest_url = data.manifest_url;
   info_.best_icon_url = data.icon_url;
   best_icon_ = *data.icon;
 
   icon_hasher_.reset(new WebApkIconHasher());
   Profile* profile =
       Profile::FromBrowserContext(web_contents()->GetBrowserContext());
   icon_hasher_->DownloadAndComputeMurmur2Hash(
       profile->GetRequestContext(),
       data.icon_url,
       base::Bind(&WebApkUpdateDataFetcher::OnGotIconMurmur2Hash,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void WebApkUpdateDataFetcher::OnGotIconMurmur2Hash(
     const std::string& best_icon_murmur2_hash) {
   icon_hasher_.reset();
 
   if (best_icon_murmur2_hash.empty()) {
-    // TODO(pkotwicz): Tell Java side that the Web Manifest was fetched but the
-    // Web Manifest is not WebAPK-compatible. (http://crbug.com/639536)
+    OnWebManifestNotWebApkCompatible();
     return;
   }
 
   OnDataAvailable(info_, best_icon_murmur2_hash, best_icon_);
 }
 
 void WebApkUpdateDataFetcher::OnDataAvailable(
     const ShortcutInfo& info,
     const std::string& best_icon_murmur2_hash,
     const SkBitmap& best_icon_bitmap) {
@@ skipping 16 matching lines @@
 
   ScopedJavaLocalRef<jobjectArray> java_icon_urls =
       base::android::ToJavaArrayOfStrings(env, info.icon_urls);
 
   Java_WebApkUpdateDataFetcher_onDataAvailable(
       env, java_ref_, java_url, java_scope, java_name, java_short_name,
       java_best_icon_url, java_best_icon_murmur2_hash, java_best_bitmap,
       java_icon_urls, info.display, info.orientation, info.theme_color,
       info.background_color);
 }
+
+void WebApkUpdateDataFetcher::OnWebManifestNotWebApkCompatible() {
+  if (!is_initial_fetch_)
+    return;
+
+  Java_WebApkUpdateDataFetcher_onWebManifestForInitialUrlNotWebApkCompatible(
+      base::android::AttachCurrentThread(), java_ref_);
+}