Migrate add to homescreen data fetcher to use InstallableManager.

Migrate add to homescreen data fetcher to use InstallableManager.

This CL stops the add to homescreen data fetcher from fetching
a manifest and icon itself, and instead defer that task to the
InstallableManager. This is particularly beneficial for PWAs, which may
have already run the InstallableManager to fetch the manifest and icon
to verify whether they are banner eligible. In this case, the add to
homescreen data fetcher no longer needs to IPC to the renderer process,
as the data it needs is already cached in InstallableManager.

This also paves the way for the WebAPK install flow to be triggered from
the add to homescreen dialog. A future CL will make the add to
homescreen data fetcher check whether a site is a PWA using the
InstallableManager - kicking off the WebAPK flow if so.

This CL also tidies up includes and code style in the data fetcher and
dialog helper.

BUG=628921
Committed: https://crrev.com/ed8b475808ee068d8d453b78a46fe1d8f63f5662
Cr-Commit-Position: refs/heads/master@{#411483}

[dominickn, 2016-08-10 04:43:25]

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

[commit-bot: I haz the power, 2016-08-10 04:43:48]

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

[dominickn, 2016-08-10 04:45:27]

Description was changed from

==========
Make the add to homescreen data fetcher use the InstallableManager.

This CL stops the add to homescreen data fetcher from fetching
a manifest and icon itself, and instead defer that task to the
InstallableManager. This is particularly beneficial for PWAs, which may
have already run the InstallableManager to fetch the manifest and icon
to verify whether they are banner eligible. In this case, the add to
homescreen data fetcher no longer needs to IPC to the renderer process,
as the data it needs is already cached in InstallableManager.

This also paves the way for the WebAPK install flow to be triggered from
the add to homescreen dialog. A future CL will make the add to
homescreen data fetcher check whether a site is a PWA using the
InstallableManager - kicking off the WebAPK flow if so.

This CL also tidies up includes and code style in the data fetcher and
dialog helper.

BUG=628921
==========

to

==========
Make the add to homescreen data fetcher use the InstallableManager.

This CL stops the add to homescreen data fetcher from fetching
a manifest and icon itself, and instead defer that task to the
InstallableManager. This is particularly beneficial for PWAs, which may
have already run the InstallableManager to fetch the manifest and icon
to verify whether they are banner eligible. In this case, the add to
homescreen data fetcher no longer needs to IPC to the renderer process,
as the data it needs is already cached in InstallableManager.

This also paves the way for the WebAPK install flow to be triggered from
the add to homescreen dialog. A future CL will make the add to
homescreen data fetcher check whether a site is a PWA using the
InstallableManager - kicking off the WebAPK flow if so.

This CL also tidies up includes and code style in the data fetcher and
dialog helper.

BUG=628921
==========

[dominickn, 2016-08-10 04:45:27]

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

[dominickn, 2016-08-10 04:51:58]

Description was changed from

==========
Make the add to homescreen data fetcher use the InstallableManager.

This CL stops the add to homescreen data fetcher from fetching
a manifest and icon itself, and instead defer that task to the
InstallableManager. This is particularly beneficial for PWAs, which may
have already run the InstallableManager to fetch the manifest and icon
to verify whether they are banner eligible. In this case, the add to
homescreen data fetcher no longer needs to IPC to the renderer process,
as the data it needs is already cached in InstallableManager.

This also paves the way for the WebAPK install flow to be triggered from
the add to homescreen dialog. A future CL will make the add to
homescreen data fetcher check whether a site is a PWA using the
InstallableManager - kicking off the WebAPK flow if so.

This CL also tidies up includes and code style in the data fetcher and
dialog helper.

BUG=628921
==========

to

==========
Migrate add to homescreen data fetcher to use InstallableManager.

This CL stops the add to homescreen data fetcher from fetching
a manifest and icon itself, and instead defer that task to the
InstallableManager. This is particularly beneficial for PWAs, which may
have already run the InstallableManager to fetch the manifest and icon
to verify whether they are banner eligible. In this case, the add to
homescreen data fetcher no longer needs to IPC to the renderer process,
as the data it needs is already cached in InstallableManager.

This also paves the way for the WebAPK install flow to be triggered from
the add to homescreen dialog. A future CL will make the add to
homescreen data fetcher check whether a site is a PWA using the
InstallableManager - kicking off the WebAPK flow if so.

This CL also tidies up includes and code style in the data fetcher and
dialog helper.

BUG=628921
==========

[dominickn, 2016-08-10 05:04:06]

PTAL thanks!

CC list: FYI. This CL clashes with crrev.com/2228273002 and
crrev.com/2218413002, so depending on which ones land first, rebasing will be
incurred.

@hanxi: I've left TODOs for you to add WebAPK specific functionality.

[commit-bot: I haz the power, 2016-08-10 05:31:58]

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

[commit-bot: I haz the power, 2016-08-10 05:31:59]

Dry run: This issue passed the CQ dry run.

[Xi Han, 2016-08-10 14:00:56]

On 2016/08/10 05:04:06, dominickn wrote:
> PTAL thanks!
> 
> CC list: FYI. This CL clashes with crrev.com/2228273002 and
> crrev.com/2218413002, so depending on which ones land first, rebasing will be
> incurred.
> 
> @hanxi: I've left TODOs for you to add WebAPK specific functionality.

Thank you Dominick for wrapping up this CL!

[gone, 2016-08-10 22:31:58]

lgtm, but I'm a little confused about whether the CreateForWebContents call does
anything on the second pass.  I'm not seeing it in my local repo, so I'm
guessing I need to sync up.

https://chromiumcodereview.appspot.com/2230773002/diff/1/chrome/browser/andro...
File chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc (right):

https://chromiumcodereview.appspot.com/2230773002/diff/1/chrome/browser/andro...
chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc:118:
InstallableManager::CreateForWebContents(web_contents());
Is this a no-op if it's already happened once?

https://chromiumcodereview.appspot.com/2230773002/diff/1/chrome/browser/andro...
chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc:203:
favicon_base::TOUCH_PRECOMPOSED_ICON | favicon_base::TOUCH_ICON};
Should }; go on the next line down?  Honestly unsure.

