[win] Set the profile path when enabling the app launcher.

[win] Set the profile path when enabling the app launcher.

On windows, when enabling the app launcher for the first time via the
webstore the app launcher is not shown, and if you switch to a different
profile and exit the browser before activiating the app launcher for the
first time, it can show a different profile.

This passes the profile responsible for enabling the app launcher
through to platform-specific enabling code, which then ensures the
profile path is saved to the local prefs.

BUG=259654
TEST=On windows, in a fresh user-data-dir, make two users and leave
browser windows open. Install an app from the webstore using the second
user after signing in. Close that browser. Close the first browser.
Launch the app launcher. Launcher should show the second user, with
apps, and not a sign in page for the first user.
TBR=xiyuan@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=211624

[tapted, 2013-07-11 06:54:45]

https://codereview.chromium.org/18421007/diff/19001/apps/app_launcher.cc
File apps/app_launcher.cc (left):

https://codereview.chromium.org/18421007/diff/19001/apps/app_launcher.cc#oldc...
apps/app_launcher.cc:28: #if defined(USE_ASH)
guarding the next two lines doesn't really add anything, and makes things less
readable..

https://codereview.chromium.org/18421007/diff/19001/apps/app_launcher.cc
File apps/app_launcher.cc (right):

https://codereview.chromium.org/18421007/diff/19001/apps/app_launcher.cc#newc...
apps/app_launcher.cc:22: bool IsAppLauncherEnabled() {
note: since /apps is skipped on andriod and iOS, the first #if is now logically
equivalent to 

#if defined(OS_LINUX) && !defined(OS_CHROMEOS)

maybe better is:

#if defined(OS_CHROMEOS)
  return true;
#elif defined(OS_LINUX)
  return false;
#elif defined(ENABLE_APP_LIST)
#error
#else
/* etc */

[tapted, 2013-07-11 07:02:02]

https://codereview.chromium.org/18421007/diff/19001/chrome/common/chrome_swit...
File chrome/common/chrome_switches.cc (left):

https://codereview.chromium.org/18421007/diff/19001/chrome/common/chrome_swit...
chrome/common/chrome_switches.cc:265: // Replaces the audio IPC layer for
<audio> and <video> with a mock audio
note: this was previously attached to a kDisableAudio flag which was reaped
without this comment.

[benwells, 2013-07-12 01:41:47]

https://codereview.chromium.org/18421007/diff/19001/apps/app_launcher.cc
File apps/app_launcher.cc (right):

https://codereview.chromium.org/18421007/diff/19001/apps/app_launcher.cc#newc...
apps/app_launcher.cc:22: bool IsAppLauncherEnabled() {
On 2013/07/11 06:54:45, tapted wrote:
> note: since /apps is skipped on andriod and iOS, the first #if is now
logically
> equivalent to 
> 
> #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
> 
> maybe better is:
> 
> #if defined(OS_CHROMEOS)
>   return true;
> #elif defined(OS_LINUX)
>   return false;
> #elif defined(ENABLE_APP_LIST)
> #error
> #else
> /* etc */

Why would that be better? It is longer, and couples the ifdeffery with the
gypdeffery (shudder).

https://codereview.chromium.org/18421007/diff/19001/chrome/browser/ui/app_lis...
File chrome/browser/ui/app_list/app_list_service.h (left):

https://codereview.chromium.org/18421007/diff/19001/chrome/browser/ui/app_lis...
chrome/browser/ui/app_list/app_list_service.h:48: // TODO(koz): Merge this into
ShowAppList().
I think this renaming is a cleanup and not necessary to fix the profile bug. Why
has the TODO gone too?

[tapted, 2013-07-12 04:58:27]

PTAL - this now doesn't expose the EnableAppList flow on OSX. Instead it's about
decoupling the platform-specific 'create shortcuts' part of EnableAppList from
the "set flag and profile path" part which is common (but is currently missing
the "set profile path" sub-part, which this CL adds).

https://codereview.chromium.org/18421007/diff/19001/apps/app_launcher.cc
File apps/app_launcher.cc (right):

https://codereview.chromium.org/18421007/diff/19001/apps/app_launcher.cc#newc...
apps/app_launcher.cc:22: bool IsAppLauncherEnabled() {
On 2013/07/12 01:41:48, benwells wrote:
> On 2013/07/11 06:54:45, tapted wrote:
> > note: since /apps is skipped on andriod and iOS, the first #if is now
> logically
> > equivalent to 
> > 
> > #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
> > 
> > maybe better is:
> > 
> > #if defined(OS_CHROMEOS)
> >   return true;
> > #elif defined(OS_LINUX)
> >   return false;
> > #elif defined(ENABLE_APP_LIST)
> > #error
> > #else
> > /* etc */
> 
> Why would that be better? It is longer, and couples the ifdeffery with the
> gypdeffery (shudder).

The alternative makes it possible to #error in case this file gets exposed to
some invalid set of flags, and make the CrOS condition more obvious. But yeah,
that would assume someone had made a mistake in the gyp conditions.

https://codereview.chromium.org/18421007/diff/19001/chrome/browser/ui/app_lis...
File chrome/browser/ui/app_list/app_list_service.h (left):

https://codereview.chromium.org/18421007/diff/19001/chrome/browser/ui/app_lis...
chrome/browser/ui/app_list/app_list_service.h:48: // TODO(koz): Merge this into
ShowAppList().
On 2013/07/12 01:41:48, benwells wrote:
> I think this renaming is a cleanup and not necessary to fix the profile bug.
Why
> has the TODO gone too?

The 2-3 parts of this function were (a) set the local_pref (b) maybe load the
profile asynchronously, and (c) show the app list.

(a) is now SetProfilePath, which is now also needed by EnableAppList, so we can
set a profile without showing it the launcher.

(b)-(c) are ShowForSavedProfile

The (a) part is merged into ShowAppList, but it doesn't make sense for (b) and
(c) to go there, since that path is only needed when there's a chance the
Profile is not leaded, which is never the case for ShowAppList.

[tapted, 2013-07-12 08:56:58]

I think this is the minimal change. I've moved the rest to
http://crrev.com/19091003 and http://crrev.com/18736003 to make sure I wasn't
missing anything.

The added SaveProfilePathToLocalState(..) still feels out of place to me. It's 
removed in http://crrev.com/19091003 , but like this it will give us a better
chance of merging (although webstore_private_api.cc has moved on quite a bit
already..)

[benwells, 2013-07-15 01:05:50]

lgtm

[tapted, 2013-07-15 01:22:38]

TBR xiyuan for refactoring change in app_list_service_ash.cc that is mechanical
in nature.

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    chrome/browser/ui/ash/app_list/app_list_service_ash.cc

Presubmit checks took 3.7s to calculate.

[commit-bot: I haz the power, 2013-07-15 01:22:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tapted@chromium.org/18421007/81001

[commit-bot: I haz the power, 2013-07-15 09:42:49]

Change committed as 211624