[views-mus] Prevent creating a native OzonePlatform in mus tests.

[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform
owns. This ensures that AuraTestHelper doesn't create a local
implementation using the same detection method that Env uses to flip
into aura-mus mode.

BUG=705037, 710195, 710939
Review-Url: https://codereview.chromium.org/2807833002
Cr-Commit-Position: refs/heads/master@{#464147}
Committed: https://chromium.googlesource.com/chromium/src/+/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3

[Elliot Glaysher, 2017-04-07 22:54:30]

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

[commit-bot: I haz the power, 2017-04-07 22:55:04]

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, 2017-04-07 23:04:29]

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

[commit-bot: I haz the power, 2017-04-07 23:04:30]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Elliot Glaysher, 2017-04-07 23:05:36]

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

[commit-bot: I haz the power, 2017-04-07 23:05:58]

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

[Elliot Glaysher, 2017-04-07 23:07:31]

The CQ bit was unchecked by erg@chromium.org

[Elliot Glaysher, 2017-04-07 23:08:07]

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

[commit-bot: I haz the power, 2017-04-07 23:09:09]

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, 2017-04-07 23:50:11]

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

[commit-bot: I haz the power, 2017-04-07 23:50:11]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-04-10 22:02:52]

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

[commit-bot: I haz the power, 2017-04-10 22:04:23]

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, 2017-04-10 23:15:00]

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

[commit-bot: I haz the power, 2017-04-10 23:15:00]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-04-10 23:36:12]

Description was changed from

