Move ViewHostMsg_CreateWindow to mojom

Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some transitional public
   constants have been added to satisfy existing references to
   the old enum values. This will be removed in future CLs.
 - Adds a mojom definition for blink::mojom::WindowFeatures,
   corresponding to blink::WebWindowFeatures. A typemap is
   added between these types and is shared by both Blink and
   Chromium bindings configurations.
 - Adds a blink::mojom::Referrer struct analogous to
   content::Renderer. Chromium bindings typemap this new mojom
   struct to content::Referrer.
 - Adds a mojom blink::mojom::ReferrerPolicy enum with a typemap
   to blink::WebReferrerPolicy, shared by Chromium and Blink
   bindings. This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/5c478a7b04b55630c5c491d4d1d2863e965cfa32
Cr-Commit-Position: refs/heads/master@{#421674}

[Ken Rockot(use gerrit already), 2016-09-21 22:53:08]

Description was changed from

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType is an alias for the new
   content::mojom::WindowContainerType. Some public constants have
   been added to satisfy existing references to the old enum values.
 - Adds a mojom definition for WindowFeatures corresponding to
   WebWindowFeatures. A typemap is added between these types and is
   shared by both Blink and Chromium bindings configurations.
 - Adds mojo::StringTraits<blink::WebString> and
   mojo::ArrayTraits<blink::WebVector> to support WebWindowFeatures
   typemap.
 - content::Referrer now has a mojom equivalent in third_party/WebKit
   called blink::mojom::Referrer. Chromium bindings typemap this
   to content::Referrer.
 - blink::mojom::ReferrerPolicy has a typemap to
   blink::WebReferrerPolicy shared by Chromium and Blink bindings.

BUG=612500
==========

to

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType is an alias for the new
   content::mojom::WindowContainerType. Some public constants have
   been added to satisfy existing references to the old enum values.
 - Adds a mojom definition for WindowFeatures corresponding to
   WebWindowFeatures. A typemap is added between these types and is
   shared by both Blink and Chromium bindings configurations.
 - Adds mojo::StringTraits<blink::WebString> and
   mojo::ArrayTraits<blink::WebVector> to support WebWindowFeatures
   typemap.
 - content::Referrer now has a mojom equivalent in third_party/WebKit
   called blink::mojom::Referrer. Chromium bindings typemap this
   to content::Referrer.
 - blink::mojom::ReferrerPolicy has a typemap to
   blink::WebReferrerPolicy shared by Chromium and Blink bindings.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Ken Rockot(use gerrit already), 2016-09-22 00:04:34]

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

[commit-bot: I haz the power, 2016-09-22 00:04:59]

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

[Ken Rockot(use gerrit already), 2016-09-22 00:07:27]

Description was changed from

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType is an alias for the new
   content::mojom::WindowContainerType. Some public constants have
   been added to satisfy existing references to the old enum values.
 - Adds a mojom definition for WindowFeatures corresponding to
   WebWindowFeatures. A typemap is added between these types and is
   shared by both Blink and Chromium bindings configurations.
 - Adds mojo::StringTraits<blink::WebString> and
   mojo::ArrayTraits<blink::WebVector> to support WebWindowFeatures
   typemap.
 - content::Referrer now has a mojom equivalent in third_party/WebKit
   called blink::mojom::Referrer. Chromium bindings typemap this
   to content::Referrer.
 - blink::mojom::ReferrerPolicy has a typemap to
   blink::WebReferrerPolicy shared by Chromium and Blink bindings.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some public constants have
   been added to satisfy existing references to the old enum values.
 - Adds a mojom definition for WindowFeatures corresponding to
   WebWindowFeatures. A typemap is added between these types and is
   shared by both Blink and Chromium bindings configurations.
 - Adds mojo::StringTraits<blink::WebString> and
   mojo::ArrayTraits<blink::WebVector> to support WebWindowFeatures
   typemap.
 - content::Referrer now has a mojom equivalent in third_party/WebKit
   called blink::mojom::Referrer. Chromium bindings typemap this
   to content::Referrer.
 - blink::mojom::ReferrerPolicy has a typemap to
   blink::WebReferrerPolicy shared by Chromium and Blink bindings.
   This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-09-22 00:09:18]

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

[commit-bot: I haz the power, 2016-09-22 00:09:19]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Ken Rockot(use gerrit already), 2016-09-22 00:11:49]

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

[Ken Rockot(use gerrit already), 2016-09-22 00:12:02]

Patchset #1 (id:1) has been deleted

[Ken Rockot(use gerrit already), 2016-09-22 00:12:08]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-09-22 00:12:10]

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-22 00:40:55]

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

[commit-bot: I haz the power, 2016-09-22 00:40:57]

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

[Ken Rockot(use gerrit already), 2016-09-22 15:49:09]

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

[commit-bot: I haz the power, 2016-09-22 15:49:21]

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-22 16:13:18]

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

[commit-bot: I haz the power, 2016-09-22 16:13:19]

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

[Ken Rockot(use gerrit already), 2016-09-22 16:29:36]

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

[commit-bot: I haz the power, 2016-09-22 16:29:52]

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

[Ken Rockot(use gerrit already), 2016-09-22 16:31:02]

Patchset #1 (id:40001) has been deleted

[Ken Rockot(use gerrit already), 2016-09-22 16:31:10]

Patchset #1 (id:60001) has been deleted

