Upstream: Add additional checks before creating a WebAPK after clicking "Add to Homescreen"

Upstream: Add additional checks before creating a WebAPK after clicking "Add to Homescreen"

This CL makes "Add to Homescreen" from the app banner and from the app menu
create a WebAPK if the following conditions are met:
- Chrome is run with the --enable-webapk command line flag
- active tab is controlled by a service worker
- web manifest has the following non-empty fields:
  - name
  - short name
  - start_url
- display field in web manifest is either "fullscreen" or "standalone"

Additionally, this CL:
- Moves AppBannerDataFetcher::IsManifestValidForWebApp() to
  webapp_manifest_validator.cc because the function is used by both
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc

- Moves the computation of whether a web page is "web app capable" from
  ShortcutHelper.java to app_banner_data_fetcher.cc and
  add_to_homescreen_data_fetcher.cc - the same place that the computation of
  whether a web page is "WebAPK capable" is made.

BUG=609122

[pkotwicz, 2016-06-09 00:08:15]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2016-06-09 00:56:05]

Patchset #2 (id:40001) has been deleted

[pkotwicz, 2016-06-09 01:09:26]

Patchset #1 (id:20001) has been deleted

[pkotwicz, 2016-06-09 01:09:41]

Patchset #1 (id:60001) has been deleted

[pkotwicz, 2016-06-09 01:24:30]

Description was changed from

==========
ABCD
==========

to

==========
Add additional checks before creating a WebAPK after clicking "Add to
Homescreen"

This CL makes "Add to Homescreen" from the app banner and from the app menu
create a WebAPK if the following conditions are met:
- Chrome is run with the --enable-webapk command line flag
- active tab is controlled by a service worker
- web manifest has the following non-empty fields:
  - name
  - short name
  - start_url
- display field in web manifest is either "fullscreen" or "standalone"

Additionally, this CL:
- Moves AppBannerDataFetcher::IsManifestValidForWebApp() to
  webapp_manifest_validator.cc because the function is used by both
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc
- Moves the computation of whether a web page is "web app capable" to
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc - the same
  place that the computation of whether a web page is "WebAPK capable" is made.
==========

[pkotwicz, 2016-06-09 01:24:30]

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

[pkotwicz, 2016-06-09 01:26:00]

Description was changed from

==========
Add additional checks before creating a WebAPK after clicking "Add to
Homescreen"

This CL makes "Add to Homescreen" from the app banner and from the app menu
create a WebAPK if the following conditions are met:
- Chrome is run with the --enable-webapk command line flag
- active tab is controlled by a service worker
- web manifest has the following non-empty fields:
  - name
  - short name
  - start_url
- display field in web manifest is either "fullscreen" or "standalone"

Additionally, this CL:
- Moves AppBannerDataFetcher::IsManifestValidForWebApp() to
  webapp_manifest_validator.cc because the function is used by both
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc
- Moves the computation of whether a web page is "web app capable" to
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc - the same
  place that the computation of whether a web page is "WebAPK capable" is made.
==========

to

==========
Add additional checks before creating a WebAPK after clicking "Add to
Homescreen"

This CL makes "Add to Homescreen" from the app banner and from the app menu
create a WebAPK if the following conditions are met:
- Chrome is run with the --enable-webapk command line flag
- active tab is controlled by a service worker
- web manifest has the following non-empty fields:
  - name
  - short name
  - start_url
- display field in web manifest is either "fullscreen" or "standalone"

Additionally, this CL:
- Moves AppBannerDataFetcher::IsManifestValidForWebApp() to
  webapp_manifest_validator.cc because the function is used by both
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc
- Moves the computation of whether a web page is "web app capable" from
  ShortcutHelper.java to app_banner_data_fetcher.cc and
  add_to_homescreen_data_fetcher.cc - the same place that the computation of
  whether a web page is "WebAPK capable" is made.
==========

[pkotwicz, 2016-06-09 01:26:23]

Description was changed from

==========
Add additional checks before creating a WebAPK after clicking "Add to
Homescreen"

This CL makes "Add to Homescreen" from the app banner and from the app menu
create a WebAPK if the following conditions are met:
- Chrome is run with the --enable-webapk command line flag
- active tab is controlled by a service worker
- web manifest has the following non-empty fields:
  - name
  - short name
  - start_url
