Cleanup/ Use ExtensionRegistry from app_sync_ui_state.*

Use ExtensionRegistry from app_sync_ui_state.*

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

[limasdf, 2014-07-21 16:24:15]

When you have time

[James Cook, 2014-07-21 17:26:51]

Please add this to AppSyncUIStateFactory::AppSyncUIStateFactory:

DependsOn(ExtensionRegistryFactory::GetInstance());

That will ensure that ExtensionRegistry is destroyed after AppSyncUIState.

Also, you might want to do a try job through linux_chromeos_valgrind just to
make sure you aren't leaking observers.

[limasdf, 2014-07-22 16:13:46]

>Please add this to >AppSyncUIStateFactory::AppSyncUIStateFactory:
> DependsOn(ExtensionRegistryFactory::GetInstance());

Done.
and it linux_chromeos_valgrind seems fine except with already exist failures.

Thanks you for the review. PTAL!

[limasdf, 2014-07-22 16:20:14]

you can see linux_chromeos_valgrind result from patch set #1

[James Cook, 2014-07-22 16:29:24]

LGTM with nit

https://codereview.chromium.org/403843005/diff/40001/chrome/browser/ui/ash/ap...
File chrome/browser/ui/ash/app_sync_ui_state.cc (right):

https://codereview.chromium.org/403843005/diff/40001/chrome/browser/ui/ash/ap...
chrome/browser/ui/ash/app_sync_ui_state.cc:142: void
AppSyncUIState::OnExtensionLoaded(content::BrowserContext* browser_context,
and when you change the order in the header, change the order of the functions
here too

https://codereview.chromium.org/403843005/diff/40001/chrome/browser/ui/ash/ap...
File chrome/browser/ui/ash/app_sync_ui_state.h (right):

https://codereview.chromium.org/403843005/diff/40001/chrome/browser/ui/ash/ap...
chrome/browser/ui/ash/app_sync_ui_state.h:70: //
extensions::ExtensionRegistryObserver overrides:
nit: Change this to match the order of the interfaces in the class
AppSyncUIState : public... declaration.  So list ProfileSyncServiceObserver
overrides first.

[limasdf, 2014-07-22 16:38:10]

https://codereview.chromium.org/403843005/diff/40001/chrome/browser/ui/ash/ap...
File chrome/browser/ui/ash/app_sync_ui_state.cc (right):

https://codereview.chromium.org/403843005/diff/40001/chrome/browser/ui/ash/ap...
chrome/browser/ui/ash/app_sync_ui_state.cc:142: void
AppSyncUIState::OnExtensionLoaded(content::BrowserContext* browser_context,
On 2014/07/22 16:29:24, James Cook wrote:
> and when you change the order in the header, change the order of the functions
> here too

Done.

https://codereview.chromium.org/403843005/diff/40001/chrome/browser/ui/ash/ap...
File chrome/browser/ui/ash/app_sync_ui_state.h (right):

https://codereview.chromium.org/403843005/diff/40001/chrome/browser/ui/ash/ap...
chrome/browser/ui/ash/app_sync_ui_state.h:70: //
extensions::ExtensionRegistryObserver overrides:
On 2014/07/22 16:29:24, James Cook wrote:
> nit: Change this to match the order of the interfaces in the class
> AppSyncUIState : public... declaration.  So list ProfileSyncServiceObserver
> overrides first.

Done.

[limasdf, 2014-07-22 16:38:14]

The CQ bit was checked by limasdf@gmail.com

[commit-bot: I haz the power, 2014-07-22 16:39:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/limasdf@gmail.com/403843005/60001

[commit-bot: I haz the power, 2014-07-22 18:40:52]

Change committed as 284741