Add bookmark apps to taskbar automatically.

Add bookmark apps to taskbar automatically.

The code to do this was present but broken. Now the code works on
Windows.

BUG=456650
Committed: https://crrev.com/a0e8eaaed65690d95e2b97aff2369c8a969bbacf
Cr-Commit-Position: refs/heads/master@{#316162}

[benwells, 2015-02-11 06:18:47]

benwells@chromium.org changed reviewers:
+ jackhou@chromium.org

[jackhou1, 2015-02-11 07:14:42]

https://codereview.chromium.org/915973002/diff/1/chrome/browser/web_applicati...
File chrome/browser/web_applications/web_app.cc (right):

https://codereview.chromium.org/915973002/diff/1/chrome/browser/web_applicati...
chrome/browser/web_applications/web_app.cc:286: bool
ShouldCreateShortcutFor(web_app::ShortcutCreationReason reason,
Need to update the use of this function in web_app_mac.mm

https://codereview.chromium.org/915973002/diff/1/chrome/browser/web_applicati...
chrome/browser/web_applications/web_app.cc:296: if
(!base::CommandLine::ForCurrentProcess()->HasSwitch(
With the inverted logic, if there's an automatic shortcut creation, and that
switch is on, this will no longer create shortcuts for hosted apps.

I think the logic should stay the same for Mac. I.e. on Mac we ignore |reason|
as before. So maybe something like:

  // Only create the shortcut automatically if they are platform apps.
  bool app_type_requires_shortcut = extension->is_platform_app();

// An additional check here for OS X. We need app shims to be
// able to show them in the dock.
#if defined(OS_MACOSX)
  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
          switches::kEnableHostedAppShimCreation)) {
    app_type_requires_shortcut |= extension->is_hosted_app();
  }
#else
  // Always create the shortcut if it was requested by the user.
  app_type_requires_shortcut |= reason == SHORTCUT_CREATION_BY_USER;
#endif

[benwells, 2015-02-12 02:09:36]

https://codereview.chromium.org/915973002/diff/1/chrome/browser/web_applicati...
File chrome/browser/web_applications/web_app.cc (right):

https://codereview.chromium.org/915973002/diff/1/chrome/browser/web_applicati...
chrome/browser/web_applications/web_app.cc:286: bool
ShouldCreateShortcutFor(web_app::ShortcutCreationReason reason,
On 2015/02/11 07:14:42, jackhou1 wrote:
> Need to update the use of this function in web_app_mac.mm

Done.

https://codereview.chromium.org/915973002/diff/1/chrome/browser/web_applicati...
chrome/browser/web_applications/web_app.cc:296: if
(!base::CommandLine::ForCurrentProcess()->HasSwitch(
On 2015/02/11 07:14:42, jackhou1 wrote:
> With the inverted logic, if there's an automatic shortcut creation, and that
> switch is on, this will no longer create shortcuts for hosted apps.
> 
> I think the logic should stay the same for Mac. I.e. on Mac we ignore |reason|
> as before. So maybe something like:
> 
>   // Only create the shortcut automatically if they are platform apps.
>   bool app_type_requires_shortcut = extension->is_platform_app();
> 
> // An additional check here for OS X. We need app shims to be
> // able to show them in the dock.
> #if defined(OS_MACOSX)
>   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
>           switches::kEnableHostedAppShimCreation)) {
>     app_type_requires_shortcut |= extension->is_hosted_app();
>   }
> #else
>   // Always create the shortcut if it was requested by the user.
>   app_type_requires_shortcut |= reason == SHORTCUT_CREATION_BY_USER;
> #endif

Good point. I've update the logic to always create hosted apps on mac with the
flag enabled. I've also simplified the logic a bit to make it easier to
understand; hopefully its still equivalent.

[jackhou1, 2015-02-12 04:57:25]

lgtm

[benwells, 2015-02-12 04:57:50]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2015-02-12 04:58:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/915973002/20001

[commit-bot: I haz the power, 2015-02-12 05:24:31]

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

[commit-bot: I haz the power, 2015-02-12 05:24:32]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[benwells, 2015-02-12 23:35:49]

New patchsets have been uploaded after l-g-t-m from jackhou@chromium.org

[benwells, 2015-02-12 23:36:04]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2015-02-12 23:37:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/915973002/40001

[commit-bot: I haz the power, 2015-02-13 01:28:43]

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

[commit-bot: I haz the power, 2015-02-13 01:28:44]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[benwells, 2015-02-13 02:44:56]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2015-02-13 02:46:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/915973002/60001

[commit-bot: I haz the power, 2015-02-13 04:32:34]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-13 04:33:13]

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