Make ManifestUpgradeDetector correctly fetch "icon Murmur2 hash" if hash is big

Make ManifestUpgradeDetector correctly fetch "icon Murmur2 hash" if hash is big

Murmur2 hashes can take on values up to 2^64 - 1. 2^64 - 1 is larger than
Long#MAX_VALUE. This CL introduces
WebApkMetaDataUtils#getIconMurmur2HashFromMetaData() to extract the hash from
the meta data in the WebAPK's Android Manifest.

BUG=659348
TEST=WebApkMetaDataUtilsTest#testGetIconMurmur2HashFromMetaData,
     ManifestUpgradeDetectorFetcherTest.testIconMurmur2HashParsing

Committed: https://crrev.com/aaab0997ef06ae47b24670b0510a5e67ce5ff7ec
Cr-Commit-Position: refs/heads/master@{#428140}

[pkotwicz, 2016-10-25 23:47:26]

Patchset #3 (id:40001) has been deleted

[pkotwicz, 2016-10-25 23:47:33]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2016-10-25 23:48:15]

Description was changed from

==========
Make ManifestUpgradeDetector correctly fetch "icon Murmur2 hash" if hash is big

Murmur2 hashes can take on values up to 2^64 - 1. 2^64 - 1 is larger than
Long#MAX_VALUE.
This CL introduces WebApkMetaDataUtils#getIconMurmur2HashFromMetaData() to
extract the
hash from the <meta-data> in the WebAPK's Android Manifest.

BUG=659348
TEST=WebApkMetaDataUtilsTest#testGetIconMurmur2HashFromMetaData,
     ManifestUpgradeDetectorFetcherTest.testIconMurmur2HashParsing
==========

to

==========
Make ManifestUpgradeDetector correctly fetch "icon Murmur2 hash" if hash is big

Murmur2 hashes can take on values up to 2^64 - 1. 2^64 - 1 is larger than
Long#MAX_VALUE. This CL introduces
WebApkMetaDataUtils#getIconMurmur2HashFromMetaData() to extract the hash from
the meta data in the WebAPK's Android Manifest.

BUG=659348
TEST=WebApkMetaDataUtilsTest#testGetIconMurmur2HashFromMetaData,
     ManifestUpgradeDetectorFetcherTest.testIconMurmur2HashParsing
==========

[pkotwicz, 2016-10-25 23:49:25]

Patchset #1 (id:20001) has been deleted

[pkotwicz, 2016-10-25 23:55:08]

Patchset #1 (id:60001) has been deleted

[pkotwicz, 2016-10-25 23:55:30]

pkotwicz@chromium.org changed reviewers:
+ dominickn@chromium.org

[pkotwicz, 2016-10-25 23:55:31]

Dominick, can you please take a look?

[dominickn, 2016-10-26 03:07:40]

Please CQ dry-run this once you've addressed the comments.

https://codereview.chromium.org/2452823002/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorFetcherTest.java
(right):

https://codereview.chromium.org/2452823002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorFetcherTest.java:149:
final String manifestUrl =
"/chrome/test/data/banners/manifest_long_icon_murmur2_hash.json";
Should these be constants like the other links in this file?

https://codereview.chromium.org/2452823002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorFetcherTest.java:150:
final String scopeUrl = "/chrome/test/data/banners";
Why is this scope different to the WEB_MANIFEST_SCOPE above?

[pkotwicz, 2016-10-26 04:04:08]

Dominick, can you please take a look?

https://codereview.chromium.org/2452823002/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorFetcherTest.java
(right):

https://codereview.chromium.org/2452823002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorFetcherTest.java:149:
final String manifestUrl =
"/chrome/test/data/banners/manifest_long_icon_murmur2_hash.json";
The constants are used in this only. I don't think that they need to be global
to this file.

https://codereview.chromium.org/2452823002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorFetcherTest.java:150:
final String scopeUrl = "/chrome/test/data/banners";
I changed this to have the same value as WEB_MANIFEST_SCOPE

[pkotwicz, 2016-10-26 04:04:19]

The CQ bit was checked by pkotwicz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-26 04:04:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 04:12:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-26 04:12:48]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dominickn, 2016-10-26 06:03:55]

https://codereview.chromium.org/2452823002/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorFetcherTest.java
(right):

