Fix duplicate logging of ActiveBrowserChanged action on views.

Fix duplicate logging of ActiveBrowserChanged action on views.

This was caused by redundant calls to BrowserList::SetLastActive,
one from BrowserFrame and another from BrowserView:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
BrowserList::SetLastActive()

and:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
views::Widget::OnNativeWidgetActivationChanged()
BrowserView::OnWidgetActivationChanged()
BrowserList::SetLastActive()

The change removes the call from BrowserFrame since it seems
redundant with the one in BrowserView which observes its widget
(via GetWidget()->AddObserver(this) call in InitViews()), so it
will always get the OnWidgetActivationChanged() notifications -
and it needs them for other things already.

Also fixes ChromeVisibilityObserverBrowserTest on Windows which was
fragile to the ordering of active/inactive events (and just happened to
work due to the duplicate events). That test was also made unnecessarily
complex by https://codereview.chromium.org/2591783002 so this change
reverts most of that CL and simplifies things via RunLoop::RunUntilIdle().

BUG=689553
TEST=Open two windows and then load chrome://user-actions. Switch
windows and observe only a single instance of ActiveBrowserChanged
logged per window switch.

Review-Url: https://codereview.chromium.org/2939943004
Cr-Commit-Position: refs/heads/master@{#483695}
Committed: https://chromium.googlesource.com/chromium/src/+/ba8e10dc8119f1bd7d2f365b2b3078f68aeb16cf

[Alexei Svitkine (slow), 2017-06-15 17:24:45]

The CQ bit was checked by asvitkine@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-15 17:25:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Alexei Svitkine (slow), 2017-06-15 17:42:08]

asvitkine@chromium.org changed reviewers:
+ pkasting@chromium.org

[commit-bot: I haz the power, 2017-06-15 19:35:11]

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

[commit-bot: I haz the power, 2017-06-15 19:35:12]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Alexei Svitkine (slow), 2017-06-15 19:35:54]

Actually, please hold off on this review.

Looks like there's a problem with
ChromeVisibilityObserverBrowserTest.VisibilityTest on Windows.

Will investigate.

[Alexei Svitkine (slow), 2017-06-27 21:34:05]

Description was changed from

==========
Fix duplicate logging of ActiveBrowserChanged action on views.

This was caused by redundant calls to BrowserList::SetLastActive,
one from BrowserFrame and another from BrowserView:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
BrowserList::SetLastActive()

and:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
views::Widget::OnNativeWidgetActivationChanged()
BrowserView::OnWidgetActivationChanged()
BrowserList::SetLastActive()

The change removes the call from BrowserFrame since it seems
redundant with the one in BrowserView which observes its widget
(via GetWidget()->AddObserver(this) call in InitViews()), so it
will always get the OnWidgetActivationChanged() notifications -
and it needs them for other things already.

BUG=689553
TEST=Open two windows and then load chrome://user-actions. Switch
windows and observe only a single instance of ActiveBrowserChanged
logged per window switch.
==========

to

==========
Fix duplicate logging of ActiveBrowserChanged action on views.

This was caused by redundant calls to BrowserList::SetLastActive,
one from BrowserFrame and another from BrowserView:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
BrowserList::SetLastActive()

and:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
views::Widget::OnNativeWidgetActivationChanged()
BrowserView::OnWidgetActivationChanged()
BrowserList::SetLastActive()

The change removes the call from BrowserFrame since it seems
redundant with the one in BrowserView which observes its widget
(via GetWidget()->AddObserver(this) call in InitViews()), so it
will always get the OnWidgetActivationChanged() notifications -
and it needs them for other things already.

BUG=689553
TEST=Open two windows and then load chrome://user-actions. Switch
windows and observe only a single instance of ActiveBrowserChanged
logged per window switch.
==========

[Alexei Svitkine (slow), 2017-06-27 21:34:06]

asvitkine@chromium.org changed reviewers:
- pkasting@chromium.org

[Alexei Svitkine (slow), 2017-06-29 18:53:48]

The CQ bit was checked by asvitkine@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-29 18:54:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Alexei Svitkine (slow), 2017-06-29 18:56:19]

Description was changed from

==========
Fix duplicate logging of ActiveBrowserChanged action on views.

This was caused by redundant calls to BrowserList::SetLastActive,
one from BrowserFrame and another from BrowserView:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
BrowserList::SetLastActive()

and:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
views::Widget::OnNativeWidgetActivationChanged()
BrowserView::OnWidgetActivationChanged()
BrowserList::SetLastActive()

The change removes the call from BrowserFrame since it seems
redundant with the one in BrowserView which observes its widget
(via GetWidget()->AddObserver(this) call in InitViews()), so it
will always get the OnWidgetActivationChanged() notifications -
and it needs them for other things already.

BUG=689553
TEST=Open two windows and then load chrome://user-actions. Switch
windows and observe only a single instance of ActiveBrowserChanged
logged per window switch.
==========

