[Profiles] Remove the NotificationService from the ProfileInfoCache

[Profiles] Remove the NotificationService from the ProfileInfoCache

This removes NOTIFICATION_PROFILE_CACHED_INFO_CHANGED and
NOTIFICATION_PROFILE_CACHE_PICTURE_SAVED notifications from all the
things.

This is part of the long process of killing the NotificationService with fire. In
particular, this ProfileInfoCache notification is terrible because it's incredibly
generic, gets shouted out any time *anything* happens to *any* profile, and
a bunch of things in Chrome listen to it, when in fact they care about very
specific profile notifications (i.e. has a name been changed, has a user been
added)

BUG=454845
TEST=All tests should pass. Chrome shouldn't behave weirdly. Everyone should
get a pony.

Committed: https://crrev.com/75ca0190e37b841400182d657d44ee077180f792
Cr-Commit-Position: refs/heads/master@{#318178}

[noms (inactive), 2015-02-05 00:37:32]

Patchset #1 (id:1) has been deleted

[noms (inactive), 2015-02-05 00:37:39]

Patchset #1 (id:20001) has been deleted

[noms (inactive), 2015-02-05 00:37:47]

Patchset #1 (id:40001) has been deleted

[noms (inactive), 2015-02-05 00:37:54]

Patchset #1 (id:60001) has been deleted

[noms (inactive), 2015-02-05 00:38:08]

Patchset #1 (id:80001) has been deleted

[noms (inactive), 2015-02-05 00:38:16]

Patchset #1 (id:100001) has been deleted

[noms (inactive), 2015-02-05 00:38:23]

Patchset #1 (id:120001) has been deleted

[noms (inactive), 2015-02-05 00:38:31]

Patchset #1 (id:140001) has been deleted

[noms (inactive), 2015-02-05 20:27:48]

Patchset #1 (id:160001) has been deleted

[noms (inactive), 2015-02-05 20:27:54]

Patchset #1 (id:170001) has been deleted

[noms (inactive), 2015-02-05 20:28:02]

Patchset #1 (id:190001) has been deleted

[noms (inactive), 2015-02-05 20:36:30]

noms@chromium.org changed reviewers:
+ caitkp@chromium.org, rlp@chromium.org

[noms (inactive), 2015-02-05 20:36:31]

Woohoo! I think I've removed the old notifications from the ProfileInfoCache,
and Chrome is still running! 
┏(･o･)┛♪┗ (･o･) ┓

Please take a look! (Rachel for the Profile bits, Cait for the notification
bits). Thanks!

Also, here is a super excited sheep that also probably did a ProfileInfoCache
refactor: http://i.imgur.com/WhD57wy.gifv

[Cait (Slow), 2015-02-06 16:47:22]

Notification bits look v. good! A couple clean-up comments  inline.

Also: from past battles with observers and such, I recommend running this CL
through the ASAN/valgrind bots before landing, just to make sure we're not
trying to remove observers from objects that are already gone.

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/jumplist...
File chrome/browser/jumplist_win.h (right):

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/jumplist...
chrome/browser/jumplist_win.h:35: class NotificationRegistrar;
Not needed anymore

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/notifica...
File chrome/browser/notifications/message_center_settings_controller.cc (right):

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/notifica...
chrome/browser/notifications/message_center_settings_controller.cc:437:
RebuildNotifierGroups();
This pattern (Rebuild & Notify Observers) seems to happen quite often. What
about either a) adding a RebuildAndNotify() method or b) passing a flag to the
Rebuild() method to tell whether or not to notify observers after rebuilding?

[rpetterson, 2015-02-07 00:04:58]

LGTM

overall pretty good. just a couple of  nits but none which should block checkin.

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/glass_browser_frame_view.cc (right):

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:591: if
(browser_view()->IsRegularOrGuestSession() && switches::IsNewAvatarMenu())
I feel like I'm seeing this update code frequently (here,
opaque_browser_frame_view). Would it make sense to group it into a helper
function?

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/ntp/ntp_login_handler.cc (right):

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/ui/webui...
chrome/browser/ui/webui/ntp/ntp_login_handler.cc:79:
g_browser_process->profile_manager()->GetProfileInfoCache().AddObserver(this);
What about making a static function for this? I think it might be
clearer/cleaner.
ProfileInfoCache::AddObserver(..)

Similarly for remove.

[noms (inactive), 2015-02-10 16:30:46]

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

