chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java - Issue 2453423002: Send all of the icon URLs listed in Web Manifest to WebAPK Server.

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java

Issue 2453423002: Send all of the icon URLs listed in Web Manifest to WebAPK Server. (Closed)

Patch Set: pkotwicz@'s comments. Created 4 years, 1 month ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« no previous file with comments | « no previous file | chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorFetcher.java » ('j') | chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java

index 1874d797bbd8038aae8ef85434f2ababc50207ad..bf09fa99aee13abb3dd47668c6d26a692d62187f 100644

--- a/chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java

+++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java

@@ -7,11 +7,13 @@ package org.chromium.chrome.browser.webapps;

import android.graphics.Bitmap;

import android.text.TextUtils;

-import org.chromium.base.Log;

+import org.chromium.base.CollectionUtil;

import org.chromium.chrome.browser.ShortcutHelper;

import org.chromium.chrome.browser.tab.Tab;

import org.chromium.chrome.browser.util.UrlUtilities;

+import java.util.Set;

/**

* This class checks whether the WebAPK needs to be re-installed and sends a request to re-install

* the WebAPK if it needs to be re-installed.

@@ -51,13 +53,14 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C

public String scopeUrl;

public String name;

public String shortName;

- public String iconUrl;

+ public String bestIconUrl;

// Hash of untransformed icon bytes. The hash should have been taken prior to any

// encoding/decoding.

- public String iconMurmur2Hash;

+ public String bestIconMurmur2Hash;

- public Bitmap icon;

+ public Bitmap bestIcon;

+ public Set<String> iconUrls;

public int displayMode;

public int orientation;

public long themeColor;

@@ -131,8 +134,9 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C

@Override

public void onGotManifestData(String startUrl, String scopeUrl, String name, String shortName,

- String iconUrl, String iconMurmur2Hash, Bitmap iconBitmap, int displayMode,

- int orientation, long themeColor, long backgroundColor) {

+ String bestIconUrl, String bestIconMurmur2Hash, Bitmap bestIconBitmap,

+ String[] iconUrls, int displayMode, int orientation, long themeColor,

+ long backgroundColor) {

mFetcher.destroy();

mFetcher = null;

@@ -145,9 +149,10 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C

fetchedData.scopeUrl = scopeUrl;

fetchedData.name = name;

fetchedData.shortName = shortName;

- fetchedData.iconUrl = iconUrl;

- fetchedData.iconMurmur2Hash = iconMurmur2Hash;

- fetchedData.icon = iconBitmap;

+ fetchedData.bestIconUrl = bestIconUrl;

+ fetchedData.bestIconMurmur2Hash = bestIconMurmur2Hash;

+ fetchedData.bestIcon = bestIconBitmap;

+ fetchedData.iconUrls = CollectionUtil.newHashSet(iconUrls);

fetchedData.displayMode = displayMode;

fetchedData.orientation = orientation;

fetchedData.themeColor = themeColor;

@@ -163,35 +168,22 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C

* Checks whether the WebAPK needs to be upgraded provided the fetched manifest data.

private boolean needsUpgrade(FetchedManifestData fetchedData) {

- if (!urlsMatchIgnoringFragments(mMetaData.iconUrl, fetchedData.iconUrl)

- || !mMetaData.iconMurmur2Hash.equals(fetchedData.iconMurmur2Hash)) {

- return true;

- }

- if (!urlsMatchIgnoringFragments(mMetaData.scope, fetchedData.scopeUrl)) {

- // Sometimes the scope doesn't match due to a missing "/" at the end of the scope URL.

- // Print log to find such cases.

- Log.d(TAG, "Needs to request update since the scope from WebappInfo (%s) doesn't match"

- + "the one fetched from Web Manifest(%s).", mMetaData.scope,

- fetchedData.scopeUrl);

- return true;

- }

- if (!urlsMatchIgnoringFragments(mMetaData.startUrl, fetchedData.startUrl)

+ return !mMetaData.iconUrlAndMurmur2HashMap.containsKey(fetchedData.bestIconUrl)

+ || !mMetaData.iconUrlAndMurmur2HashMap.get(fetchedData.bestIconUrl).equals(

+ fetchedData.bestIconMurmur2Hash)

pkotwicz 2016/11/14 22:43:10 Nit: I think you can simplify that top two lines t

Xi Han 2016/11/15 20:07:07 Done.

+ || !urlsMatchIgnoringFragments(mMetaData.scope, fetchedData.scopeUrl)

+ || !urlsMatchIgnoringFragments(mMetaData.startUrl, fetchedData.startUrl)

|| !TextUtils.equals(mMetaData.shortName, fetchedData.shortName)

|| !TextUtils.equals(mMetaData.name, fetchedData.name)

|| mMetaData.backgroundColor != fetchedData.backgroundColor

|| mMetaData.themeColor != fetchedData.themeColor

|| mMetaData.orientation != fetchedData.orientation

- || mMetaData.displayMode != fetchedData.displayMode) {

- return true;

- }

- return false;

+ || mMetaData.displayMode != fetchedData.displayMode

+ || !mMetaData.iconUrlAndMurmur2HashMap.keySet().equals(fetchedData.iconUrls);

pkotwicz 2016/11/14 22:43:10 Nit: Group the if statements related to iconUrlAnd

Xi Han 2016/11/15 20:07:07 Done.

}

/**

- * Returns whether the urls match ignoring fragments. Canonicalizes the URLs prior to doing the

+ * Returns whether the URLs match ignoring fragments. Canonicalizes the URLs prior to doing the

* comparison.

protected boolean urlsMatchIgnoringFragments(String url1, String url2) {