[Ken Rockot(use gerrit already), 2016-09-22 16:35:06]

Description was changed from

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some public constants have
   been added to satisfy existing references to the old enum values.
 - Adds a mojom definition for WindowFeatures corresponding to
   WebWindowFeatures. A typemap is added between these types and is
   shared by both Blink and Chromium bindings configurations.
 - Adds mojo::StringTraits<blink::WebString> and
   mojo::ArrayTraits<blink::WebVector> to support WebWindowFeatures
   typemap.
 - content::Referrer now has a mojom equivalent in third_party/WebKit
   called blink::mojom::Referrer. Chromium bindings typemap this
   to content::Referrer.
 - blink::mojom::ReferrerPolicy has a typemap to
   blink::WebReferrerPolicy shared by Chromium and Blink bindings.
   This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some transitional public
   constants have been added to satisfy existing references to
   the old enum values. This will be removed in future CLs.
 - Adds a mojom definition for blink::mojom::WindowFeatures,
   corresponding to blink::WebWindowFeatures. A typemap is
   added between these types and is shared by both Blink and
   Chromium bindings configurations.
 - Adds Mojo string and array traits for WebString and WebVector
   to support public blink mojom types WindowFeatures.
 - Adds a blink::mojom::Referrer struct analogous to
   content::Renderer. Chromium bindings typemap this new mojom
   struct to content::Referrer.
 - Adds a mojom blink::mojom::ReferrerPolicy enum with a typemap
   to blink::WebReferrerPolicy, shared by Chromium and Blink
   bindings. This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-09-22 17:09:58]

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

[commit-bot: I haz the power, 2016-09-22 17:09:59]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Ken Rockot(use gerrit already), 2016-09-22 19:30:41]

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

[commit-bot: I haz the power, 2016-09-22 19:31:12]

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-22 20:34:48]

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

[commit-bot: I haz the power, 2016-09-22 20:34:49]

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

[Ken Rockot(use gerrit already), 2016-09-22 21:32:27]

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

[commit-bot: I haz the power, 2016-09-22 21:33:22]

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

[Ken Rockot(use gerrit already), 2016-09-22 21:42:29]

Patchset #1 (id:80001) has been deleted

[Ken Rockot(use gerrit already), 2016-09-22 21:42:37]

Patchset #1 (id:100001) has been deleted

[commit-bot: I haz the power, 2016-09-22 22:45:44]

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

[commit-bot: I haz the power, 2016-09-22 22:45:46]

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_...)

[Ken Rockot(use gerrit already), 2016-09-22 22:54:24]

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

[commit-bot: I haz the power, 2016-09-22 22:54:41]

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-22 22:56:24]

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

[commit-bot: I haz the power, 2016-09-22 22:56:25]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ken Rockot(use gerrit already), 2016-09-23 17:57:01]

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

[commit-bot: I haz the power, 2016-09-23 17:58:11]

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-23 18:51:33]

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

[commit-bot: I haz the power, 2016-09-23 18:51:34]

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

[Ken Rockot(use gerrit already), 2016-09-23 23:29:38]

Patchset #1 (id:120001) has been deleted

[Ken Rockot(use gerrit already), 2016-09-23 23:29:43]

Patchset #1 (id:140001) has been deleted

[Ken Rockot(use gerrit already), 2016-09-23 23:29:52]

Patchset #1 (id:160001) has been deleted

[Ken Rockot(use gerrit already), 2016-09-23 23:29:54]

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

[Ken Rockot(use gerrit already), 2016-09-23 23:30:05]

Patchset #1 (id:160001) has been deleted

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

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-24 00:50:04]

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

[commit-bot: I haz the power, 2016-09-24 00:50:05]

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

[Ken Rockot(use gerrit already), 2016-09-26 18:27:41]

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

[commit-bot: I haz the power, 2016-09-26 18:28:14]

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

[Ken Rockot(use gerrit already), 2016-09-26 19:07:34]

Description was changed from

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some transitional public
   constants have been added to satisfy existing references to
   the old enum values. This will be removed in future CLs.
 - Adds a mojom definition for blink::mojom::WindowFeatures,
   corresponding to blink::WebWindowFeatures. A typemap is
   added between these types and is shared by both Blink and
   Chromium bindings configurations.
 - Adds Mojo string and array traits for WebString and WebVector
   to support public blink mojom types WindowFeatures.
 - Adds a blink::mojom::Referrer struct analogous to
   content::Renderer. Chromium bindings typemap this new mojom
   struct to content::Referrer.
 - Adds a mojom blink::mojom::ReferrerPolicy enum with a typemap
   to blink::WebReferrerPolicy, shared by Chromium and Blink
   bindings. This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some transitional public
   constants have been added to satisfy existing references to
   the old enum values. This will be removed in future CLs.
 - Adds a mojom definition for blink::mojom::WindowFeatures,
   corresponding to blink::WebWindowFeatures. A typemap is
   added between these types and is shared by both Blink and
   Chromium bindings configurations.
 - Adds a blink::mojom::Referrer struct analogous to
   content::Renderer. Chromium bindings typemap this new mojom
   struct to content::Referrer.
 - Adds a mojom blink::mojom::ReferrerPolicy enum with a typemap
   to blink::WebReferrerPolicy, shared by Chromium and Blink
   bindings. This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-09-26 19:58:33]

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

[commit-bot: I haz the power, 2016-09-26 19:58:34]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Ken Rockot(use gerrit already), 2016-09-27 16:30:17]