[rpetterson, 2015-02-11 08:47:10]

I still vote for static functions ProfileInfoCache::AddObserver and
ProfileInfoCache::RemoveObserver, but I'm not gonna fight you on it. so still
LGTM.

[Cait (Slow), 2015-02-11 16:24:30]

Notifications bits LGTM

https://codereview.chromium.org/895803003/diff/310001/chrome/browser/jumplist...
File chrome/browser/jumplist_win.h (right):

https://codereview.chromium.org/895803003/diff/310001/chrome/browser/jumplist...
chrome/browser/jumplist_win.h:34: namespace content {
nit: no need to forward declare this now that the .h file is included.

[noms (inactive), 2015-02-12 18:53:32]

Patchset #5 (id:290001) has been deleted

[noms (inactive), 2015-02-12 21:46:00]

noms@chromium.org changed reviewers:
+ dbeam@chromium.org, dewittj@chromium.org, sky@chromium.org

[noms (inactive), 2015-02-12 21:46:02]

Adding owners while I address nits:

dewittj: c/b/notifications
sky: c/b/ui/views and the things directly under c/b/
dbeam: c/b/ui/webui

Thanks!

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/jumplist...
File chrome/browser/jumplist_win.h (right):

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/jumplist...
chrome/browser/jumplist_win.h:35: class NotificationRegistrar;
On 2015/02/06 16:47:22, Cait wrote:
> Not needed anymore

The jumplist uses it for the browser notifications, and it was getting the
headers for free from avatar_menu.h. Since I took these headers away from
avatar_menu.h, I needed to add them here...

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/notifica...
File chrome/browser/notifications/message_center_settings_controller.cc (right):

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/notifica...
chrome/browser/notifications/message_center_settings_controller.cc:437:
RebuildNotifierGroups();
On 2015/02/06 16:47:22, Cait wrote:
> This pattern (Rebuild & Notify Observers) seems to happen quite often. What
> about either a) adding a RebuildAndNotify() method or b) passing a flag to the
> Rebuild() method to tell whether or not to notify observers after rebuilding? 

Done.

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/glass_browser_frame_view.cc (right):

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:591: if
(browser_view()->IsRegularOrGuestSession() && switches::IsNewAvatarMenu())
On 2015/02/07 00:04:58, rpetterson wrote:
> I feel like I'm seeing this update code frequently (here,
> opaque_browser_frame_view). Would it make sense to group it into a helper
> function?

Done.

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/ntp/ntp_login_handler.cc (right):

https://codereview.chromium.org/895803003/diff/230001/chrome/browser/ui/webui...
chrome/browser/ui/webui/ntp/ntp_login_handler.cc:79:
g_browser_process->profile_manager()->GetProfileInfoCache().AddObserver(this);
On 2015/02/07 00:04:58, rpetterson wrote:
> What about making a static function for this? I think it might be
> clearer/cleaner.
> ProfileInfoCache::AddObserver(..)
> 
> Similarly for remove.

Hmm, I'm not sure if that would be confusing. We still have to keep the
non-static methods around (since they have to add to the observer list), and the
static methods would be very similar (same parameter, slightly different name).
I don't know if it would be clear why to use one over the other... :(

[dewittj, 2015-02-12 21:52:14]

c/b/notifications lgtm

[noms (inactive), 2015-02-13 19:36:02]

New patchsets have been uploaded after l-g-t-m from
rlp@chromium.org,caitkp@chromium.org,dewittj@chromium.org

[noms (inactive), 2015-02-17 15:37:25]

Scott and Dan: ping! :)

https://codereview.chromium.org/895803003/diff/310001/chrome/browser/jumplist...
File chrome/browser/jumplist_win.h (right):

