views/mus: Run some more tests from views_unittests in mus.

views/mus: Run some more tests from views_unittests in mus.

BUG=577274
Committed: https://crrev.com/3a5132a428812c397f17b99cfe9101863f8a851c
Cr-Commit-Position: refs/heads/master@{#398705}

[sadrul, 2016-05-29 05:36:46]

Description was changed from

==========
views/mus: Run some more tests from views_unittests in mus.

...

BUG=
==========

to

==========
views/mus: Run some more tests from views_unittests in mus.

...

BUG=577274
==========

[sadrul, 2016-06-07 23:37:03]

Description was changed from

==========
views/mus: Run some more tests from views_unittests in mus.

...

BUG=577274
==========

to

==========
views/mus: Run some more tests from views_unittests in mus.

BUG=577274
==========

[sadrul, 2016-06-07 23:37:55]

sadrul@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2016-06-08 04:25:02]

https://codereview.chromium.org/2010083002/diff/140001/ui/views/focus/focus_m...
File ui/views/focus/focus_manager_unittest.cc (right):

https://codereview.chromium.org/2010083002/diff/140001/ui/views/focus/focus_m...
ui/views/focus/focus_manager_unittest.cc:126: // SimulateNativeActive() doesn't
work correctly in mus.
I recommend NOTIMPLEMENTEDs and filing bugs for all things that should work.
Otherwise we're going to forget them.

https://codereview.chromium.org/2010083002/diff/140001/ui/views/mus/native_wi...
File ui/views/mus/native_widget_mus.cc (right):

https://codereview.chromium.org/2010083002/diff/140001/ui/views/mus/native_wi...
ui/views/mus/native_widget_mus.cc:827: content_->SetBounds(bounds);
This makes me nervous.
Setting the bounds of the content assumes the SetSharedProperty above is going
to take, which isn't guaranteed. And as it's just a property we don't have a way
of knowing if the property is ignored, so that content_ is then out of sync.
Also, the coordinates are not necessarily the same.

It seems like we should set the property on the window, and set the bounds of
window_. If the call to set the window_ bounds fails then we'll revert
correctly.

[sadrul, 2016-06-08 14:57:42]

https://codereview.chromium.org/2010083002/diff/140001/ui/views/focus/focus_m...
File ui/views/focus/focus_manager_unittest.cc (right):

https://codereview.chromium.org/2010083002/diff/140001/ui/views/focus/focus_m...
ui/views/focus/focus_manager_unittest.cc:126: // SimulateNativeActive() doesn't
work correctly in mus.
On 2016/06/08 04:25:02, sky wrote:
> I recommend NOTIMPLEMENTEDs and filing bugs for all things that should work.
> Otherwise we're going to forget them.

Ended up fixing this particular test in
https://codereview.chromium.org/2049723002/

I have avoided adding NOTIMPLEMENTED()s in other places, since they add too much
noise in the tests. But I believe I have filed bugs tracking the rest of the
things that do not currently work in mus.

https://codereview.chromium.org/2010083002/diff/140001/ui/views/mus/native_wi...
File ui/views/mus/native_widget_mus.cc (right):

https://codereview.chromium.org/2010083002/diff/140001/ui/views/mus/native_wi...
ui/views/mus/native_widget_mus.cc:827: content_->SetBounds(bounds);
On 2016/06/08 04:25:02, sky wrote:
> This makes me nervous.
> Setting the bounds of the content assumes the SetSharedProperty above is going
> to take, which isn't guaranteed. And as it's just a property we don't have a
way
> of knowing if the property is ignored, so that content_ is then out of sync.
> Also, the coordinates are not necessarily the same.
> 
> It seems like we should set the property on the window, and set the bounds of
> window_. If the call to set the window_ bounds fails then we'll revert
> correctly.

Done.

[sky, 2016-06-08 17:31:46]

LGTM

[sadrul, 2016-06-08 17:55:13]

sadrul@chromium.org changed reviewers:
+ ben@chromium.org

[sadrul, 2016-06-08 17:55:13]

+ben@ for the change in toplevel .gn

[Ben Goodger (Google), 2016-06-08 22:12:50]

lgtm

[sadrul, 2016-06-08 22:22:07]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2016-06-08 22:22:07]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2010083002/#ps200001
(title: ".")

[commit-bot: I haz the power, 2016-06-08 22:22:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2010083002/200001

[commit-bot: I haz the power, 2016-06-08 22:29:29]

Description was changed from

==========
views/mus: Run some more tests from views_unittests in mus.

BUG=577274
==========

to

==========
views/mus: Run some more tests from views_unittests in mus.

BUG=577274
==========

[commit-bot: I haz the power, 2016-06-08 22:29:30]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-06-08 22:32:25]

Description was changed from

==========
views/mus: Run some more tests from views_unittests in mus.

BUG=577274
==========

to

==========
views/mus: Run some more tests from views_unittests in mus.

BUG=577274

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

[commit-bot: I haz the power, 2016-06-08 22:32:27]

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