Canonicalize URLs prior to comparing in ManifestUpgradeDetector

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

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 a0ffd9e2078f833088572265ee2d76ce3b46dd2c..6de64ece922210208cd8ee6a241cf4407772195f 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
@@ -12,6 +12,7 @@ import org.chromium.base.Log;
 import org.chromium.chrome.browser.ShortcutHelper;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.util.IntentUtils;
+import org.chromium.chrome.browser.util.UrlUtilities;
 import org.chromium.webapk.lib.common.WebApkMetaDataKeys;
 
 /**
@@ -23,7 +24,7 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
      * Called when the process of checking Web Manifest update is complete.
      */
     public interface Callback {
-        public void onUpgradeNeededCheckFinished(boolean isUpgraded, FetchedManifestData data);
+        public void onUpgradeNeededCheckFinished(boolean needsUpgrade, FetchedManifestData data);
     }
 
     private static final String TAG = "cr_UpgradeDetector";
@@ -178,20 +179,21 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
 
         // TODO(hanxi): crbug.com/627824. Validate whether the new fetched data is
         // WebAPK-compatible.
-        boolean upgradeRequired = requireUpgrade(fetchedData);
-        mCallback.onUpgradeNeededCheckFinished(upgradeRequired, fetchedData);
+        boolean needsUpgrade = needsUpgrade(fetchedData);

[dominickn, 2016/09/20 00:25:01]

Don't have variables and methods with the same name - it's needlessly confusing.
Change either the name of the method or the name of the variable.

+        mCallback.onUpgradeNeededCheckFinished(needsUpgrade, fetchedData);
     }
 
     /**
      * Checks whether the WebAPK needs to be upgraded provided the fetched manifest data.
      */
-    private boolean requireUpgrade(FetchedManifestData fetchedData) {
-        if (!TextUtils.equals(mIconUrl, fetchedData.iconUrl)
+    private boolean needsUpgrade(FetchedManifestData fetchedData) {
+        if (!urlsMatchIgnoringFragments(mIconUrl, fetchedData.iconUrl)
                 || !mIconMurmur2Hash.equals(fetchedData.iconMurmur2Hash)) {
             return true;
         }
 
-        boolean scopeMatch = mWebappInfo.scopeUri().toString().equals(fetchedData.scopeUrl);
+        boolean scopeMatch = urlsMatchIgnoringFragments(

[dominickn, 2016/09/20 00:25:02]

Nit: inline this calculation into the if condition so it's consistent with the
check above.

+                mWebappInfo.scopeUri().toString(), fetchedData.scopeUrl);
         if (!scopeMatch) {
             // Sometimes the scope doesn't match due to a missing "/" at the end of the scope URL.
             // Print log to find such cases.
@@ -201,7 +203,7 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
             return true;
         }
 
-        if (!TextUtils.equals(mStartUrl, fetchedData.startUrl)
+        if (!urlsMatchIgnoringFragments(mStartUrl, fetchedData.startUrl)
                 || !TextUtils.equals(mWebappInfo.shortName(), fetchedData.shortName)
                 || !TextUtils.equals(mWebappInfo.name(), fetchedData.name)
                 || mWebappInfo.backgroundColor() != fetchedData.backgroundColor
@@ -213,4 +215,12 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
 
         return false;
     }
+
+    /**
+     * Returns whether the urls match ignoring fragments. Canonicalizes the URLs prior to doing the
+     * comparison.
+     */
+    protected boolean urlsMatchIgnoringFragments(String url1, String url2) {
+        return UrlUtilities.urlsMatchIgnoringFragments(url1, url2);
+    }
 }