https://codereview.chromium.org/895803003/diff/310001/chrome/browser/jumplist...
chrome/browser/jumplist_win.h:34: namespace content {
On 2015/02/11 16:24:30, Cait wrote:
> nit: no need to forward declare this now that the .h file is included.

Done.

[sky, 2015-02-19 15:59:36]

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/glass_browser_frame_view.cc (right):

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:588:
UpdateAvatar(this, NewAvatarButton::NATIVE_BUTTON);
The new code for all this looks quite a bit different than the old. I'm assuming
you know what you're doing;)

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:606: if
(browser_view()->IsOffTheRecord() || !switches::IsNewAvatarMenu())
Seems silly to duplicate code like this between glass and opaque. Is there a way
to refactor common code like this? Maybe a new superclass that glass/opaque
share?

[noms (inactive), 2015-02-19 16:07:20]

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/glass_browser_frame_view.cc (right):

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:588:
UpdateAvatar(this, NewAvatarButton::NATIVE_BUTTON);
On 2015/02/19 15:59:36, sky wrote:
> The new code for all this looks quite a bit different than the old. I'm
assuming
> you know what you're doing;)

Oof, that's some misplaced confidence :)

Jokes aside, very little has actually changed. Before we were updating the
button any time the ProfileInfoCache changed, now we're only updating it when
things affect it (the active name changes, profiles get added/deleted). A lot of
actions (changing the name of a different profile), don't really affect *this*
browser window.

And UpdateAvatar just engulfed lines 603-608.

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:606: if
(browser_view()->IsOffTheRecord() || !switches::IsNewAvatarMenu())
On 2015/02/19 15:59:36, sky wrote:
> Seems silly to duplicate code like this between glass and opaque. Is there a
way
> to refactor common code like this? Maybe a new superclass that glass/opaque
> share?

Do you think BrowserNonClientFrameView would be a good place? It's the one that
actually defines the listener (glass and opaque inherit it by magic), and this
UpdateAvatarInfo() function. I will try that.

[sky, 2015-02-19 16:09:18]

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/glass_browser_frame_view.cc (right):

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:606: if
(browser_view()->IsOffTheRecord() || !switches::IsNewAvatarMenu())
On 2015/02/19 16:07:19, Monica Dinculescu wrote:
> On 2015/02/19 15:59:36, sky wrote:
> > Seems silly to duplicate code like this between glass and opaque. Is there a
> way
> > to refactor common code like this? Maybe a new superclass that glass/opaque
> > share?
> 
> Do you think BrowserNonClientFrameView would be a good place? It's the one
that
> actually defines the listener (glass and opaque inherit it by magic), and this
> UpdateAvatarInfo() function. I will try that.

I was assuming you didn't want this for ash, in which case promoting doesn't
make sense. Would it make more sense to move the profile/avatar functionality
off into it's own class that both glass and opaque browser frame view own?

[noms (inactive), 2015-02-19 16:11:49]

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/glass_browser_frame_view.cc (right):

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:606: if
(browser_view()->IsOffTheRecord() || !switches::IsNewAvatarMenu())
On 2015/02/19 16:09:18, sky wrote:
> On 2015/02/19 16:07:19, Monica Dinculescu wrote:
> > On 2015/02/19 15:59:36, sky wrote:
> > > Seems silly to duplicate code like this between glass and opaque. Is there
a
> > way
> > > to refactor common code like this? Maybe a new superclass that
glass/opaque
> > > share?
> > 
> > Do you think BrowserNonClientFrameView would be a good place? It's the one
> that
> > actually defines the listener (glass and opaque inherit it by magic), and
this
> > UpdateAvatarInfo() function. I will try that.
> 
> I was assuming you didn't want this for ash, in which case promoting doesn't
> make sense. Would it make more sense to move the profile/avatar functionality
> off into it's own class that both glass and opaque browser frame view own?

Oh Ash has this button too (Metro mode, not ChromeOS)

[sky, 2015-02-19 17:53:07]

If it makes sense for all subclasses, then definitely promote it.

On Thu, Feb 19, 2015 at 8:11 AM,  <noms@chromium.org> wrote:
>
>
https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
> File chrome/browser/ui/views/frame/glass_browser_frame_view.cc (right):
>
>
https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
> chrome/browser/ui/views/frame/glass_browser_frame_view.cc:606: if
> (browser_view()->IsOffTheRecord() || !switches::IsNewAvatarMenu())
> On 2015/02/19 16:09:18, sky wrote:
>>
>> On 2015/02/19 16:07:19, Monica Dinculescu wrote:
>> > On 2015/02/19 15:59:36, sky wrote:
>> > > Seems silly to duplicate code like this between glass and opaque.
>
> Is there a
>>
>> > way
>> > > to refactor common code like this? Maybe a new superclass that
>
> glass/opaque
>>
>> > > share?
>> >
>> > Do you think BrowserNonClientFrameView would be a good place? It's
>
> the one
>>
>> that
>> > actually defines the listener (glass and opaque inherit it by
>
> magic), and this
>>
>> > UpdateAvatarInfo() function. I will try that.
>
>
>> I was assuming you didn't want this for ash, in which case promoting
>
> doesn't
>>
>> make sense. Would it make more sense to move the profile/avatar
>
> functionality
>>
>> off into it's own class that both glass and opaque browser frame view
>
> own?
>
> Oh Ash has this button too (Metro mode, not ChromeOS)
>
> https://codereview.chromium.org/895803003/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[noms, 2015-02-20 21:24:33]

noms@google.com changed reviewers:
+ noms@google.com

[noms, 2015-02-20 21:24:34]

Hi Scott,

Refactored ALL the things. I think everything is functionally the same, but
cleaner. Please take a look! Thanks :)

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/glass_browser_frame_view.cc (right):