rockot@chromium.org changed reviewers:
+ jam@chromium.org

[nasko, 2016-09-27 16:55:41]

nasko@chromium.org changed reviewers:
+ nasko@chromium.org, nick@chromium.org

[nasko, 2016-09-27 16:55:42]

Adding nick@, as he has done a lot of work for mojoifying this IPC.

[nasko, 2016-09-27 16:55:55]

Description was changed from

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some transitional public
   constants have been added to satisfy existing references to
   the old enum values. This will be removed in future CLs.
 - Adds a mojom definition for blink::mojom::WindowFeatures,
   corresponding to blink::WebWindowFeatures. A typemap is
   added between these types and is shared by both Blink and
   Chromium bindings configurations.
 - Adds a blink::mojom::Referrer struct analogous to
   content::Renderer. Chromium bindings typemap this new mojom
   struct to content::Referrer.
 - Adds a mojom blink::mojom::ReferrerPolicy enum with a typemap
   to blink::WebReferrerPolicy, shared by Chromium and Blink
   bindings. This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some transitional public
   constants have been added to satisfy existing references to
   the old enum values. This will be removed in future CLs.
 - Adds a mojom definition for blink::mojom::WindowFeatures,
   corresponding to blink::WebWindowFeatures. A typemap is
   added between these types and is shared by both Blink and
   Chromium bindings configurations.
 - Adds a blink::mojom::Referrer struct analogous to
   content::Renderer. Chromium bindings typemap this new mojom
   struct to content::Referrer.
 - Adds a mojom blink::mojom::ReferrerPolicy enum with a typemap
   to blink::WebReferrerPolicy, shared by Chromium and Blink
   bindings. This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[nasko, 2016-09-27 16:55:55]

nasko@chromium.org changed reviewers:
- nasko@chromium.org

[jam, 2016-09-27 19:18:41]

lgtm

[Ken Rockot(use gerrit already), 2016-09-27 19:29:43]

rockot@chromium.org changed reviewers:
+ dcheng@chromium.org

[Ken Rockot(use gerrit already), 2016-09-27 19:29:44]

+dcheng - I notice you're in third_party/WebKit/public OWNERS. Could you please
review for that and security in general?

[ncarter (slow), 2016-09-27 20:32:42]

https://codereview.chromium.org/2363573002/diff/200001/content/browser/render...
File content/browser/renderer_host/render_message_filter.cc (right):

https://codereview.chromium.org/2363573002/diff/200001/content/browser/render...
content/browser/renderer_host/render_message_filter.cc:301:
callback.Run(std::move(reply));
Can we now guarantee that the task posted to the UI thread by |CreateNewWindow|
will run before any UI-thread destined mojo message that may be sent from the
child process after the reply message is received?

https://codereview.chromium.org/2363573002/diff/200001/content/common/render_...
File content/common/render_message_filter.mojom (right):

https://codereview.chromium.org/2363573002/diff/200001/content/common/render_...
content/common/render_message_filter.mojom:58: blink.mojom.WindowFeatures
features;
I double checked, and removing |additional_features| seems okay.

https://codereview.chromium.org/2363573002/diff/200001/content/common/render_...
content/common/render_message_filter.mojom:67: int32 main_frame_route_id;
Long-term, do you think we'll eventually be returning FrameHost mojom interfaces
here (and other places where we mint routing ids)?

https://codereview.chromium.org/2363573002/diff/200001/content/public/test/mo...
File content/public/test/mock_render_thread.cc (right):

https://codereview.chromium.org/2363573002/diff/200001/content/public/test/mo...
content/public/test/mock_render_thread.cc:50: // Note that
?

https://codereview.chromium.org/2363573002/diff/200001/content/public/test/mo...
File content/public/test/mock_render_thread.h (right):

https://codereview.chromium.org/2363573002/diff/200001/content/public/test/mo...
content/public/test/mock_render_thread.h:45: // ViewMsg_Close : async, send to
the Widget.
Let's redo this class comment so that it better describes why this class exists.
Something like this:

// This class provides a mock implementation of RenderThread, which is not
// actually backed by a channel to a browser process. Only a few IPC messages
// are handled, just enough allow top-level objects (e.g. frames and widgets)
// to be created.
//
// This class is designed for inheritance: embedders of content may subclass
// it to handle additional IPCs.

