Fix a bug where Guest Profiles in CrOS could cause crashes when collecting

Fix a bug where Guest Profiles in CrOS could cause crashes when collecting
data for the Website Settings options page.

BUG=409527
Committed: https://crrev.com/a35a53a65e17c4fe1d79f456a4f71827198c9b89
Cr-Commit-Position: refs/heads/master@{#296077}

[Daniel Nishi, 2014-09-19 00:26:05]

dhnishi@chromium.org changed reviewers:
+ bauerb@chromium.org, derat@chromium.org

[Daniel Nishi, 2014-09-19 00:26:06]

bauerb: PTAL at website_settings

derat: PTAL at process_power_map.cc

[Daniel Erat, 2014-09-19 00:39:41]

derat@chromium.org changed reviewers:
+ nkostylev@chromium.org

[Daniel Erat, 2014-09-19 00:39:42]

nikita, is there another approach that would make this just work for chrome os?
i assume we don't want to have special-casing like what's in
website_settings_handler.cc all over the place.

https://codereview.chromium.org/582983002/diff/20001/chrome/browser/power/pro...
File chrome/browser/power/process_power_collector.cc (right):

https://codereview.chromium.org/582983002/diff/20001/chrome/browser/power/pro...
chrome/browser/power/process_power_collector.cc:174: if (!origin_power_map)
nit: add a comment documenting that this can happen in chrome os guest mode?

https://codereview.chromium.org/582983002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/website_settings_handler.cc (right):

https://codereview.chromium.org/582983002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/website_settings_handler.cc:771: // ChromeOS
special case : in Guest mode settings are opened in Incognito
nit: s/ChromeOS/Chrome OS/

[Daniel Nishi, 2014-09-19 00:50:15]

https://codereview.chromium.org/582983002/diff/20001/chrome/browser/power/pro...
File chrome/browser/power/process_power_collector.cc (right):

https://codereview.chromium.org/582983002/diff/20001/chrome/browser/power/pro...
chrome/browser/power/process_power_collector.cc:174: if (!origin_power_map)
On 2014/09/19 00:39:42, Daniel Erat wrote:
> nit: add a comment documenting that this can happen in chrome os guest mode?

Done.

https://codereview.chromium.org/582983002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/website_settings_handler.cc (right):

https://codereview.chromium.org/582983002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/website_settings_handler.cc:771: // ChromeOS
special case : in Guest mode settings are opened in Incognito
On 2014/09/19 00:39:42, Daniel Erat wrote:
> nit: s/ChromeOS/Chrome OS/

Done.

[Bernhard Bauer, 2014-09-19 08:29:17]

LGTM, but I do agree with this:

On 2014/09/19 00:39:42, Daniel Erat wrote:
> nikita, is there another approach that would make this just work for chrome
os?
> i assume we don't want to have special-casing like what's in
> website_settings_handler.cc all over the place.

https://codereview.chromium.org/582983002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/website_settings_handler.cc (right):

https://codereview.chromium.org/582983002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/website_settings_handler.cc:771: // Chrome OS
special case : in Guest mode settings are opened in Incognito
Nit: no space before colon.

[Daniel Nishi, 2014-09-19 16:48:24]

https://codereview.chromium.org/582983002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/website_settings_handler.cc (right):

https://codereview.chromium.org/582983002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/website_settings_handler.cc:771: // Chrome OS
special case : in Guest mode settings are opened in Incognito
On 2014/09/19 08:29:17, Bernhard Bauer wrote:
> Nit: no space before colon.

Done.

[Nikita (slow), 2014-09-22 13:16:34]

On 2014/09/19 08:29:17, Bernhard Bauer wrote:
> LGTM, but I do agree with this:
> 
> On 2014/09/19 00:39:42, Daniel Erat wrote:
> > nikita, is there another approach that would make this just work for chrome
> os?
> > i assume we don't want to have special-casing like what's in
> > website_settings_handler.cc all over the place.

It seems that as long as Guest session uses Incognito profile all services that
rely on its parent profile would have to use workarounds like this one.

We should take a look at the new profile type: GUEST_PROFILE.

enum ProfileType {
  REGULAR_PROFILE,  // Login user's normal profile
  INCOGNITO_PROFILE,  // Login user's off-the-record profile
  GUEST_PROFILE,  // Guest session's profile
};

[Bernhard Bauer, 2014-09-22 13:23:22]

On 2014/09/22 13:16:34, Nikita Kostylev wrote:
> On 2014/09/19 08:29:17, Bernhard Bauer wrote:
> > LGTM, but I do agree with this:
> > 
> > On 2014/09/19 00:39:42, Daniel Erat wrote:
> > > nikita, is there another approach that would make this just work for
chrome
> > os?
> > > i assume we don't want to have special-casing like what's in
> > > website_settings_handler.cc all over the place.
> 
> It seems that as long as Guest session uses Incognito profile all services
that
> rely on its parent profile would have to use workarounds like this one.
> 
> We should take a look at the new profile type: GUEST_PROFILE.
> 
> enum ProfileType {
>   REGULAR_PROFILE,  // Login user's normal profile
>   INCOGNITO_PROFILE,  // Login user's off-the-record profile
>   GUEST_PROFILE,  // Guest session's profile
> };

Personally, I would rather go the other way, and make a Guest profile as much as
possible like a regular profile, except that its state gets wiped on signout
(but which the profile doesn't need to be aware of).

[scheib, 2014-09-22 20:14:59]

On 2014/09/22 13:23:22, Bernhard Bauer wrote:
> On 2014/09/22 13:16:34, Nikita Kostylev wrote:
> > On 2014/09/19 08:29:17, Bernhard Bauer wrote:
> > > LGTM, but I do agree with this:
> > > 
> > > On 2014/09/19 00:39:42, Daniel Erat wrote:
> > > > nikita, is there another approach that would make this just work for
> chrome
> > > os?
> > > > i assume we don't want to have special-casing like what's in
> > > > website_settings_handler.cc all over the place.
> > 
> > It seems that as long as Guest session uses Incognito profile all services
> that
> > rely on its parent profile would have to use workarounds like this one.
> > 
> > We should take a look at the new profile type: GUEST_PROFILE.
> > 
> > enum ProfileType {
> >   REGULAR_PROFILE,  // Login user's normal profile
> >   INCOGNITO_PROFILE,  // Login user's off-the-record profile
> >   GUEST_PROFILE,  // Guest session's profile
> > };
> 
> Personally, I would rather go the other way, and make a Guest profile as much
as
> possible like a regular profile, except that its state gets wiped on signout
> (but which the profile doesn't need to be aware of).

May I suggest we land the crash fix and then assign an issue to refactor this
into a more ideal solution?

[Daniel Erat, 2014-09-22 20:19:05]

sgtm

[Daniel Erat, 2014-09-22 21:48:05]

filed http://crbug.com/416644 and putting this in the CQ

[Daniel Erat, 2014-09-22 21:48:10]

The CQ bit was checked by derat@chromium.org

[commit-bot: I haz the power, 2014-09-22 21:49:20]

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

[commit-bot: I haz the power, 2014-09-22 22:46:10]

Committed patchset #4 (id:60001) as a8c298543e259a9c6a6a8331ab3da2384add61de

[commit-bot: I haz the power, 2014-09-22 22:46:45]

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