==========
[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform owns.

BUG=705037
==========

to

==========
[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform owns.

BUG=705037, 710195
==========

[Elliot Glaysher, 2017-04-10 23:44:31]

Description was changed from

==========
[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform owns.

BUG=705037, 710195
==========

to

==========
[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform owns. This
ensures that AuraTestHelper doesn't create a local implementation using the same
detection method that Env uses to flip into aura-mus mode.

BUG=705037, 710195
==========

[Elliot Glaysher, 2017-04-10 23:44:31]

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

[Elliot Glaysher, 2017-04-10 23:44:54]

Description was changed from

==========
[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform owns. This
ensures that AuraTestHelper doesn't create a local implementation using the same
detection method that Env uses to flip into aura-mus mode.

BUG=705037, 710195
==========

to

==========
[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform
owns. This ensures that AuraTestHelper doesn't create a local
implementation using the same detection method that Env uses to flip
into aura-mus mode.

BUG=705037, 710195
==========

[Elliot Glaysher, 2017-04-10 23:46:14]

We mostly work when we ensure that we aren't using the local aura method. I've
only had to disable one test which didn't directly relied on AuraTestBase in
views.

https://codereview.chromium.org/2807833002/diff/40001/ui/views/BUILD.gn
File ui/views/BUILD.gn (right):

https://codereview.chromium.org/2807833002/diff/40001/ui/views/BUILD.gn#newco...
ui/views/BUILD.gn:1022: # Some of the tests need drag-drop support.
crbug.com/614037
git cl format is preventing me from separating these into sections.

https://codereview.chromium.org/2807833002/diff/40001/ui/views/test/widget_te...
File ui/views/test/widget_test_unittest.cc (right):

https://codereview.chromium.org/2807833002/diff/40001/ui/views/test/widget_te...
ui/views/test/widget_test_unittest.cc:50: // aura-mus client.
http://crbug.com/663781.
This is an example of a test which was only succeeding because it went to the
default non-mus local aura host, which provided enough data for GetContext() to
succeed here. (And now that it's disabled in mus builds, fails.)

[sky, 2017-04-10 23:53:26]

https://codereview.chromium.org/2807833002/diff/40001/ui/aura/test/aura_test_...
File ui/aura/test/aura_test_helper.cc (right):

https://codereview.chromium.org/2807833002/diff/40001/ui/aura/test/aura_test_...
ui/aura/test/aura_test_helper.cc:52: "primordial-pipe-token");
Use kPrimordialPipeToken?

https://codereview.chromium.org/2807833002/diff/40001/ui/aura/test/aura_test_...
ui/aura/test/aura_test_helper.cc:136: // TODO(sky): creating the screen and host
should not happen for mus.
You can remove this todo now.

https://codereview.chromium.org/2807833002/diff/40001/ui/views/BUILD.gn
File ui/views/BUILD.gn (right):

https://codereview.chromium.org/2807833002/diff/40001/ui/views/BUILD.gn#newco...
ui/views/BUILD.gn:1019: # These tests currently use AuraTestBase, but then try
to use views
Please file a bug on these so we can update them.

https://codereview.chromium.org/2807833002/diff/40001/ui/views/mus/views_mus_...
File ui/views/mus/views_mus_test_suite.cc (right):

https://codereview.chromium.org/2807833002/diff/40001/ui/views/mus/views_mus_...
ui/views/mus/views_mus_test_suite.cc:261:
command_line->AppendSwitch("primordial-pipe-token");
This is mildly confusing and could easily confuse other things that look for
this switch. How about adding a private static bool indicating if in mus that
this code sets to true? AuraTestBase would explicitly friend ViewsMusTestSuite
as we don't want individual tests changing the state.

[Elliot Glaysher, 2017-04-11 21:52:48]

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

[commit-bot: I haz the power, 2017-04-11 21:53:47]

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, 2017-04-11 23:13:44]

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

[commit-bot: I haz the power, 2017-04-11 23:13:44]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-04-11 23:55:41]

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

[commit-bot: I haz the power, 2017-04-11 23:56:18]

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, 2017-04-12 01:16:12]

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

[commit-bot: I haz the power, 2017-04-12 01:16:13]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-04-12 18:08:01]

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

[commit-bot: I haz the power, 2017-04-12 18:08:59]

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

[Elliot Glaysher, 2017-04-12 18:09:16]

https://codereview.chromium.org/2807833002/diff/40001/ui/aura/test/aura_test_...
File ui/aura/test/aura_test_helper.cc (right):

https://codereview.chromium.org/2807833002/diff/40001/ui/aura/test/aura_test_...
ui/aura/test/aura_test_helper.cc:52: "primordial-pipe-token");
On 2017/04/10 23:53:26, sky wrote:
> Use kPrimordialPipeToken?

Removed.

https://codereview.chromium.org/2807833002/diff/40001/ui/aura/test/aura_test_...
ui/aura/test/aura_test_helper.cc:136: // TODO(sky): creating the screen and host
should not happen for mus.
On 2017/04/10 23:53:26, sky wrote:
> You can remove this todo now.

Done.

https://codereview.chromium.org/2807833002/diff/40001/ui/views/mus/views_mus_...
File ui/views/mus/views_mus_test_suite.cc (right):

https://codereview.chromium.org/2807833002/diff/40001/ui/views/mus/views_mus_...
ui/views/mus/views_mus_test_suite.cc:261:
command_line->AppendSwitch("primordial-pipe-token");
On 2017/04/10 23:53:26, sky wrote:
> This is mildly confusing and could easily confuse other things that look for
> this switch. How about adding a private static bool indicating if in mus that
> this code sets to true? AuraTestBase would explicitly friend ViewsMusTestSuite
> as we don't want individual tests changing the state.

I've changed how mus works so that it no longer detects based on a command line
flag anywhere, and instead uses the Env::mode(). This requires some small
changes to how views tests work.

[commit-bot: I haz the power, 2017-04-12 19:21:21]

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

[commit-bot: I haz the power, 2017-04-12 19:21:22]

Dry run: This issue passed the CQ dry run.

[sky, 2017-04-12 19:38:15]

LGTM

https://codereview.chromium.org/2807833002/diff/100001/ui/base/dragdrop/os_ex...
File ui/base/dragdrop/os_exchange_data_provider_factory.h (right):

https://codereview.chromium.org/2807833002/diff/100001/ui/base/dragdrop/os_ex...
ui/base/dragdrop/os_exchange_data_provider_factory.h:29: static Factory*
TakeFactory();
Can this return a std::unique_ptr?

[Elliot Glaysher, 2017-04-12 19:52:26]

https://codereview.chromium.org/2807833002/diff/100001/ui/base/dragdrop/os_ex...
File ui/base/dragdrop/os_exchange_data_provider_factory.h (right):

https://codereview.chromium.org/2807833002/diff/100001/ui/base/dragdrop/os_ex...
ui/base/dragdrop/os_exchange_data_provider_factory.h:29: static Factory*
TakeFactory();
On 2017/04/12 19:38:15, sky wrote:
> Can this return a std::unique_ptr?

Not easily without changing the ownership the Factory, which in all cases I know
about is an object with its own ownership semantics.

[Elliot Glaysher, 2017-04-12 19:52:33]

The CQ bit was checked by erg@chromium.org

[commit-bot: I haz the power, 2017-04-12 19:53:36]

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

[sky, 2017-04-12 19:53:40]

Fair enough.

On Wed, Apr 12, 2017 at 12:52 PM, <erg@chromium.org> wrote:

>
> https://codereview.chromium.org/2807833002/diff/100001/ui/
> base/dragdrop/os_exchange_data_provider_factory.h
> File ui/base/dragdrop/os_exchange_data_provider_factory.h (right):
>
> https://codereview.chromium.org/2807833002/diff/100001/ui/
> base/dragdrop/os_exchange_data_provider_factory.h#newcode29
> ui/base/dragdrop/os_exchange_data_provider_factory.h:29: static Factory*
> TakeFactory();
> On 2017/04/12 19:38:15, sky wrote:
> > Can this return a std::unique_ptr?
>
> Not easily without changing the ownership the Factory, which in all
> cases I know about is an object with its own ownership semantics.
>
> https://codereview.chromium.org/2807833002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Elliot Glaysher, 2017-04-12 20:06:55]

Description was changed from

==========
[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform
owns. This ensures that AuraTestHelper doesn't create a local
implementation using the same detection method that Env uses to flip
into aura-mus mode.

BUG=705037, 710195
==========

to

==========
[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform
owns. This ensures that AuraTestHelper doesn't create a local
implementation using the same detection method that Env uses to flip
into aura-mus mode.

BUG=705037, 710195, 710939
==========

[commit-bot: I haz the power, 2017-04-12 20:54:58]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1492026753861240,
"parent_rev": "e0c1afa12d5cc615038e3a88c65047814607bb99", "commit_rev":
"ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3"}

[commit-bot: I haz the power, 2017-04-12 20:55:58]

Description was changed from

==========
[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform
owns. This ensures that AuraTestHelper doesn't create a local
implementation using the same detection method that Env uses to flip
into aura-mus mode.

BUG=705037, 710195, 710939
==========

to

==========
[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform
owns. This ensures that AuraTestHelper doesn't create a local
implementation using the same detection method that Env uses to flip
into aura-mus mode.

BUG=705037, 710195, 710939

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

[commit-bot: I haz the power, 2017-04-12 20:55:59]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/ac2e0aeea11a2641dfca4d1b192b...