https://codereview.chromium.org/2452823002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorFetcherTest.java:150:
final String scopeUrl = "/chrome/test/data/banners";
On 2016/10/26 04:04:07, pkotwicz wrote:
> I changed this to have the same value as WEB_MANIFEST_SCOPE

If it has the same value, couldn't you just use WEB_MANIFEST_SCOPE?

Or at the very least, can't you have String scopeUrl = WEB_MANIFEST_SCOPE?

[pkotwicz, 2016-10-26 18:05:05]

Dominick, can you please take another look?

https://codereview.chromium.org/2452823002/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorFetcherTest.java
(right):

https://codereview.chromium.org/2452823002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorFetcherTest.java:150:
final String scopeUrl = "/chrome/test/data/banners";
I switched to declaring the constants at the top of the file

[dominickn, 2016-10-27 00:39:52]

lgtm, thanks

[pkotwicz, 2016-10-27 15:20:30]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2016-10-27 15:20:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 15:28:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-27 15:28:05]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[pkotwicz, 2016-10-27 17:32:21]

pkotwicz@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[pkotwicz, 2016-10-27 17:32:22]

dfalcantara@ for change to WebApkMetaDataUtils

[gone, 2016-10-27 17:35:45]

That file lgtm

[pkotwicz, 2016-10-27 21:13:37]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2016-10-27 21:14:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 21:22:32]

Description was changed from

==========
Make ManifestUpgradeDetector correctly fetch "icon Murmur2 hash" if hash is big

Murmur2 hashes can take on values up to 2^64 - 1. 2^64 - 1 is larger than
Long#MAX_VALUE. This CL introduces
WebApkMetaDataUtils#getIconMurmur2HashFromMetaData() to extract the hash from
the meta data in the WebAPK's Android Manifest.

BUG=659348
TEST=WebApkMetaDataUtilsTest#testGetIconMurmur2HashFromMetaData,
     ManifestUpgradeDetectorFetcherTest.testIconMurmur2HashParsing
==========

to

==========
Make ManifestUpgradeDetector correctly fetch "icon Murmur2 hash" if hash is big

Murmur2 hashes can take on values up to 2^64 - 1. 2^64 - 1 is larger than
Long#MAX_VALUE. This CL introduces
WebApkMetaDataUtils#getIconMurmur2HashFromMetaData() to extract the hash from
the meta data in the WebAPK's Android Manifest.

BUG=659348
TEST=WebApkMetaDataUtilsTest#testGetIconMurmur2HashFromMetaData,
     ManifestUpgradeDetectorFetcherTest.testIconMurmur2HashParsing
==========

[commit-bot: I haz the power, 2016-10-27 21:22:34]

Committed patchset #4 (id:140001)

[commit-bot: I haz the power, 2016-10-27 21:26:06]

Description was changed from

==========
Make ManifestUpgradeDetector correctly fetch "icon Murmur2 hash" if hash is big

Murmur2 hashes can take on values up to 2^64 - 1. 2^64 - 1 is larger than
Long#MAX_VALUE. This CL introduces
WebApkMetaDataUtils#getIconMurmur2HashFromMetaData() to extract the hash from
the meta data in the WebAPK's Android Manifest.

BUG=659348
TEST=WebApkMetaDataUtilsTest#testGetIconMurmur2HashFromMetaData,
     ManifestUpgradeDetectorFetcherTest.testIconMurmur2HashParsing
==========

to

==========
Make ManifestUpgradeDetector correctly fetch "icon Murmur2 hash" if hash is big

Murmur2 hashes can take on values up to 2^64 - 1. 2^64 - 1 is larger than
Long#MAX_VALUE. This CL introduces
WebApkMetaDataUtils#getIconMurmur2HashFromMetaData() to extract the hash from
the meta data in the WebAPK's Android Manifest.

BUG=659348
TEST=WebApkMetaDataUtilsTest#testGetIconMurmur2HashFromMetaData,
     ManifestUpgradeDetectorFetcherTest.testIconMurmur2HashParsing

Committed: https://crrev.com/aaab0997ef06ae47b24670b0510a5e67ce5ff7ec
Cr-Commit-Position: refs/heads/master@{#428140}
==========

[commit-bot: I haz the power, 2016-10-27 21:26:08]

Patchset 4 (id:??) landed as
https://crrev.com/aaab0997ef06ae47b24670b0510a5e67ce5ff7ec
Cr-Commit-Position: refs/heads/master@{#428140}