Fixing a lifetime issue for Speech Recognition Bubble

Fixing a lifetime issue for Speech Recognition Bubble
It seems that on a slow XP machine the view can be deleted before the
Impl. Fixed by a simple observer pattern.

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

[Tommy Widenflycht, 2014-03-26 16:21:09]

Tomi, can you do an initial sanity pass, please?

[tommi (sloooow) - chröme, 2014-03-26 17:01:42]

https://codereview.chromium.org/213153002/diff/1/chrome/browser/ui/views/spee...
File chrome/browser/ui/views/speech_recognition_bubble_views.cc (right):

https://codereview.chromium.org/213153002/diff/1/chrome/browser/ui/views/spee...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:53: void
SetLifeTImeObserver(
SetLifeTimeObserver
(lowercase i)

https://codereview.chromium.org/213153002/diff/1/chrome/browser/ui/views/spee...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:55:
life_time_observer_ = life_time_observer;
fix indent

https://codereview.chromium.org/213153002/diff/1/chrome/browser/ui/views/spee...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:135:
BubbleDelegateView::OnWidgetDestroyed(widget);
fix indent

[Tommy Widenflycht, 2014-03-27 12:50:43]

https://codereview.chromium.org/213153002/diff/1/chrome/browser/ui/views/spee...
File chrome/browser/ui/views/speech_recognition_bubble_views.cc (right):

https://codereview.chromium.org/213153002/diff/1/chrome/browser/ui/views/spee...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:53: void
SetLifeTImeObserver(
On 2014/03/26 17:01:43, tommi wrote:
> SetLifeTimeObserver
> (lowercase i)

Done.

https://codereview.chromium.org/213153002/diff/1/chrome/browser/ui/views/spee...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:55:
life_time_observer_ = life_time_observer;
On 2014/03/26 17:01:43, tommi wrote:
> fix indent

Done.

https://codereview.chromium.org/213153002/diff/1/chrome/browser/ui/views/spee...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:135:
BubbleDelegateView::OnWidgetDestroyed(widget);
On 2014/03/26 17:01:43, tommi wrote:
> fix indent

Done.

[tommi (sloooow) - chröme, 2014-03-27 14:44:02]

lgtm

https://codereview.chromium.org/213153002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/speech_recognition_bubble_views.cc (right):

https://codereview.chromium.org/213153002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:134:
life_time_observer_ = life_time_observer;
nit: First DCHECK(!life_time_observer_)?
or if life_time_observer can be NULL,
DCHECK(life_time_observer_ == life_time_observer ||
       (!life_time_observer_ && life_time_observer) ||
       (life_time_observer_ && !life_time_observer));

https://codereview.chromium.org/213153002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:139: if
(life_time_observer_) {
nit: no {}

[Tommy Widenflycht, 2014-03-31 10:45:54]

msw@ & sky@: Need OWNERS approval

https://codereview.chromium.org/213153002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/speech_recognition_bubble_views.cc (right):

https://codereview.chromium.org/213153002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:134:
life_time_observer_ = life_time_observer;
On 2014/03/27 14:44:03, tommi wrote:
> nit: First DCHECK(!life_time_observer_)?
> or if life_time_observer can be NULL,
> DCHECK(life_time_observer_ == life_time_observer ||
>        (!life_time_observer_ && life_time_observer) ||
>        (life_time_observer_ && !life_time_observer));

Done.

https://codereview.chromium.org/213153002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:139: if
(life_time_observer_) {
On 2014/03/27 14:44:03, tommi wrote:
> nit: no {}

Done.

[sky, 2014-03-31 13:34:44]

LGTM

[msw, 2014-03-31 16:53:30]

https://codereview.chromium.org/213153002/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/speech_recognition_bubble_views.cc (right):

https://codereview.chromium.org/213153002/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:34: class
SpeechRecognitionBubbleViewLifeTimeObserver {
Use the pre-existing WidgetObserver, which already has an OnWidgetDestroyed
callback, and wouldn't necessitate any changes to the
SpeechRecognitionBubbleView class.

[Tommy Widenflycht, 2014-04-01 11:29:03]

https://codereview.chromium.org/213153002/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/speech_recognition_bubble_views.cc (right):

https://codereview.chromium.org/213153002/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/speech_recognition_bubble_views.cc:34: class
SpeechRecognitionBubbleViewLifeTimeObserver {
On 2014/03/31 16:53:30, msw wrote:
> Use the pre-existing WidgetObserver, which already has an OnWidgetDestroyed
> callback, and wouldn't necessitate any changes to the
> SpeechRecognitionBubbleView class.

Done.

[msw, 2014-04-01 17:25:22]

lgtm

[Tommy Widenflycht, 2014-04-02 13:05:38]

The CQ bit was checked by tommyw@chromium.org

[commit-bot: I haz the power, 2014-04-02 13:05:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommyw@chromium.org/213153002/60001

[commit-bot: I haz the power, 2014-04-02 14:23:18]

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

[commit-bot: I haz the power, 2014-04-02 14:23:19]

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

[Tommy Widenflycht, 2014-04-03 08:38:17]

The CQ bit was checked by tommyw@chromium.org

[Tommy Widenflycht, 2014-04-03 08:38:20]

The CQ bit was unchecked by tommyw@chromium.org

[Tommy Widenflycht, 2014-04-04 11:04:04]

The CQ bit was checked by tommyw@chromium.org

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

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommyw@chromium.org/213153002/80001

[commit-bot: I haz the power, 2014-04-04 13:48:27]

Change committed as 261737