to

==========
Fix duplicate logging of ActiveBrowserChanged action on views.

This was caused by redundant calls to BrowserList::SetLastActive,
one from BrowserFrame and another from BrowserView:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
BrowserList::SetLastActive()

and:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
views::Widget::OnNativeWidgetActivationChanged()
BrowserView::OnWidgetActivationChanged()
BrowserList::SetLastActive()

The change removes the call from BrowserFrame since it seems
redundant with the one in BrowserView which observes its widget
(via GetWidget()->AddObserver(this) call in InitViews()), so it
will always get the OnWidgetActivationChanged() notifications -
and it needs them for other things already.

Also fixes ChromeVisibilityObserverBrowserTest on Windows which
was fragile to the ordering of active/inactive events. It was also made
unnecessarily complex by https://codereview.chromium.org/2591783002
so this change actually reverts most of that CL by using the simpler
RunLoop::RunUntilIdle().

BUG=689553
TEST=Open two windows and then load chrome://user-actions. Switch
windows and observe only a single instance of ActiveBrowserChanged
logged per window switch.
==========

[Alexei Svitkine (slow), 2017-06-29 18:56:32]

Patchset #2 (id:20001) has been deleted

[Alexei Svitkine (slow), 2017-06-29 19:35:49]

Description was changed from

==========
Fix duplicate logging of ActiveBrowserChanged action on views.

This was caused by redundant calls to BrowserList::SetLastActive,
one from BrowserFrame and another from BrowserView:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
BrowserList::SetLastActive()

and:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
views::Widget::OnNativeWidgetActivationChanged()
BrowserView::OnWidgetActivationChanged()
BrowserList::SetLastActive()

The change removes the call from BrowserFrame since it seems
redundant with the one in BrowserView which observes its widget
(via GetWidget()->AddObserver(this) call in InitViews()), so it
will always get the OnWidgetActivationChanged() notifications -
and it needs them for other things already.

Also fixes ChromeVisibilityObserverBrowserTest on Windows which
was fragile to the ordering of active/inactive events. It was also made
unnecessarily complex by https://codereview.chromium.org/2591783002
so this change actually reverts most of that CL by using the simpler
RunLoop::RunUntilIdle().

BUG=689553
TEST=Open two windows and then load chrome://user-actions. Switch
windows and observe only a single instance of ActiveBrowserChanged
logged per window switch.
==========

to

==========
Fix duplicate logging of ActiveBrowserChanged action on views.

This was caused by redundant calls to BrowserList::SetLastActive,
one from BrowserFrame and another from BrowserView:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
BrowserList::SetLastActive()

and:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
views::Widget::OnNativeWidgetActivationChanged()
BrowserView::OnWidgetActivationChanged()
BrowserList::SetLastActive()

The change removes the call from BrowserFrame since it seems
redundant with the one in BrowserView which observes its widget
(via GetWidget()->AddObserver(this) call in InitViews()), so it
will always get the OnWidgetActivationChanged() notifications -
and it needs them for other things already.

Also fixes ChromeVisibilityObserverBrowserTest on Windows which
was fragile to the ordering of active/inactive events (and just happened
to work due to the duplicate events). That test was also made unnecessarily
complex by https://codereview.chromium.org/2591783002 so this change
reverts most of that CL and simplifies things via RunLoop::RunUntilIdle().

BUG=689553
TEST=Open two windows and then load chrome://user-actions. Switch
windows and observe only a single instance of ActiveBrowserChanged
logged per window switch.
==========

[Alexei Svitkine (slow), 2017-06-29 19:36:11]

Description was changed from

==========
Fix duplicate logging of ActiveBrowserChanged action on views.

This was caused by redundant calls to BrowserList::SetLastActive,
one from BrowserFrame and another from BrowserView:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
BrowserList::SetLastActive()

and:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
views::Widget::OnNativeWidgetActivationChanged()
BrowserView::OnWidgetActivationChanged()
BrowserList::SetLastActive()

The change removes the call from BrowserFrame since it seems
redundant with the one in BrowserView which observes its widget
(via GetWidget()->AddObserver(this) call in InitViews()), so it
will always get the OnWidgetActivationChanged() notifications -
and it needs them for other things already.

Also fixes ChromeVisibilityObserverBrowserTest on Windows which
was fragile to the ordering of active/inactive events (and just happened
to work due to the duplicate events). That test was also made unnecessarily
complex by https://codereview.chromium.org/2591783002 so this change
reverts most of that CL and simplifies things via RunLoop::RunUntilIdle().

BUG=689553
TEST=Open two windows and then load chrome://user-actions. Switch
windows and observe only a single instance of ActiveBrowserChanged
logged per window switch.
==========

to

==========
Fix duplicate logging of ActiveBrowserChanged action on views.

