Move default apps from Chrome to Chrome OS

Move default apps from Chrome to Chrome OS

Also changed dir with default apps for managed users.
Main CL that moves cache: https://gerrit-int.chromium.org/#/c/42720/

BUG=271247
TEST=manual
R=jhawkins@chromium.org, mek@chromium.org

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

[asargent_no_longer_on_chrome, 2013-08-14 21:40:11]

Adding mek as reviewer instead of me since he's now in the extensions OWNERS -
trying to lighten my review load =)

[Marijn Kruisselbrink, 2013-08-14 23:35:41]

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/external_provider_impl.cc (right):

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/external_provider_impl.cc:390:
chrome::DIR_STANDALONE_EXTERNAL_EXTENSIONS;
I must be missing something, in my chrome/common/chrome_paths.h
DIR_STANDALONE_EXTERNAL_EXTENSIONS is only defined for OS_LINUX, so either this
shouldn't compile (but it does), or OS_LINUX implies OS_CHROMEOS making the
above #if slightly redundant (and confusing).

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/external_provider_impl.cc:406:
provider_list->push_back(
I assume !profile->IsManaged implies !is_chromeos_demo_session, so
DIR_EXTERNAL_EXTENSIONS still only gets added when it's not a chromeos demo
session?

[Dmitry Polukhin, 2013-08-14 23:44:09]

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/external_provider_impl.cc (right):

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/external_provider_impl.cc:390:
chrome::DIR_STANDALONE_EXTERNAL_EXTENSIONS;
On 2013/08/14 23:35:41, Marijn Kruisselbrink wrote:
> I must be missing something, in my chrome/common/chrome_paths.h
> DIR_STANDALONE_EXTERNAL_EXTENSIONS is only defined for OS_LINUX, so either
this
> shouldn't compile (but it does), or OS_LINUX implies OS_CHROMEOS making the
> above #if slightly redundant (and confusing).

OS_CHROMEOS implies OS_LINUX. I tried to make it clearly that this piece is used
for both OSes. If you think that it adds confusion, I'll remove redundant
condition. In general I don't like how this code is organized but I don't see
how it can be made easier without code duplication.

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/external_provider_impl.cc:406:
provider_list->push_back(
On 2013/08/14 23:35:41, Marijn Kruisselbrink wrote:
> I assume !profile->IsManaged implies !is_chromeos_demo_session, so
> DIR_EXTERNAL_EXTENSIONS still only gets added when it's not a chromeos demo
> session?

Yes, managed and demo are mutually exclusive modes. Here I swapped
DIR_EXTERNAL_EXTENSIONS with DIR_STANDALONE_EXTERNAL_EXTENSIONS.

[Marijn Kruisselbrink, 2013-08-14 23:59:42]

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/external_provider_impl.cc (right):

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/external_provider_impl.cc:390:
chrome::DIR_STANDALONE_EXTERNAL_EXTENSIONS;
On 2013/08/14 23:44:09, Dmitry Polukhin wrote:
> On 2013/08/14 23:35:41, Marijn Kruisselbrink wrote:
> > I must be missing something, in my chrome/common/chrome_paths.h
> > DIR_STANDALONE_EXTERNAL_EXTENSIONS is only defined for OS_LINUX, so either
> this
> > shouldn't compile (but it does), or OS_LINUX implies OS_CHROMEOS making the
> > above #if slightly redundant (and confusing).
> 
> OS_CHROMEOS implies OS_LINUX. I tried to make it clearly that this piece is
used
> for both OSes. If you think that it adds confusion, I'll remove redundant
> condition. In general I don't like how this code is organized but I don't see
> how it can be made easier without code duplication.
Yeah, I'm not sure what is more confusing, having the || defined(OS_CHROMEOS),
which makes it look like other checks for OS_LINUX don't apply to chromeos, or
making it clear here that this does in fact also apply to chromeos.

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/external_provider_impl.cc:406:
provider_list->push_back(
On 2013/08/14 23:44:09, Dmitry Polukhin wrote:
> On 2013/08/14 23:35:41, Marijn Kruisselbrink wrote:
> > I assume !profile->IsManaged implies !is_chromeos_demo_session, so
> > DIR_EXTERNAL_EXTENSIONS still only gets added when it's not a chromeos demo
> > session?
> 
> Yes, managed and demo are mutually exclusive modes. Here I swapped
> DIR_EXTERNAL_EXTENSIONS with DIR_STANDALONE_EXTERNAL_EXTENSIONS.
Well, if managed and demo are mutually exclusive, then it would be possible to
be both not managed and in demo mode. So where in the before code
DIR_EXTERNAL_EXTENSIONS only got added when not in a demo session, in the new
code it would get added also in demo mode (since apparently in demo mode
IsManaged would always be false, so it would always execute this block of code).

https://codereview.chromium.org/22931006/diff/1/chrome/common/chrome_paths.cc
File chrome/common/chrome_paths.cc (right):

https://codereview.chromium.org/22931006/diff/1/chrome/common/chrome_paths.cc...
chrome/common/chrome_paths.cc:400: if
(!PathService::Get(chrome::DIR_STANDALONE_EXTERNAL_EXTENSIONS, &cur))
I'm assuming ENABLE_MANAGED_USERS is only ever set on OS_LINUX?

[Dmitry Polukhin, 2013-08-15 00:28:45]

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/external_provider_impl.cc (right):

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/external_provider_impl.cc:390:
chrome::DIR_STANDALONE_EXTERNAL_EXTENSIONS;
On 2013/08/14 23:59:42, Marijn Kruisselbrink wrote:
> On 2013/08/14 23:44:09, Dmitry Polukhin wrote:
> > On 2013/08/14 23:35:41, Marijn Kruisselbrink wrote:
> > > I must be missing something, in my chrome/common/chrome_paths.h
> > > DIR_STANDALONE_EXTERNAL_EXTENSIONS is only defined for OS_LINUX, so either
> > this
> > > shouldn't compile (but it does), or OS_LINUX implies OS_CHROMEOS making
the
> > > above #if slightly redundant (and confusing).
> > 
> > OS_CHROMEOS implies OS_LINUX. I tried to make it clearly that this piece is
> used
> > for both OSes. If you think that it adds confusion, I'll remove redundant
> > condition. In general I don't like how this code is organized but I don't
see
> > how it can be made easier without code duplication.
> Yeah, I'm not sure what is more confusing, having the || defined(OS_CHROMEOS),
> which makes it look like other checks for OS_LINUX don't apply to chromeos, or
> making it clear here that this does in fact also apply to chromeos.

I removed '|| defined(OS_CHROMEOS)' because we don't do it in other cases so
there is no sense to do different here.

https://codereview.chromium.org/22931006/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/external_provider_impl.cc:406:
provider_list->push_back(
On 2013/08/14 23:59:42, Marijn Kruisselbrink wrote:
> On 2013/08/14 23:44:09, Dmitry Polukhin wrote:
> > On 2013/08/14 23:35:41, Marijn Kruisselbrink wrote:
> > > I assume !profile->IsManaged implies !is_chromeos_demo_session, so
> > > DIR_EXTERNAL_EXTENSIONS still only gets added when it's not a chromeos
demo
> > > session?
> > 
> > Yes, managed and demo are mutually exclusive modes. Here I swapped
> > DIR_EXTERNAL_EXTENSIONS with DIR_STANDALONE_EXTERNAL_EXTENSIONS.
> Well, if managed and demo are mutually exclusive, then it would be possible to
> be both not managed and in demo mode. So where in the before code
> DIR_EXTERNAL_EXTENSIONS only got added when not in a demo session, in the new
> code it would get added also in demo mode (since apparently in demo mode
> IsManaged would always be false, so it would always execute this block of
code).

Good catch, thanks! I accidentally changed the behavior. Fixed.

[Dmitry Polukhin, 2013-08-15 19:52:24]

PTAL

[Marijn Kruisselbrink, 2013-08-15 19:53:45]

lgtm

[Dmitry Polukhin, 2013-08-15 19:59:06]

+jhawkins@ for OWNERS review in
  chrome/chrome.gyp
  chrome/common/chrome_paths.cc

[James Hawkins, 2013-08-19 23:26:04]

lgtm

[Dmitry Polukhin, 2013-08-29 22:56:00]

Committed patchset #4 manually as r220410 (presubmit successful).