- display field in web manifest is either "fullscreen" or "standalone"

Additionally, this CL:
- Moves AppBannerDataFetcher::IsManifestValidForWebApp() to
  webapp_manifest_validator.cc because the function is used by both
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc
- Moves the computation of whether a web page is "web app capable" from
  ShortcutHelper.java to app_banner_data_fetcher.cc and
  add_to_homescreen_data_fetcher.cc - the same place that the computation of
  whether a web page is "WebAPK capable" is made.
==========

to

==========
Add additional checks before creating a WebAPK after clicking "Add to
Homescreen"

This CL makes "Add to Homescreen" from the app banner and from the app menu
create a WebAPK if the following conditions are met:
- Chrome is run with the --enable-webapk command line flag
- active tab is controlled by a service worker
- web manifest has the following non-empty fields:
  - name
  - short name
  - start_url
- display field in web manifest is either "fullscreen" or "standalone"

Additionally, this CL:
- Moves AppBannerDataFetcher::IsManifestValidForWebApp() to
  webapp_manifest_validator.cc because the function is used by both
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc

- Moves the computation of whether a web page is "web app capable" from
  ShortcutHelper.java to app_banner_data_fetcher.cc and
  add_to_homescreen_data_fetcher.cc - the same place that the computation of
  whether a web page is "WebAPK capable" is made.
==========

[pkotwicz, 2016-06-09 01:33:22]

Description was changed from

==========
Add additional checks before creating a WebAPK after clicking "Add to
Homescreen"

This CL makes "Add to Homescreen" from the app banner and from the app menu
create a WebAPK if the following conditions are met:
- Chrome is run with the --enable-webapk command line flag
- active tab is controlled by a service worker
- web manifest has the following non-empty fields:
  - name
  - short name
  - start_url
- display field in web manifest is either "fullscreen" or "standalone"

Additionally, this CL:
- Moves AppBannerDataFetcher::IsManifestValidForWebApp() to
  webapp_manifest_validator.cc because the function is used by both
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc

- Moves the computation of whether a web page is "web app capable" from
  ShortcutHelper.java to app_banner_data_fetcher.cc and
  add_to_homescreen_data_fetcher.cc - the same place that the computation of
  whether a web page is "WebAPK capable" is made.
==========

to

==========
Upstream: Add additional checks before creating a WebAPK after clicking "Add to
Homescreen"

This CL makes "Add to Homescreen" from the app banner and from the app menu
create a WebAPK if the following conditions are met:
- Chrome is run with the --enable-webapk command line flag
- active tab is controlled by a service worker
- web manifest has the following non-empty fields:
  - name
  - short name
  - start_url
- display field in web manifest is either "fullscreen" or "standalone"

Additionally, this CL:
- Moves AppBannerDataFetcher::IsManifestValidForWebApp() to
  webapp_manifest_validator.cc because the function is used by both
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc

- Moves the computation of whether a web page is "web app capable" from
  ShortcutHelper.java to app_banner_data_fetcher.cc and
  add_to_homescreen_data_fetcher.cc - the same place that the computation of
  whether a web page is "WebAPK capable" is made.
==========

[pkotwicz, 2016-06-09 01:39:39]

dominickn@ can you please take a look?

This CL upstreams code in the WebAPK experimental branch. It is an OKR to
upstream everything. The CL depends on
https://codereview.chromium.org/2046423003/

I am very open to restructuring this CL to make it fit better with your future
plans for unifying add_to_homescreen_data_fetcher.cc and
app_banner_data_fetcher.cc

https://codereview.chromium.org/2050933002/diff/80001/chrome/browser/banners/...
File chrome/browser/banners/webapp_manifest_validator.h (right):

https://codereview.chromium.org/2050933002/diff/80001/chrome/browser/banners/...
chrome/browser/banners/webapp_manifest_validator.h:24: // If not, sets |code| as
to why.
I am not sold on this function name. Do you have any suggestions?

[pkotwicz, 2016-06-09 01:39:42]

dominickn@ can you please take a look?

This CL upstreams code in the WebAPK experimental branch. It is an OKR to
upstream everything. The CL depends on
https://codereview.chromium.org/2046423003/

I am very open to restructuring this CL to make it fit better with your future
plans for unifying add_to_homescreen_data_fetcher.cc and
app_banner_data_fetcher.cc