[dominickn, 2016-08-10 23:31:46]

Thanks!

https://codereview.chromium.org/2230773002/diff/1/chrome/browser/android/weba...
File chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc (right):

https://codereview.chromium.org/2230773002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc:118:
InstallableManager::CreateForWebContents(web_contents());
On 2016/08/10 22:31:58, dfalcantara wrote:
> Is this a no-op if it's already happened once?

Yes, this does nothing if the manager has already been created.

https://codereview.chromium.org/2230773002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc:203:
favicon_base::TOUCH_PRECOMPOSED_ICON | favicon_base::TOUCH_ICON};
On 2016/08/10 22:31:58, dfalcantara wrote:
> Should }; go on the next line down?  Honestly unsure.

This is what clang-format gave me! Seems weird, but I think it's best to keep it
like this so it won't be reformatted again and again.

[dominickn, 2016-08-11 03:48:49]

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

[commit-bot: I haz the power, 2016-08-11 03:49:07]

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

[dominickn, 2016-08-11 03:50:12]

Rebased - PTAL to sanity check, thanks!

[commit-bot: I haz the power, 2016-08-11 04:30:11]

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

[commit-bot: I haz the power, 2016-08-11 04:30:12]

Dry run: This issue passed the CQ dry run.

[dominickn, 2016-08-11 06:57:36]

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

[commit-bot: I haz the power, 2016-08-11 06:57:57]

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-08-11 07:31:36]

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

[commit-bot: I haz the power, 2016-08-11 07:31:37]

Dry run: This issue passed the CQ dry run.

[gone, 2016-08-12 00:12:23]

lgtm

[dominickn, 2016-08-12 00:13:27]

The CQ bit was checked by dominickn@chromium.org

[commit-bot: I haz the power, 2016-08-12 00:14:34]

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-08-12 00:36:21]

Description was changed from

==========
Migrate add to homescreen data fetcher to use InstallableManager.

This CL stops the add to homescreen data fetcher from fetching
a manifest and icon itself, and instead defer that task to the
InstallableManager. This is particularly beneficial for PWAs, which may
have already run the InstallableManager to fetch the manifest and icon
to verify whether they are banner eligible. In this case, the add to
homescreen data fetcher no longer needs to IPC to the renderer process,
as the data it needs is already cached in InstallableManager.

This also paves the way for the WebAPK install flow to be triggered from
the add to homescreen dialog. A future CL will make the add to
homescreen data fetcher check whether a site is a PWA using the
InstallableManager - kicking off the WebAPK flow if so.

This CL also tidies up includes and code style in the data fetcher and
dialog helper.

BUG=628921
==========

to

==========
Migrate add to homescreen data fetcher to use InstallableManager.

This CL stops the add to homescreen data fetcher from fetching
a manifest and icon itself, and instead defer that task to the
InstallableManager. This is particularly beneficial for PWAs, which may
have already run the InstallableManager to fetch the manifest and icon
to verify whether they are banner eligible. In this case, the add to
homescreen data fetcher no longer needs to IPC to the renderer process,
as the data it needs is already cached in InstallableManager.

This also paves the way for the WebAPK install flow to be triggered from
the add to homescreen dialog. A future CL will make the add to
homescreen data fetcher check whether a site is a PWA using the
InstallableManager - kicking off the WebAPK flow if so.

This CL also tidies up includes and code style in the data fetcher and
dialog helper.

BUG=628921
==========

[commit-bot: I haz the power, 2016-08-12 00:36:22]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-12 00:39:47]

Description was changed from

==========
Migrate add to homescreen data fetcher to use InstallableManager.

This CL stops the add to homescreen data fetcher from fetching
a manifest and icon itself, and instead defer that task to the
InstallableManager. This is particularly beneficial for PWAs, which may
have already run the InstallableManager to fetch the manifest and icon
to verify whether they are banner eligible. In this case, the add to
homescreen data fetcher no longer needs to IPC to the renderer process,
as the data it needs is already cached in InstallableManager.

This also paves the way for the WebAPK install flow to be triggered from
the add to homescreen dialog. A future CL will make the add to
homescreen data fetcher check whether a site is a PWA using the
InstallableManager - kicking off the WebAPK flow if so.

This CL also tidies up includes and code style in the data fetcher and
dialog helper.

BUG=628921
==========

to

==========
Migrate add to homescreen data fetcher to use InstallableManager.

This CL stops the add to homescreen data fetcher from fetching
a manifest and icon itself, and instead defer that task to the
InstallableManager. This is particularly beneficial for PWAs, which may
have already run the InstallableManager to fetch the manifest and icon
to verify whether they are banner eligible. In this case, the add to
homescreen data fetcher no longer needs to IPC to the renderer process,
as the data it needs is already cached in InstallableManager.

This also paves the way for the WebAPK install flow to be triggered from
the add to homescreen dialog. A future CL will make the add to
homescreen data fetcher check whether a site is a PWA using the
InstallableManager - kicking off the WebAPK flow if so.

This CL also tidies up includes and code style in the data fetcher and
dialog helper.

BUG=628921

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

[commit-bot: I haz the power, 2016-08-12 00:39:48]

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