CameraPresenceNotifier observers removal supported.

CameraPresenceNotifier observers removal supported.

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

[merkulova, 2014-04-03 11:24:52]

This CL is FYI. No tests added. Just shown changes that I think are necessary.

[ygorshenin1, 2014-04-03 11:45:49]

Test is necessary for the fix.

https://codereview.chromium.org/213623005/diff/1/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/screens/user_image_screen_actor.h (right):

https://codereview.chromium.org/213623005/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/screens/user_image_screen_actor.h:27:
CameraPresenceNotifier::GetInstance()->RemoveObserver(this);
No need to call RemoveObserver() here. It'll be called in dtor of
CameraPresenceNotifier::Observer.

https://codereview.chromium.org/213623005/diff/1/chrome/browser/ui/webui/chro...
File
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.cc
(left):

https://codereview.chromium.org/213623005/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.cc:47:
CameraPresenceNotifier::GetInstance()->RemoveObserver(delegate_);
Please, leave the call to RemoveObserver() here. The reason is that delegate_ is
not destructed here, it's just unsubscribes from notifications.

https://codereview.chromium.org/213623005/diff/1/chrome/browser/ui/webui/chro...
File
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.h
(right):

https://codereview.chromium.org/213623005/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.h:28:
CameraPresenceNotifier::GetInstance()->RemoveObserver(this);
No need to call RemoveObserver() here. It'll be called in dtor of
CameraPresenceNotifier::Observer.

https://codereview.chromium.org/213623005/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc (left):

https://codereview.chromium.org/213623005/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc:51:
CameraPresenceNotifier::GetInstance()->RemoveObserver(screen_);
Please, leave the call to RemoveObserver().

https://codereview.chromium.org/213623005/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc
(right):

https://codereview.chromium.org/213623005/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc:84: if
(screen_)
It's OK to remove NULL from observer list.

[Nikita (slow), 2014-04-03 11:47:29]

https://codereview.chromium.org/213623005/diff/1/chrome/browser/chromeos/came...
File chrome/browser/chromeos/camera_presence_notifier.h (right):

https://codereview.chromium.org/213623005/diff/1/chrome/browser/chromeos/came...
chrome/browser/chromeos/camera_presence_notifier.h:23:
CameraPresenceNotifier::GetInstance()->RemoveObserver(this);
With virtual destructors all dtors for the type hierarchy will be called. This
means that you do need RemoveObserver() call only on the class that added itself
in AddObserver()

[merkulova, 2014-04-03 12:50:06]

Moved logic to screens classes for OOBE.

[Nikita (slow), 2014-04-03 13:47:48]

lgtm

Please work with Yuri to produce a test.

[Nikita (slow), 2014-04-03 13:47:51]

lgtm

Please work with Yuri to produce a test.

[ygorshenin1, 2014-04-04 10:57:11]

lgtm

[merkulova, 2014-04-04 11:14:40]

The CQ bit was checked by merkulova@chromium.org

[commit-bot: I haz the power, 2014-04-04 11:14:45]

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

[commit-bot: I haz the power, 2014-04-04 12:37:03]

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

[commit-bot: I haz the power, 2014-04-04 12:37:04]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[merkulova, 2014-04-04 12:40:06]

The CQ bit was checked by merkulova@chromium.org

[commit-bot: I haz the power, 2014-04-04 12:40:27]

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

[commit-bot: I haz the power, 2014-04-04 15:28:12]

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

[commit-bot: I haz the power, 2014-04-04 18:30:11]

Change committed as 261797