Adds ability to suspend notifying observers when display list is changed

Adds ability to suspend notifying observers when display list is changed

This is needed so that the add can happen, some processing, then
notify observers. I can't do the processing first as the processing
may end up querying for the displays.

BUG=none
TEST=none
R=jamescook@chromium.org

Committed: https://crrev.com/3e56abe29aec0e9046a3546f58e1551785eeff5d
Cr-Commit-Position: refs/heads/master@{#421904}

[sky, 2016-09-28 23:30:15]

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

[commit-bot: I haz the power, 2016-09-28 23:30:33]

Dry run: 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, 2016-09-29 00:24:06]

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

[commit-bot: I haz the power, 2016-09-29 00:24:07]

Dry run: This issue passed the CQ dry run.

[James Cook, 2016-09-29 04:15:05]

Hmm. This feels complex to me. DisplayListObserverLock feels like it should
queue observer notifications, but doesn't do it completely
(OnDisplayMetricsChanged notifications are discarded, Add only happens once), so
then it needs a bunch of docs about what it does and doesn't do.

What about something like this?

1. DisplayListObserverLock just turns off all observer notifications, period.
They aren't queued.
2. DisplayList provides a getter for the observer list.
3. Ash code explicitly triggers the OnDisplayAdded() one you need by walking the
exposed observer list.

Then the code inside DisplayList is probably simpler, the reader won't have to
reason about how/why OnDisplayMetricsChanged doesn't fire, and the notifications
occur in an explicit piece of code rather than as a side effect of a lock object
going out of scope.

WDYT?

https://codereview.chromium.org/2378923002/diff/20001/ash/mus/window_manager.cc
File ash/mus/window_manager.cc (right):

https://codereview.chromium.org/2378923002/diff/20001/ash/mus/window_manager....
ash/mus/window_manager.cc:142: // notifying DisplayObservers. Ash code assumes
when this happens their is
nit: their -> there

https://codereview.chromium.org/2378923002/diff/20001/ash/mus/window_manager....
ash/mus/window_manager.cc:149: const bool was_displays_empty =
screen_->display_list()->displays().empty();
optional: |is_first_display| ?

https://codereview.chromium.org/2378923002/diff/20001/ash/mus/window_manager....
ash/mus/window_manager.cc:150: // TODO(sky): should be passed whether display is
primary.
Is this todo still relevant?

https://codereview.chromium.org/2378923002/diff/20001/ui/display/display_list.h
File ui/display/display_list.h (right):

https://codereview.chromium.org/2378923002/diff/20001/ui/display/display_list...
ui/display/display_list.h:24: class DISPLAY_EXPORT DisplayListObserverLock {
I like lock objects.

https://codereview.chromium.org/2378923002/diff/20001/ui/display/display_list...
ui/display/display_list.h:68: // NOTE: this only supports suspsending followed
by a single add and any
nit: suspending

https://codereview.chromium.org/2378923002/diff/20001/ui/display/display_list...
ui/display/display_list.h:90: size_t original_display_count = 0u;
Unused?

[sky, 2016-09-29 18:04:44]

I was hoping to make sure nothing additional happened while suspended, but I
definitely like simpler. Updated.

https://codereview.chromium.org/2378923002/diff/20001/ash/mus/window_manager.cc
File ash/mus/window_manager.cc (right):

https://codereview.chromium.org/2378923002/diff/20001/ash/mus/window_manager....
ash/mus/window_manager.cc:142: // notifying DisplayObservers. Ash code assumes
when this happens their is
On 2016/09/29 04:15:05, James Cook wrote:
> nit: their -> there

Done.

https://codereview.chromium.org/2378923002/diff/20001/ash/mus/window_manager....
ash/mus/window_manager.cc:149: const bool was_displays_empty =
screen_->display_list()->displays().empty();
On 2016/09/29 04:15:05, James Cook wrote:
> optional: |is_first_display| ?

Done.

https://codereview.chromium.org/2378923002/diff/20001/ash/mus/window_manager....
ash/mus/window_manager.cc:150: // TODO(sky): should be passed whether display is
primary.
On 2016/09/29 04:15:05, James Cook wrote:
> Is this todo still relevant?

I *think* so, but I'm not 100% positive. I'm not sure if a new display can be
added and made primary in one distinct step.

[sky, 2016-09-29 18:04:51]

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

[commit-bot: I haz the power, 2016-09-29 18:05:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2016-09-29 19:26:44]

Nice. LGTM

[sky, 2016-09-29 19:29:27]

The CQ bit was unchecked by sky@chromium.org

[sky, 2016-09-29 19:29:32]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2016-09-29 19:29:56]

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, 2016-09-29 19:38:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-29 19:41:46]

Description was changed from

==========
Adds ability to suspend notifying observers when display list is changed

This is needed so that the add can happen, some processing, then
notify observers. I can't do the processing first as the processing
may end up querying for the displays.

BUG=none
TEST=none
R=jamescook@chromium.org
==========

to

==========
Adds ability to suspend notifying observers when display list is changed

This is needed so that the add can happen, some processing, then
notify observers. I can't do the processing first as the processing
may end up querying for the displays.

BUG=none
TEST=none
R=jamescook@chromium.org

Committed: https://crrev.com/3e56abe29aec0e9046a3546f58e1551785eeff5d
Cr-Commit-Position: refs/heads/master@{#421904}
==========

[commit-bot: I haz the power, 2016-09-29 19:41:47]

Patchset 3 (id:??) landed as
https://crrev.com/3e56abe29aec0e9046a3546f58e1551785eeff5d
Cr-Commit-Position: refs/heads/master@{#421904}