[pkotwicz, 2016-06-09 01:40:26]

Description was changed from

==========
Upstream: Add additional checks before creating a WebAPK after clicking "Add to
Homescreen"

This CL makes "Add to Homescreen" from the app banner and from the app menu
create a WebAPK if the following conditions are met:
- Chrome is run with the --enable-webapk command line flag
- active tab is controlled by a service worker
- web manifest has the following non-empty fields:
  - name
  - short name
  - start_url
- display field in web manifest is either "fullscreen" or "standalone"

Additionally, this CL:
- Moves AppBannerDataFetcher::IsManifestValidForWebApp() to
  webapp_manifest_validator.cc because the function is used by both
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc

- Moves the computation of whether a web page is "web app capable" from
  ShortcutHelper.java to app_banner_data_fetcher.cc and
  add_to_homescreen_data_fetcher.cc - the same place that the computation of
  whether a web page is "WebAPK capable" is made.
==========

to

==========
Upstream: Add additional checks before creating a WebAPK after clicking "Add to
Homescreen"

This CL makes "Add to Homescreen" from the app banner and from the app menu
create a WebAPK if the following conditions are met:
- Chrome is run with the --enable-webapk command line flag
- active tab is controlled by a service worker
- web manifest has the following non-empty fields:
  - name
  - short name
  - start_url
- display field in web manifest is either "fullscreen" or "standalone"

Additionally, this CL:
- Moves AppBannerDataFetcher::IsManifestValidForWebApp() to
  webapp_manifest_validator.cc because the function is used by both
  app_banner_data_fetcher.cc and add_to_homescreen_data_fetcher.cc

- Moves the computation of whether a web page is "web app capable" from
  ShortcutHelper.java to app_banner_data_fetcher.cc and
  add_to_homescreen_data_fetcher.cc - the same place that the computation of
  whether a web page is "WebAPK capable" is made.

BUG=609122
==========

[dominickn, 2016-06-09 03:34:33]

On 2016/06/09 01:39:42, pkotwicz wrote:
> dominickn@ can you please take a look?
> 
> This CL upstreams code in the WebAPK experimental branch. It is an OKR to
> upstream everything. The CL depends on
> https://codereview.chromium.org/2046423003/
> 
> I am very open to restructuring this CL to make it fit better with your future
> plans for unifying add_to_homescreen_data_fetcher.cc and
> app_banner_data_fetcher.cc

Thanks for sending this through. I'm travelling for the next day for the Fizz
offsite and Blink-On, so my review may take an extra day or two. I will look
this
over and write up more details of what I have planned for extracting the
installable check from the app banner code.

[pkotwicz, 2016-07-05 02:10:13]

dominickn@ can you please take a look? This CL depends on
https://codereview.chromium.org/2046423003/

[dominickn, 2016-07-05 02:43:37]

On 2016/07/05 02:10:13, pkotwicz wrote:
> dominickn@ can you please take a look? This CL depends on
> https://codereview.chromium.org/2046423003/

I am about to dramatically refactor the banner code as per the document I sent
you and hanxi@ some weeks ago. Both crrev.com/2046423003 and this CL will need
substantial changes when the refactor lands, but the amount of duplicated
checking/manifest fetching will be substantially reduced. As such, I would
prefer that the refactor lands first because it is being motivated by the need
to support WebAPKs in add to homescreen and app banners.

[pkotwicz, 2016-07-06 01:38:56]

dominickn@ when do you expect the substantial changes to be (mostly) completed.
In two weeks?

[pkotwicz, 2016-07-06 01:39:34]

dominickn@ when do you expect the substantial changes to be (mostly) completed.
In two weeks? (Mentioning a random date)

[dominickn, 2016-07-06 01:43:07]

On 2016/07/06 01:39:34, pkotwicz wrote:
> dominickn@ when do you expect the substantial changes to be (mostly)
completed.
> In two weeks? (Mentioning a random date)

I want to have it done by the end of the no-meeting period, so yes, hopefully
two weeks. I know it's blocking you. :)

[pkotwicz, 2016-07-06 02:44:59]

Awesome!

[pkotwicz, 2016-08-17 02:35:45]

Abandoning this CL. This CL will be replaced with
https://codereview.chromium.org/2244223002/