https://codereview.chromium.org/2363573002/diff/200001/content/renderer/rende...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/2363573002/diff/200001/content/renderer/rende...
content/renderer/render_view_impl.cc:1556:
RenderThreadImpl::current_render_message_filter()->CreateNewWindow(
Why use this global and getter? Is it just about avoiding exposing it on
RenderThread (which is content/public)?

It seems weird to call into RenderThreadImpl:: to get a value that's owned by
the MockRenderThread. I'm not totally against this, but curious if there are any
alternatives worth considering.

[Ken Rockot(use gerrit already), 2016-09-27 21:05:42]

Thanks for looking

https://codereview.chromium.org/2363573002/diff/200001/content/browser/render...
File content/browser/renderer_host/render_message_filter.cc (right):

https://codereview.chromium.org/2363573002/diff/200001/content/browser/render...
content/browser/renderer_host/render_message_filter.cc:301:
callback.Run(std::move(reply));
On 2016/09/27 at 20:32:41, ncarter wrote:
> Can we now guarantee that the task posted to the UI thread by
|CreateNewWindow| will run before any UI-thread destined mojo message that may
be sent from the child process after the reply message is received?

Yes. Any interfaces (such as mojom::RenderMessageFilter) which are associated
with the IPC::Channel  exhibit perfect ordering with respect to Channel on the
IO thread and ChannelProxy on the main thread.

To put it another way, this conversion has zero impact on the ordering of the
message or its reply.

https://codereview.chromium.org/2363573002/diff/200001/content/common/render_...
File content/common/render_message_filter.mojom (right):

https://codereview.chromium.org/2363573002/diff/200001/content/common/render_...
content/common/render_message_filter.mojom:58: blink.mojom.WindowFeatures
features;
On 2016/09/27 at 20:32:41, ncarter wrote:
> I double checked, and removing |additional_features| seems okay.

Ack.

https://codereview.chromium.org/2363573002/diff/200001/content/common/render_...
content/common/render_message_filter.mojom:67: int32 main_frame_route_id;
On 2016/09/27 at 20:32:41, ncarter wrote:
> Long-term, do you think we'll eventually be returning FrameHost mojom
interfaces here (and other places where we mint routing ids)?

Yes, that's the idea. I may even try to start pairing primordial interfaces with
routing IDs sooner rather than later if it means we can avoid routing ID params
in other message conversions (i.e. imagine if every message which introduced a
new routing ID to its receiver also provided an AssociatedInterfaceProviderPtr).

Starting small though, I decided to stick with rote conversion here.

https://codereview.chromium.org/2363573002/diff/200001/content/public/test/mo...
File content/public/test/mock_render_thread.cc (right):

https://codereview.chromium.org/2363573002/diff/200001/content/public/test/mo...
content/public/test/mock_render_thread.cc:50: // Note that
On 2016/09/27 at 20:32:41, ncarter wrote:
> ?

Oops. :>   Removed - this was going to be the comment which is now in the method
above.

https://codereview.chromium.org/2363573002/diff/200001/content/public/test/mo...
File content/public/test/mock_render_thread.h (right):

https://codereview.chromium.org/2363573002/diff/200001/content/public/test/mo...
content/public/test/mock_render_thread.h:45: // ViewMsg_Close : async, send to
the Widget.
On 2016/09/27 at 20:32:41, ncarter wrote:
> Let's redo this class comment so that it better describes why this class
exists. Something like this:
> 
> // This class provides a mock implementation of RenderThread, which is not
> // actually backed by a channel to a browser process. Only a few IPC messages
> // are handled, just enough allow top-level objects (e.g. frames and widgets)
> // to be created.
> //
> // This class is designed for inheritance: embedders of content may subclass
> // it to handle additional IPCs.

SGTM as-is. Done.

https://codereview.chromium.org/2363573002/diff/200001/content/renderer/rende...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/2363573002/diff/200001/content/renderer/rende...
content/renderer/render_view_impl.cc:1556:
RenderThreadImpl::current_render_message_filter()->CreateNewWindow(
On 2016/09/27 at 20:32:41, ncarter wrote:
> Why use this global and getter? Is it just about avoiding exposing it on
RenderThread (which is content/public)?
> 
> It seems weird to call into RenderThreadImpl:: to get a value that's owned by
the MockRenderThread. I'm not totally against this, but curious if there are any
alternatives worth considering.

Yeah, this was a semi-painful way to avoid exposing the mojom in content/public,
which I do think is worth avoiding. I'm not crazy about this either, so I'm
happy to entertain alternative ideas.

One approach would be to have each RenderViewImpl cache its own
RenderMessageFilterAssociatedPtr it gets from RenderThread's
GetChannel()->GetRemoteAssociatedInterface(). It's a bit of extra code to have
MockRenderThread implement its own mock Channel, but maybe that would be more
flexible long-term. WDYT? (happy to prototype it quickly if that helps)

[ncarter (slow), 2016-09-27 22:47:53]

lgtm

https://codereview.chromium.org/2363573002/diff/200001/content/browser/render...
File content/browser/renderer_host/render_message_filter.cc (right):

https://codereview.chromium.org/2363573002/diff/200001/content/browser/render...
content/browser/renderer_host/render_message_filter.cc:301:
callback.Run(std::move(reply));
On 2016/09/27 21:05:41, Ken Rockot wrote:
> On 2016/09/27 at 20:32:41, ncarter wrote:
> > Can we now guarantee that the task posted to the UI thread by
> |CreateNewWindow| will run before any UI-thread destined mojo message that may
> be sent from the child process after the reply message is received?
> 
> Yes. Any interfaces (such as mojom::RenderMessageFilter) which are associated
> with the IPC::Channel  exhibit perfect ordering with respect to Channel on the
> IO thread and ChannelProxy on the main thread.
> 
> To put it another way, this conversion has zero impact on the ordering of the
> message or its reply.

Thanks, and awesome that we have this safety guarantee.

I was particularly worried about the ordering once the next message gets
mojofied, the one that shows the view, since it comes in (pretty weirdly!) on
the opener's interface.

https://codereview.chromium.org/2363573002/diff/200001/content/common/render_...
File content/common/render_message_filter.mojom (right):

https://codereview.chromium.org/2363573002/diff/200001/content/common/render_...
content/common/render_message_filter.mojom:67: int32 main_frame_route_id;
On 2016/09/27 21:05:41, Ken Rockot wrote:
> On 2016/09/27 at 20:32:41, ncarter wrote:
> > Long-term, do you think we'll eventually be returning FrameHost mojom
> interfaces here (and other places where we mint routing ids)?
> 
> Yes, that's the idea. I may even try to start pairing primordial interfaces
with
> routing IDs sooner rather than later if it means we can avoid routing ID
params
> in other message conversions (i.e. imagine if every message which introduced a
> new routing ID to its receiver also provided an
AssociatedInterfaceProviderPtr).
> 
> Starting small though, I decided to stick with rote conversion here.

Looking forward to seeing that happen.

I still have on my TODO list to port CreateNewWindow from View to Frame (needed
to fix bug 627852). CreateWindow logically belongs on frame, because window
creation is an operation on the opener. Your CL here should make that process a
lot easier; when I get back to that bug, I'll be sure to check in with you, to
avoid conflicts.

https://codereview.chromium.org/2363573002/diff/200001/content/renderer/rende...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/2363573002/diff/200001/content/renderer/rende...
content/renderer/render_view_impl.cc:1556:
RenderThreadImpl::current_render_message_filter()->CreateNewWindow(
On 2016/09/27 21:05:42, Ken Rockot wrote:
> On 2016/09/27 at 20:32:41, ncarter wrote:
> > Why use this global and getter? Is it just about avoiding exposing it on
> RenderThread (which is content/public)?
> > 
> > It seems weird to call into RenderThreadImpl:: to get a value that's owned
by
> the MockRenderThread. I'm not totally against this, but curious if there are
any
> alternatives worth considering.
> 
> Yeah, this was a semi-painful way to avoid exposing the mojom in
content/public,
> which I do think is worth avoiding. I'm not crazy about this either, so I'm
> happy to entertain alternative ideas.
> 
> One approach would be to have each RenderViewImpl cache its own
> RenderMessageFilterAssociatedPtr it gets from RenderThread's
> GetChannel()->GetRemoteAssociatedInterface(). It's a bit of extra code to have
> MockRenderThread implement its own mock Channel, but maybe that would be more
> flexible long-term. WDYT? (happy to prototype it quickly if that helps)

I'd prefer to keep the singleton interfaces looking like singletons; if we had
pointers to them from each View, I can see getting confusion around whether the
pointer would actually be the same for any two RenderViews -- that seems even
more confusing if we apply that pattern to RenderFrameMessageFilter and
RenderFrames.

It looks like there is an existing, endemic problem; any code that calls
RenderThreadImpl::current() is basically untestable from a
MockRenderThread-based test.

I don't see a clean way to do this other than adding an extra level of
inheritance (e.g. a content-private RenderThreadBase, inheriting from
RenderThread, which becomes the common base class of RenderThreadImpl and
MockRenderThread), or a similar trick using implemented via composition. Curious
if jam@ has thoughts here.

Your approach makes sense for now. If we see this happening for other
interfaces, we can add more structure.

[dcheng, 2016-09-27 23:09:43]

https://codereview.chromium.org/2363573002/diff/200001/content/public/common/...
File content/public/common/referrer_struct_traits.h (right):

https://codereview.chromium.org/2363573002/diff/200001/content/public/common/...
content/public/common/referrer_struct_traits.h:25: return
data.ReadUrl(&out->url) && data.ReadPolicy(&out->policy);
Let's out-of-line this (it's not clear to me how much code this ends up
inlining, but it seems like it could be a reasonable amount)

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/ReferrerPolicyEnumTraits.h (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/platform/ReferrerPolicyEnumTraits.h:16: struct
EnumTraits<::blink::mojom::ReferrerPolicy, ::blink::WebReferrerPolicy> {
Is it possible to place this in namespace blink and rely on ADL to find this?
Then we can lose the ::blink prefix here.

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/referrer.mojom (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/platform/referrer.mojom:27: struct Referrer {
It's a little surprising to me that we define this in public/platform, when it's
only created and consumed in content. Is there somewhere else it would end up
being used in Blink?

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WindowFeaturesStructTraits.h (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/web/WindowFeaturesStructTraits.h:14: struct
StructTraits<MojomDataViewType, ::blink::WebWindowFeatures> {
How come we can't fully specialize this one?

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/web/WindowFeaturesStructTraits.h:36: out->x =
data.x();
Nit: out-of-line

Also, I think the Read() portion is actually unused, right? Perhaps I'm missing
it somewhere, but I don't think we ever turn the mojo struct containing this
back into a WebWindowFeatures struct.

[Ken Rockot(use gerrit already), 2016-09-27 23:54:34]

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

[commit-bot: I haz the power, 2016-09-27 23:55:07]

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

[Ken Rockot(use gerrit already), 2016-09-28 00:20:05]

https://codereview.chromium.org/2363573002/diff/200001/content/public/common/...
File content/public/common/referrer_struct_traits.h (right):

https://codereview.chromium.org/2363573002/diff/200001/content/public/common/...
content/public/common/referrer_struct_traits.h:25: return
data.ReadUrl(&out->url) && data.ReadPolicy(&out->policy);
On 2016/09/27 at 23:09:42, dcheng wrote:
> Let's out-of-line this (it's not clear to me how much code this ends up
inlining, but it seems like it could be a reasonable amount)

Done

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/ReferrerPolicyEnumTraits.h (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/platform/ReferrerPolicyEnumTraits.h:16: struct
EnumTraits<::blink::mojom::ReferrerPolicy, ::blink::WebReferrerPolicy> {
On 2016/09/27 at 23:09:42, dcheng wrote:
> Is it possible to place this in namespace blink and rely on ADL to find this?
Then we can lose the ::blink prefix here.

I'm not sure I understand what you mean. Do you mean change the bindings code to
rely on ADL everywhere by referring to unqualified StructTraits<U, V>? That
seems well beyond the scope of this CL, but we could consider it.

I'd feel a little uncomfortable using a name as generic as StructTraits in that
case too, so it might be better as MojoStructTraits, MojoEnumTraits,
MojoArrayTraits, etc.

If this is what you mean, we can always plan this work separately. I think it
might be worth doing.

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/referrer.mojom (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/platform/referrer.mojom:27: struct Referrer {
On 2016/09/27 at 23:09:42, dcheng wrote:
> It's a little surprising to me that we define this in public/platform, when
it's only created and consumed in content. Is there somewhere else it would end
up being used in Blink?

We could define it in content instead if you prefer. I guess my expectation is
that many interfaces which use it will eventually be consumed directly from
blink code, so this would very likely move to blink anyway. It feels like a web
platform concept.

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WindowFeaturesStructTraits.h (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/web/WindowFeaturesStructTraits.h:14: struct
StructTraits<MojomDataViewType, ::blink::WebWindowFeatures> {
On 2016/09/27 at 23:09:42, dcheng wrote:
> How come we can't fully specialize this one?

We can, but Chromium and Blink cannot share generated struct definitions (it
would be very complex to support), so if we fully specialized it, we'd then have
to define the same traits specialization twice: one over
blink::mojom::WindowFeatures and one over blink::mojom::blink::WindowFeatures.

Note that both Chromium and Blink typemaps map to blink::WebWindowFeatures.

This avoids explicit code duplication.

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/web/WindowFeaturesStructTraits.h:36: out->x =
data.x();
On 2016/09/27 at 23:09:42, dcheng wrote:
> Nit: out-of-line
> 

Can't move it out-of-line unless we fully specialize and duplicate the
definition for Blink and Chromium. Neither Chromium nor Blink is allowed to
include the other's generated headers, so a single cpp cannot not define full
specializations of Read for both of them.

We could define the partial specialization in a private header to reduce code
duplication and then instantiate it in separate h/cc for separate Chromium and
Blink typemaps to the same type, but this seems rather noisy and it's not clear
to me that it's worth doing.

> Also, I think the Read() portion is actually unused, right? Perhaps I'm
missing it somewhere, but I don't think we ever turn the mojo struct containing
this back into a WebWindowFeatures struct.

We definitely use it. There are references to blink::WebWindowFeatures all
around browser code, for better or worse. RenderViewHostImpl receives a
CreateNewWindowParams, and its |features| field is typemapped the same as the
sending side in blink.

[dcheng, 2016-09-28 00:30:51]

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/ReferrerPolicyEnumTraits.h (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/platform/ReferrerPolicyEnumTraits.h:16: struct
EnumTraits<::blink::mojom::ReferrerPolicy, ::blink::WebReferrerPolicy> {
On 2016/09/28 00:20:05, Ken Rockot wrote:
> On 2016/09/27 at 23:09:42, dcheng wrote:
> > Is it possible to place this in namespace blink and rely on ADL to find
this?
> Then we can lose the ::blink prefix here.
> 
> I'm not sure I understand what you mean. Do you mean change the bindings code
to
> rely on ADL everywhere by referring to unqualified StructTraits<U, V>? That
> seems well beyond the scope of this CL, but we could consider it.
> 
> I'd feel a little uncomfortable using a name as generic as StructTraits in
that
> case too, so it might be better as MojoStructTraits, MojoEnumTraits,
> MojoArrayTraits, etc.
> 
> If this is what you mean, we can always plan this work separately. I think it
> might be worth doing.

Sorry, to clarify, can we do:

namespace blink {

template<>
struct EnumTraits<mojom::ReferrerPolicy, WebReferrerPolicy> {
  // ... stuff
};

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/referrer.mojom (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/platform/referrer.mojom:27: struct Referrer {
On 2016/09/28 00:20:05, Ken Rockot wrote:
> On 2016/09/27 at 23:09:42, dcheng wrote:
> > It's a little surprising to me that we define this in public/platform, when
> it's only created and consumed in content. Is there somewhere else it would
end
> up being used in Blink?
> 
> We could define it in content instead if you prefer. I guess my expectation is
> that many interfaces which use it will eventually be consumed directly from
> blink code, so this would very likely move to blink anyway. It feels like a
web
> platform concept.

Perhaps, but it would be easy enough to move it into the appropriate layer then.

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WindowFeaturesStructTraits.h (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/web/WindowFeaturesStructTraits.h:14: struct
StructTraits<MojomDataViewType, ::blink::WebWindowFeatures> {
On 2016/09/28 00:20:05, Ken Rockot wrote:
> On 2016/09/27 at 23:09:42, dcheng wrote:
> > How come we can't fully specialize this one?
> 
> We can, but Chromium and Blink cannot share generated struct definitions (it
> would be very complex to support), so if we fully specialized it, we'd then
have
> to define the same traits specialization twice: one over
> blink::mojom::WindowFeatures and one over blink::mojom::blink::WindowFeatures.
> 
> Note that both Chromium and Blink typemaps map to blink::WebWindowFeatures.
> 
> This avoids explicit code duplication.

I'm going to need to think about this, but I thought this was the whole point of
http://crbug.com/632061

[Ken Rockot(use gerrit already), 2016-09-28 01:08:08]

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

[commit-bot: I haz the power, 2016-09-28 01:08:42]

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

[Ken Rockot(use gerrit already), 2016-09-28 01:15:23]

rockot@chromium.org changed reviewers:
+ esprehn@chromium.org

[Ken Rockot(use gerrit already), 2016-09-28 01:15:24]

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/ReferrerPolicyEnumTraits.h (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/platform/ReferrerPolicyEnumTraits.h:16: struct
EnumTraits<::blink::mojom::ReferrerPolicy, ::blink::WebReferrerPolicy> {
On 2016/09/28 at 00:30:51, dcheng wrote:
> On 2016/09/28 00:20:05, Ken Rockot wrote:
> > On 2016/09/27 at 23:09:42, dcheng wrote:
> > > Is it possible to place this in namespace blink and rely on ADL to find
this?
> > Then we can lose the ::blink prefix here.
> > 
> > I'm not sure I understand what you mean. Do you mean change the bindings
code to
> > rely on ADL everywhere by referring to unqualified StructTraits<U, V>? That
> > seems well beyond the scope of this CL, but we could consider it.
> > 
> > I'd feel a little uncomfortable using a name as generic as StructTraits in
that
> > case too, so it might be better as MojoStructTraits, MojoEnumTraits,
> > MojoArrayTraits, etc.
> > 
> > If this is what you mean, we can always plan this work separately. I think
it
> > might be worth doing.
> 
> Sorry, to clarify, can we do:
> 
> namespace blink {
> 
> template<>
> struct EnumTraits<mojom::ReferrerPolicy, WebReferrerPolicy> {
>   // ... stuff
> };

As discussed off thread, this is a no-go since ADL only applies to function
calls, not struct references.

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/referrer.mojom (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/platform/referrer.mojom:27: struct Referrer {
On 2016/09/28 at 00:30:51, dcheng wrote:
> On 2016/09/28 00:20:05, Ken Rockot wrote:
> > On 2016/09/27 at 23:09:42, dcheng wrote:
> > > It's a little surprising to me that we define this in public/platform,
when
> > it's only created and consumed in content. Is there somewhere else it would
end
> > up being used in Blink?
> > 
> > We could define it in content instead if you prefer. I guess my expectation
is
> > that many interfaces which use it will eventually be consumed directly from
> > blink code, so this would very likely move to blink anyway. It feels like a
web
> > platform concept.
> 
> Perhaps, but it would be easy enough to move it into the appropriate layer
then.

As discussed off thread, leaving this here for now since it's kind of weird no
matter where it goes (splitting mojom Referrer from mojom ReferrerPolicy is
weird, but splitting mojom ReferrerPolicy for WebReferrerPolicy is also weird).

+esprehn for opinion here

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WindowFeaturesStructTraits.h (right):

https://codereview.chromium.org/2363573002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/web/WindowFeaturesStructTraits.h:14: struct
StructTraits<MojomDataViewType, ::blink::WebWindowFeatures> {
On 2016/09/28 at 00:30:51, dcheng wrote:
> On 2016/09/28 00:20:05, Ken Rockot wrote:
> > On 2016/09/27 at 23:09:42, dcheng wrote:
> > > How come we can't fully specialize this one?
> > 
> > We can, but Chromium and Blink cannot share generated struct definitions (it
> > would be very complex to support), so if we fully specialized it, we'd then
have
> > to define the same traits specialization twice: one over
> > blink::mojom::WindowFeatures and one over
blink::mojom::blink::WindowFeatures.
> > 
> > Note that both Chromium and Blink typemaps map to blink::WebWindowFeatures.
> > 
> > This avoids explicit code duplication.
> 
> I'm going to need to think about this, but I thought this was the whole point
of http://crbug.com/632061

Indeed this was exactly the point of that work! Done.

[commit-bot: I haz the power, 2016-09-28 01:30:36]

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

[commit-bot: I haz the power, 2016-09-28 01:30:37]

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_...)

[esprehn, 2016-09-28 04:14:17]

Hmm, adding mojoms to the web/ layer is something new for sure, I think it's
okay, but I'm also trying to make the web layer smaller in general and get as
much of the code as possible moved out of there.

[Ken Rockot(use gerrit already), 2016-09-28 04:43:23]

On Sep 27, 2016 9:14 PM, <esprehn@chromium.org> wrote:
>
> Hmm, adding mojoms to the web/ layer is something new for sure, I think
it's
> okay, but I'm also trying to make the web layer smaller in general and
get as
> much of the code as possible moved out of there.

Sure, that makes sense. I would see any mojom introduced here as a
transitional phase until it can move to platform once content is no longer
using the corresponding web/ type.

>
> https://codereview.chromium.org/2363573002/

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

[Ken Rockot(use gerrit already), 2016-09-28 04:43:25]

On Sep 27, 2016 9:14 PM, <esprehn@chromium.org> wrote:
>
> Hmm, adding mojoms to the web/ layer is something new for sure, I think
it's
> okay, but I'm also trying to make the web layer smaller in general and
get as
> much of the code as possible moved out of there.

Sure, that makes sense. I would see any mojom introduced here as a
transitional phase until it can move to platform once content is no longer
using the corresponding web/ type.

>
> https://codereview.chromium.org/2363573002/

-- 
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.

[Ken Rockot(use gerrit already), 2016-09-28 15:10:59]

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

[commit-bot: I haz the power, 2016-09-28 15:11:15]

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-28 15:46:57]

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

[commit-bot: I haz the power, 2016-09-28 15:46:58]

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

[Ken Rockot(use gerrit already), 2016-09-28 15:51:12]

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

[commit-bot: I haz the power, 2016-09-28 15:51:27]

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-28 18:18:32]

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

[commit-bot: I haz the power, 2016-09-28 18:18:34]

Dry run: This issue passed the CQ dry run.

[Ken Rockot(use gerrit already), 2016-09-28 18:20:26]

So are we OK with landing this as-is?

[Ken Rockot(use gerrit already), 2016-09-28 19:01:47]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-09-28 19:01:48]

The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org,
jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2363573002/#ps280001
(title: ".")

[Ken Rockot(use gerrit already), 2016-09-28 19:01:51]

The CQ bit was unchecked by rockot@chromium.org

[dcheng, 2016-09-28 20:37:34]

mojo lgtm

[Ken Rockot(use gerrit already), 2016-09-28 23:04:25]

And blink?

[dcheng, 2016-09-28 23:05:04]

mojo *and* blink lgtm

sorry =P

[Ken Rockot(use gerrit already), 2016-09-28 23:06:37]

Thanks :p

On Sep 28, 2016 4:05 PM, <dcheng@chromium.org> wrote:

> mojo *and* blink lgtm
>
> sorry =P
>
> https://codereview.chromium.org/2363573002/
>

-- 
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.

[Ken Rockot(use gerrit already), 2016-09-28 23:06:39]

Thanks :p

On Sep 28, 2016 4:05 PM, <dcheng@chromium.org> wrote:

> mojo *and* blink lgtm
>
> sorry =P
>
> https://codereview.chromium.org/2363573002/
>

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

[Ken Rockot(use gerrit already), 2016-09-28 23:07:18]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2016-09-28 23:07:46]

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-28 23:14:37]

Description was changed from

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some transitional public
   constants have been added to satisfy existing references to
   the old enum values. This will be removed in future CLs.
 - Adds a mojom definition for blink::mojom::WindowFeatures,
   corresponding to blink::WebWindowFeatures. A typemap is
   added between these types and is shared by both Blink and
   Chromium bindings configurations.
 - Adds a blink::mojom::Referrer struct analogous to
   content::Renderer. Chromium bindings typemap this new mojom
   struct to content::Referrer.
 - Adds a mojom blink::mojom::ReferrerPolicy enum with a typemap
   to blink::WebReferrerPolicy, shared by Chromium and Blink
   bindings. This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some transitional public
   constants have been added to satisfy existing references to
   the old enum values. This will be removed in future CLs.
 - Adds a mojom definition for blink::mojom::WindowFeatures,
   corresponding to blink::WebWindowFeatures. A typemap is
   added between these types and is shared by both Blink and
   Chromium bindings configurations.
 - Adds a blink::mojom::Referrer struct analogous to
   content::Renderer. Chromium bindings typemap this new mojom
   struct to content::Referrer.
 - Adds a mojom blink::mojom::ReferrerPolicy enum with a typemap
   to blink::WebReferrerPolicy, shared by Chromium and Blink
   bindings. This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-09-28 23:14:39]

Committed patchset #6 (id:280001)

[commit-bot: I haz the power, 2016-09-28 23:20:02]

Description was changed from

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some transitional public
   constants have been added to satisfy existing references to
   the old enum values. This will be removed in future CLs.
 - Adds a mojom definition for blink::mojom::WindowFeatures,
   corresponding to blink::WebWindowFeatures. A typemap is
   added between these types and is shared by both Blink and
   Chromium bindings configurations.
 - Adds a blink::mojom::Referrer struct analogous to
   content::Renderer. Chromium bindings typemap this new mojom
   struct to content::Referrer.
 - Adds a mojom blink::mojom::ReferrerPolicy enum with a typemap
   to blink::WebReferrerPolicy, shared by Chromium and Blink
   bindings. This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Move ViewHostMsg_CreateWindow to mojom

Converts this message, addressing several of its dependencies
in the process:

 - Makes content::WindowContainerType an alias for the new
   content::mojom::WindowContainerType. Some transitional public
   constants have been added to satisfy existing references to
   the old enum values. This will be removed in future CLs.
 - Adds a mojom definition for blink::mojom::WindowFeatures,
   corresponding to blink::WebWindowFeatures. A typemap is
   added between these types and is shared by both Blink and
   Chromium bindings configurations.
 - Adds a blink::mojom::Referrer struct analogous to
   content::Renderer. Chromium bindings typemap this new mojom
   struct to content::Referrer.
 - Adds a mojom blink::mojom::ReferrerPolicy enum with a typemap
   to blink::WebReferrerPolicy, shared by Chromium and Blink
   bindings. This can be de-duplicated in future CLs.

BUG=612500
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/5c478a7b04b55630c5c491d4d1d2863e965cfa32
Cr-Commit-Position: refs/heads/master@{#421674}
==========

[commit-bot: I haz the power, 2016-09-28 23:20:04]

Patchset 6 (id:??) landed as
https://crrev.com/5c478a7b04b55630c5c491d4d1d2863e965cfa32
Cr-Commit-Position: refs/heads/master@{#421674}