Schedule an ephemeral profile for deletion once the last browser is closed.

Schedule an ephemeral profile for deletion once the last browser is closed.

BUG=302461
TEST=browser_tests
TEST=Manual:Set the ephemeral cloud policy on a profile and make sure the profile is deleted when Chrome is closed.

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

[pastarmovj, 2013-10-01 14:17:49]

Here is the next part. PTAL.

[mmenke, 2013-10-02 00:42:49]

On 2013/10/01 14:17:49, pastarmovj wrote:
> Here is the next part. PTAL.

Think you may want another profile owner for this one - I'm only an owner for
the component that runs on the IO thread.  Don't understand destruction on the
UI thread.

[pastarmovj, 2013-10-02 09:51:10]

Hey Rachel, can you please review this CL?

[pastarmovj, 2013-10-04 09:42:44]

On 2013/10/02 09:51:10, pastarmovj wrote:
> Hey Rachel, can you please review this CL?

Gentle ping :)

[rpetterson, 2013-10-04 19:42:20]

Apologies for the delay. I'm away at a conference this week.

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
chrome/browser/profiles/profile_manager.cc:702: // If it is the last browser of
a profile scheduled for deleting then do that
Nit: deleting => deletion

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
File chrome/browser/profiles/profile_manager_browsertest.cc (right):

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
chrome/browser/profiles/profile_manager_browsertest.cc:259: if
(!profiles::IsMultipleProfilesEnabled())
When would you not have multi profiles enabled? I think you'd want to force it
for this test so that the test will actually ruin and then put a DCHECK for
this.

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
chrome/browser/profiles/profile_manager_browsertest.cc:297:
EXPECT_EQ(path_profile2, browser_list->get(1)->profile()->GetPath());
You did this check earlier. Did you mean to check browser_list->get(2)?

[pastarmovj, 2013-10-10 11:46:32]

Thanks for the review! PTAL.

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
chrome/browser/profiles/profile_manager.cc:702: // If it is the last browser of
a profile scheduled for deleting then do that
On 2013/10/04 19:42:20, rpetterson wrote:
> Nit: deleting => deletion

Done.

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
File chrome/browser/profiles/profile_manager_browsertest.cc (right):

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
chrome/browser/profiles/profile_manager_browsertest.cc:259: if
(!profiles::IsMultipleProfilesEnabled())
On 2013/10/04 19:42:20, rpetterson wrote:
> When would you not have multi profiles enabled? I think you'd want to force it
> for this test so that the test will actually ruin and then put a DCHECK for
> this.

TBH I just copied this check from the other tests here assuming we keep some
non-multiprofile mode for testing just like the single-process browser flag. But
now that you asked I checked that this is done to exclude this test on Android
which doesn't support multi-profile for now, so I believe it is still valid to
do that?

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
chrome/browser/profiles/profile_manager_browsertest.cc:297:
EXPECT_EQ(path_profile2, browser_list->get(1)->profile()->GetPath());
On 2013/10/04 19:42:20, rpetterson wrote:
> You did this check earlier. Did you mean to check browser_list->get(2)?

Done.

[rpetterson, 2013-10-10 17:40:19]

LGTM

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
File chrome/browser/profiles/profile_manager_browsertest.cc (right):

https://codereview.chromium.org/25495003/diff/1/chrome/browser/profiles/profi...
chrome/browser/profiles/profile_manager_browsertest.cc:259: if
(!profiles::IsMultipleProfilesEnabled())
On 2013/10/10 11:46:32, pastarmovj wrote:
> On 2013/10/04 19:42:20, rpetterson wrote:
> > When would you not have multi profiles enabled? I think you'd want to force
it
> > for this test so that the test will actually ruin and then put a DCHECK for
> > this.
> 
> TBH I just copied this check from the other tests here assuming we keep some
> non-multiprofile mode for testing just like the single-process browser flag.
But
> now that you asked I checked that this is done to exclude this test on Android
> which doesn't support multi-profile for now, so I believe it is still valid to
> do that?

Got it. That makes sense.

[commit-bot: I haz the power, 2013-10-11 13:27:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pastarmovj@chromium.org/25495003/13001

[commit-bot: I haz the power, 2013-10-11 16:18:52]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-10-14 11:13:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pastarmovj@chromium.org/25495003/40001

[commit-bot: I haz the power, 2013-10-14 13:16:04]

Retried try job too often on win_rel for step(s) telemetry_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-10-14 13:17:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pastarmovj@chromium.org/25495003/40001

[commit-bot: I haz the power, 2013-10-14 17:18:05]

Change committed as 228474