This was caused by redundant calls to BrowserList::SetLastActive,
one from BrowserFrame and another from BrowserView:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
BrowserList::SetLastActive()

and:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
views::Widget::OnNativeWidgetActivationChanged()
BrowserView::OnWidgetActivationChanged()
BrowserList::SetLastActive()

The change removes the call from BrowserFrame since it seems
redundant with the one in BrowserView which observes its widget
(via GetWidget()->AddObserver(this) call in InitViews()), so it
will always get the OnWidgetActivationChanged() notifications -
and it needs them for other things already.

Also fixes ChromeVisibilityObserverBrowserTest on Windows which was
fragile to the ordering of active/inactive events (and just happened to
work due to the duplicate events). That test was also made unnecessarily
complex by https://codereview.chromium.org/2591783002 so this change
reverts most of that CL and simplifies things via RunLoop::RunUntilIdle().

BUG=689553
TEST=Open two windows and then load chrome://user-actions. Switch
windows and observe only a single instance of ActiveBrowserChanged
logged per window switch.
==========

[Alexei Svitkine (slow), 2017-06-29 20:18:30]

asvitkine@chromium.org changed reviewers:
+ pkasting@chromium.org, rkaplow@chromium.org

[Alexei Svitkine (slow), 2017-06-29 20:18:31]

Okay, this is ready for review again!

pkasting: please review chrome/browser/ui
rkaplow:  please review chrome/browser/metrics

[commit-bot: I haz the power, 2017-06-29 20:22:22]

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

[commit-bot: I haz the power, 2017-06-29 20:22:22]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2017-06-29 20:25:51]

LGTM

[rkaplow, 2017-06-30 13:56:10]

lgtm

[Alexei Svitkine (slow), 2017-06-30 14:15:44]

The CQ bit was checked by asvitkine@chromium.org

[commit-bot: I haz the power, 2017-06-30 14:15:51]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-30 14:20:47]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1498832144209250,
"parent_rev": "c65e5b86d2530a035bf17612b2e12109c942d878", "commit_rev":
"ba8e10dc8119f1bd7d2f365b2b3078f68aeb16cf"}

[commit-bot: I haz the power, 2017-06-30 14:21:10]

Description was changed from

==========
Fix duplicate logging of ActiveBrowserChanged action on views.

This was caused by redundant calls to BrowserList::SetLastActive,
one from BrowserFrame and another from BrowserView:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
BrowserList::SetLastActive()

and:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
views::Widget::OnNativeWidgetActivationChanged()
BrowserView::OnWidgetActivationChanged()
BrowserList::SetLastActive()

The change removes the call from BrowserFrame since it seems
redundant with the one in BrowserView which observes its widget
(via GetWidget()->AddObserver(this) call in InitViews()), so it
will always get the OnWidgetActivationChanged() notifications -
and it needs them for other things already.

Also fixes ChromeVisibilityObserverBrowserTest on Windows which was
fragile to the ordering of active/inactive events (and just happened to
work due to the duplicate events). That test was also made unnecessarily
complex by https://codereview.chromium.org/2591783002 so this change
reverts most of that CL and simplifies things via RunLoop::RunUntilIdle().

BUG=689553
TEST=Open two windows and then load chrome://user-actions. Switch
windows and observe only a single instance of ActiveBrowserChanged
logged per window switch.
==========

to

==========
Fix duplicate logging of ActiveBrowserChanged action on views.

This was caused by redundant calls to BrowserList::SetLastActive,
one from BrowserFrame and another from BrowserView:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
BrowserList::SetLastActive()

and:

views::DesktopNativeWidgetAura::HandleActivationChanged()
BrowserFrame::OnNativeWidgetActivationChanged()
views::Widget::OnNativeWidgetActivationChanged()
BrowserView::OnWidgetActivationChanged()
BrowserList::SetLastActive()

The change removes the call from BrowserFrame since it seems
redundant with the one in BrowserView which observes its widget
(via GetWidget()->AddObserver(this) call in InitViews()), so it
will always get the OnWidgetActivationChanged() notifications -
and it needs them for other things already.

Also fixes ChromeVisibilityObserverBrowserTest on Windows which was
fragile to the ordering of active/inactive events (and just happened to
work due to the duplicate events). That test was also made unnecessarily
complex by https://codereview.chromium.org/2591783002 so this change
reverts most of that CL and simplifies things via RunLoop::RunUntilIdle().

BUG=689553
TEST=Open two windows and then load chrome://user-actions. Switch
windows and observe only a single instance of ActiveBrowserChanged
logged per window switch.

Review-Url: https://codereview.chromium.org/2939943004
Cr-Commit-Position: refs/heads/master@{#483695}
Committed:
https://chromium.googlesource.com/chromium/src/+/ba8e10dc8119f1bd7d2f365b2b30...
==========

[commit-bot: I haz the power, 2017-06-30 14:21:12]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/ba8e10dc8119f1bd7d2f365b2b30...