https://codereview.chromium.org/895803003/diff/330001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/glass_browser_frame_view.cc:606: if
(browser_view()->IsOffTheRecord() || !switches::IsNewAvatarMenu())
On 2015/02/19 16:11:49, Monica Dinculescu wrote:
> On 2015/02/19 16:09:18, sky wrote:
> > On 2015/02/19 16:07:19, Monica Dinculescu wrote:
> > > On 2015/02/19 15:59:36, sky wrote:
> > > > Seems silly to duplicate code like this between glass and opaque. Is
there
> a
> > > way
> > > > to refactor common code like this? Maybe a new superclass that
> glass/opaque
> > > > share?
> > > 
> > > Do you think BrowserNonClientFrameView would be a good place? It's the one
> > that
> > > actually defines the listener (glass and opaque inherit it by magic), and
> this
> > > UpdateAvatarInfo() function. I will try that.
> > 
> > I was assuming you didn't want this for ash, in which case promoting doesn't
> > make sense. Would it make more sense to move the profile/avatar
functionality
> > off into it's own class that both glass and opaque browser frame view own?
> 
> Oh Ash has this button too (Metro mode, not ChromeOS)

Ok, done. I still had to override a method, as the avatar button needs to know
what kind of button it is (and the BrowserNonClientFrameView doesn't know), but
I think it looks much cleaner now.

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_non_client_frame_view.h (right):

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_non_client_frame_view.h:77: void
UpdateAvatarInfo();
For what it's worth, I have a plan of renaming these methods to better things
(like UpdateAvatarButton and UpdateIncognitoBadge) in the CL that cleans up the
old profiles UI.

[sky, 2015-02-20 21:43:20]

Looking much cleaner!

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_non_client_frame_view.h (right):

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_non_client_frame_view.h:95: void
BrowserNonClientFrameView::UpdateTaskbarDecoration();
nuke BrowserNonClientFrameView::.

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:189:
UpdateNewStyleAvatarInfo(this, NewAvatarButton::THEMED_BUTTON);
How about a GetNewAvatarButtonStyle() protected virtual that is overriden here
to return THEMED_BUTTON and default returns NATIVE_BUTTON?
UpdateNewStyleAvatar() is nuked and the one call site becomes
UpdateNewStyleAvatarInfo(this, GetNewAvatarButtonStyle()) ?

[noms (inactive), 2015-02-20 21:46:30]

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:189:
UpdateNewStyleAvatarInfo(this, NewAvatarButton::THEMED_BUTTON);
On 2015/02/20 21:43:19, sky wrote:
> How about a GetNewAvatarButtonStyle() protected virtual that is overriden here
> to return THEMED_BUTTON and default returns NATIVE_BUTTON?
> UpdateNewStyleAvatar() is nuked and the one call site becomes
> UpdateNewStyleAvatarInfo(this, GetNewAvatarButtonStyle()) ?

Hmm, but the `this` is also bad because BrowserNonClientFrameView isn't a
ButtonListener. Kind of scared of making it one. Should I?

[sky, 2015-02-20 21:51:01]

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:189:
UpdateNewStyleAvatarInfo(this, NewAvatarButton::THEMED_BUTTON);
On 2015/02/20 21:46:30, Monica Dinculescu wrote:
> On 2015/02/20 21:43:19, sky wrote:
> > How about a GetNewAvatarButtonStyle() protected virtual that is overriden
here
> > to return THEMED_BUTTON and default returns NATIVE_BUTTON?
> > UpdateNewStyleAvatar() is nuked and the one call site becomes
> > UpdateNewStyleAvatarInfo(this, GetNewAvatarButtonStyle()) ?
> 
> Hmm, but the `this` is also bad because BrowserNonClientFrameView isn't a
> ButtonListener. Kind of scared of making it one. Should I? 

Good point. And it looks like they all have different implementations.

Ok, keep it as you have it. But can UpdateNewStyleAvatar be protected?

[noms, 2015-02-23 15:46:20]

Patchset #9 (id:390001) has been deleted

[noms, 2015-02-23 15:46:52]

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_non_client_frame_view.h (right):

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_non_client_frame_view.h:95: void
BrowserNonClientFrameView::UpdateTaskbarDecoration();
On 2015/02/20 21:43:19, sky wrote:
> nuke BrowserNonClientFrameView::.

oops! done.

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/895803003/diff/370001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:189:
UpdateNewStyleAvatarInfo(this, NewAvatarButton::THEMED_BUTTON);
On 2015/02/20 21:51:01, sky wrote:
> On 2015/02/20 21:46:30, Monica Dinculescu wrote:
> > On 2015/02/20 21:43:19, sky wrote:
> > > How about a GetNewAvatarButtonStyle() protected virtual that is overriden
> here
> > > to return THEMED_BUTTON and default returns NATIVE_BUTTON?
> > > UpdateNewStyleAvatar() is nuked and the one call site becomes
> > > UpdateNewStyleAvatarInfo(this, GetNewAvatarButtonStyle()) ?
> > 
> > Hmm, but the `this` is also bad because BrowserNonClientFrameView isn't a
> > ButtonListener. Kind of scared of making it one. Should I? 
> 
> Good point. And it looks like they all have different implementations.
> 
> Ok, keep it as you have it. But can UpdateNewStyleAvatar be protected?

Done.

[sky, 2015-02-23 15:56:54]

LGTM

[noms (inactive), 2015-02-23 16:09:52]

Dan: ping! :)

