[WebAPKs]: Reduce the parameters of ShortcutHelper::AddToLauncherWithSkBitmap()

[WebAPKs]: Reduce the parameters of ShortcutHelper::AddToLauncherWithSkBitmap()

This CL removes the |webapp_id| and |splash_image_callback| parameters from
ShortcutHelper::AddToLauncherWithSkBitmap(). This CL adds the needed
information to ShortcutInfo so that AddToLauncherWithSkBitmap() can create
the callback itself.

The goal of this CL is to enable
AppBannerInfoBarDelegateAndroid::OnWebApkInstallFinished() to call
ShortcutHelper::AddToLauncherWithSkBitmap() in a subsequent CL.

BUG=696132
Review-Url: https://codereview.chromium.org/2724723002
Cr-Commit-Position: refs/heads/master@{#454080}
Committed: https://chromium.googlesource.com/chromium/src/+/d0d459e8dda997152b63f3fee88a1bf35b323a20

[pkotwicz, 2017-03-01 01:24:14]

Description was changed from

==========
[WebAPKs]: Reduce the parameters of ShortcutHelper::AddToLauncherWithSkBitmap()

This CL removes the |webapp_id| and |splash_image_callback| parameters from
ShortcutHelper::AddToLauncherWithSkBitmap(). This CL adds the needed
information the ShortcutInfo so that AddToLauncherWithSkBitmap() can create
the callback itself.

The goal of this CL is to enable
AppBannerInfoBarDelegateAndroid::OnWebApkInstallFinished() calling
ShortcutHelper::AddToLauncherWithSkBitmap() in a subsequent CL.

BUG=696132
==========

to

==========
[WebAPKs]: Reduce the parameters of ShortcutHelper::AddToLauncherWithSkBitmap()

This CL removes the |webapp_id| and |splash_image_callback| parameters from
ShortcutHelper::AddToLauncherWithSkBitmap(). This CL adds the needed
information to ShortcutInfo so that AddToLauncherWithSkBitmap() can create
the callback itself.

The goal of this CL is to enable
AppBannerInfoBarDelegateAndroid::OnWebApkInstallFinished() to call
ShortcutHelper::AddToLauncherWithSkBitmap() in a subsequent CL.

BUG=696132
==========

[pkotwicz, 2017-03-01 01:33:56]

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

[pkotwicz, 2017-03-01 01:33:56]

Dominick, can you please take a look?

Sam gave me the go ahead to fix https://crbug.com/696132 as we discussed on
Monday at today's WebAPK weekly meeting.

[dominickn, 2017-03-01 02:29:40]

Looks pretty good, mostly nits.

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/android/bann...
File chrome/browser/android/banners/app_banner_manager_android.cc (right):

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/android/bann...
chrome/browser/android/banners/app_banner_manager_android.cc:232:
ShortcutHelper::GetMinimumSplashImageSizeInPx();
Can you move all of this into CreateShortcutInfo?

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/android/shor...
File chrome/browser/android/shortcut_helper.cc (right):

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/android/shor...
chrome/browser/android/shortcut_helper.cc:15: #include "base/command_line.h"
This command_line.h include can be removed while you're here.

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/android/shor...
chrome/browser/android/shortcut_helper.cc:143:
base::Bind(&ShortcutHelper::FetchSplashScreenImage, web_contents,
Nit: remove the local and pass the result of base::Bind() as the last argument
in AddWebappWithSkBitmap

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/android/shor...
File chrome/browser/android/shortcut_helper.h (right):

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/android/shor...
chrome/browser/android/shortcut_helper.h:45: content::BrowserContext*
browser_context,
Nit: ideally this method should just take a WebContents as well for consistency
with AddToLauncherWithSkBitmap. Can you do that here, it's a fairly small change
(having a WebContents around is generally useful).

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

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc:10: #include
"base/callback.h"
I think you can remove the callback.h include now.

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

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapps/add_to_homescreen_data_fetcher.h:8: #include
"base/callback_forward.h"
This include isn't needed any more.

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

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapps/add_to_homescreen_manager.cc:119: 
Minor nit: remove newline on line 119

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/banners/app_...
File chrome/browser/banners/app_banner_manager.cc (right):

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/banners/app_...
chrome/browser/banners/app_banner_manager.cc:10: #include "base/callback.h"
Remove this include.

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/banners/app_...
File chrome/browser/banners/app_banner_manager.h (right):

https://codereview.chromium.org/2724723002/diff/1/chrome/browser/banners/app_...
chrome/browser/banners/app_banner_manager.h:11: #include
"base/callback_forward.h"
Remove this include

[pkotwicz, 2017-03-01 03:05:24]

Dominick, can you please take another look?

[dominickn, 2017-03-01 03:16:08]

lgtm, thanks

[pkotwicz, 2017-03-01 22:04:12]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2017-03-01 22:04:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org
Link to the patchset: https://codereview.chromium.org/2724723002/#ps40001
(title: "Merge branch 'master' into refactor_shortcut_helper2")

[commit-bot: I haz the power, 2017-03-01 22:04:51]

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, 2017-03-01 22:55:23]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488405852377190,
"parent_rev": "9b2fb40242a52cf1cf25b14eff6f095432c7bb81", "commit_rev":
"d0d459e8dda997152b63f3fee88a1bf35b323a20"}

[commit-bot: I haz the power, 2017-03-01 22:56:02]

Description was changed from

==========
[WebAPKs]: Reduce the parameters of ShortcutHelper::AddToLauncherWithSkBitmap()

This CL removes the |webapp_id| and |splash_image_callback| parameters from
ShortcutHelper::AddToLauncherWithSkBitmap(). This CL adds the needed
information to ShortcutInfo so that AddToLauncherWithSkBitmap() can create
the callback itself.

The goal of this CL is to enable
AppBannerInfoBarDelegateAndroid::OnWebApkInstallFinished() to call
ShortcutHelper::AddToLauncherWithSkBitmap() in a subsequent CL.

BUG=696132
==========

to

==========
[WebAPKs]: Reduce the parameters of ShortcutHelper::AddToLauncherWithSkBitmap()

This CL removes the |webapp_id| and |splash_image_callback| parameters from
ShortcutHelper::AddToLauncherWithSkBitmap(). This CL adds the needed
information to ShortcutInfo so that AddToLauncherWithSkBitmap() can create
the callback itself.

The goal of this CL is to enable
AppBannerInfoBarDelegateAndroid::OnWebApkInstallFinished() to call
ShortcutHelper::AddToLauncherWithSkBitmap() in a subsequent CL.

BUG=696132

Review-Url: https://codereview.chromium.org/2724723002
Cr-Commit-Position: refs/heads/master@{#454080}
Committed:
https://chromium.googlesource.com/chromium/src/+/d0d459e8dda997152b63f3fee88a...
==========

[commit-bot: I haz the power, 2017-03-01 22:56:03]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/d0d459e8dda997152b63f3fee88a...