Here's a cat-that-doesn't-fit-in-a-box gif for bribes:
http://i.imgur.com/nGVvHZR.gif

[Dan Beam, 2015-02-25 18:02:58]

https://codereview.chromium.org/895803003/diff/410001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/options_ui_browsertest.cc (left):

https://codereview.chromium.org/895803003/diff/410001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/options_ui_browsertest.cc:249:
wait_for_profile_deletion.Wait();
why is this not necessary now?

[noms (inactive), 2015-02-25 19:29:53]

https://codereview.chromium.org/895803003/diff/410001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/options_ui_browsertest.cc (left):

https://codereview.chromium.org/895803003/diff/410001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/options_ui_browsertest.cc:249:
wait_for_profile_deletion.Wait();
On 2015/02/25 18:02:58, Dan Beam wrote:
> why is this not necessary now?

If this is doing what it says it's doing (waiting for profile deletion), then
the browser_closed notification is mostly the same (the browsers are closed
after the profile has been deleted successfully:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/pro...)

[Dan Beam, 2015-02-25 20:20:17]

lgtm

[noms (inactive), 2015-02-25 20:21:05]

Thanks everyone!

[noms (inactive), 2015-02-25 20:21:13]

The CQ bit was checked by noms@chromium.org

[commit-bot: I haz the power, 2015-02-25 20:21:45]

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

[commit-bot: I haz the power, 2015-02-25 20:28:26]

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

[commit-bot: I haz the power, 2015-02-25 20:28:28]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)

[noms, 2015-02-25 22:36:54]

The CQ bit was checked by noms@google.com

[noms, 2015-02-25 22:36:55]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
rlp@chromium.org, dbeam@chromium.org, caitkp@chromium.org, dewittj@chromium.org
Link to the patchset: https://codereview.chromium.org/895803003/#ps430001
(title: "rebase")

[commit-bot: I haz the power, 2015-02-25 22:38:10]

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

[commit-bot: I haz the power, 2015-02-26 00:39:27]

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

[commit-bot: I haz the power, 2015-02-26 00:39:28]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
Timed out jobs are not retried to avoid causing additional load on the builders
with large pending queues.

[noms (inactive), 2015-02-26 02:28:38]

The CQ bit was checked by noms@chromium.org

[commit-bot: I haz the power, 2015-02-26 02:29:26]

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

[commit-bot: I haz the power, 2015-02-26 03:14:41]

Committed patchset #10 (id:430001)

[commit-bot: I haz the power, 2015-02-26 03:15:39]

Patchset 10 (id:??) landed as
https://crrev.com/75ca0190e37b841400182d657d44ee077180f792
Cr-Commit-Position: refs/heads/master@{#318178}