Issue 1461243002: [OLD ATTEMPT, DO NOT REVIEW] mustash: Enable connections to mus from the Chrome renderer

Created:
5 years, 1 month ago by Fady Samuel

Modified:
5 years ago

Reviewers:
Ben Goodger (Google)

CC:
chromium-reviews, creis+watch_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, tfarina, nasko+codewatch_chromium.org, jam, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin-cc_chromium.org, darin (slow to review), ben+mojo_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

[OLD ATTEMPT, DO NOT REVIEW] mustash: Enable connections to mus from the Chrome renderer This CL introduces the initial plumbing to enable connections to mus from the Chrome renderer. The connection is established as follows: 1. RenderViewImpl creates a RenderWidgetMus. 2. RenderWidgetMus establishes a connection with the browser, requesting a RenderProcessClient interface. 3. RenderProcessConnectionListener intercepts the request in the browser, and creates a RenderProcessConnection with the process ID of the renderer. 4. RenderWidgetMus informs the RenderProcessClient that it has been created with the specified routing ID, and provides a WindowTreeClient interface. 5. RenderProcessConnection receives the routing ID and WindowTreeClient interface ptr, and passes it along to the associated RenderWidgetHostViewMus. 6. RenderWidgetHostViewMus embeds the WindowTreeClient with its associated mus::Window. 7. RenderWidgetMus sees the OnEmbed, and begins to put things on screen inside its window. BUG=551250

Patch Set 1 #

Patch Set 2 : SubmitCompositorFrame works from renderer. Needs lots of work #

Patch Set 3 : Invert connection creation flow. Needs lots of work. #

Total comments: 12

Patch Set 4 : Something closer to a sensible solution. Still not review ready #

Patch Set 5 : A bunch of cleanup #

Total comments: 22

Patch Set 6 : Addressed Ben's comments #

Total comments: 8

Patch Set 7 : Addressed Ben's comments #

Created: 5 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+428 lines, -21 lines)			Patch
M	content/browser/BUILD.gn	View	1 2 3 4 5 6	2 chunks	+6 lines, -0 lines	0 comments	Download
M	content/browser/DEPS	View	1 2 3 4	1 chunk	+1 line, -0 lines	0 comments	Download
M	content/browser/browser_main_loop.cc	View	1 2 3 4 5	2 chunks	+2 lines, -0 lines	0 comments	Download
M	content/browser/mojo/mojo_shell_client_host.h	View	1 2 3 4 5	2 chunks	+7 lines, -0 lines	0 comments	Download
M	content/browser/mojo/mojo_shell_client_host.cc	View	1 2 3 4 5 6	3 chunks	+15 lines, -0 lines	0 comments	Download
A	content/browser/mus_util.h	View	1 2 3 4 5 6	1 chunk	+22 lines, -0 lines	0 comments	Download
A	content/browser/mus_util.cc	View	1 2 3 4 5 6	1 chunk	+17 lines, -0 lines	0 comments	Download
A	content/browser/render_process_connection_listener.h	View	1 2 3 4 5	1 chunk	+14 lines, -0 lines	0 comments	Download
A	content/browser/render_process_connection_listener.cc	View	1 2 3 4 5	1 chunk	+89 lines, -0 lines	0 comments	Download
M	content/browser/renderer_host/DEPS	View	1 2 3 4	1 chunk	+1 line, -0 lines	0 comments	Download
M	content/browser/renderer_host/render_process_host_impl.cc	View	1 2 3 4 5 6	1 chunk	+3 lines, -0 lines	0 comments	Download
M	content/browser/renderer_host/render_widget_host_view_base.h	View	1 2 3 4 5	1 chunk	+1 line, -0 lines	0 comments	Download
M	content/browser/renderer_host/render_widget_host_view_base.cc	View	1 2 3 4 5	1 chunk	+4 lines, -0 lines	0 comments	Download
M	content/browser/renderer_host/render_widget_host_view_mus.h	View	1 2 3 4 5	3 chunks	+10 lines, -1 line	0 comments	Download
M	content/browser/renderer_host/render_widget_host_view_mus.cc	View	1 2 3 4 5	8 chunks	+32 lines, -7 lines	0 comments	Download
M	content/browser/web_contents/web_contents_impl.h	View	1	1 chunk	+0 lines, -2 lines	0 comments	Download
M	content/browser/web_contents/web_contents_impl.cc	View	1 2 3 4 5 6	3 chunks	+7 lines, -2 lines	0 comments	Download
M	content/browser/web_contents/web_contents_view_mus.h	View	1	3 chunks	+5 lines, -1 line	0 comments	Download
M	content/browser/web_contents/web_contents_view_mus.cc	View	1 2	6 chunks	+18 lines, -5 lines	0 comments	Download
M	content/common/BUILD.gn	View	1 2 3 4 5	1 chunk	+1 line, -0 lines	0 comments	Download
M	content/common/mojo/mojo_shell_connection_impl.cc	View	1 2 3 4 5	2 chunks	+4 lines, -1 line	0 comments	Download
A	content/common/render_process_client.mojom	View	1 2 3 4 5	1 chunk	+12 lines, -0 lines	0 comments	Download
M	content/public/browser/BUILD.gn	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	content/public/browser/render_widget_host_view.h	View	1 2 3 4 5	2 chunks	+4 lines, -0 lines	0 comments	Download
M	content/public/common/mojo_shell_connection.h	View	1 2 3 4 5 6	2 chunks	+1 line, -1 line	0 comments	Download
M	content/renderer/BUILD.gn	View	1 2 3 4 5 6	2 chunks	+8 lines, -0 lines	0 comments	Download
M	content/renderer/DEPS	View	1 2 3 4	1 chunk	+1 line, -0 lines	0 comments	Download
M	content/renderer/render_view_impl.cc	View	1 2 3 4 5 6	2 chunks	+12 lines, -0 lines	0 comments	Download
A	content/renderer/render_widget_mus.h	View	1 2 3 4 5 6	1 chunk	+41 lines, -0 lines	0 comments	Download
A	content/renderer/render_widget_mus.cc	View	1 2 3 4 5 6	1 chunk	+83 lines, -0 lines	0 comments	Download
M	ui/views/mus/native_widget_mus.cc	View	1	1 chunk	+4 lines, -1 line	0 comments	Download

Messages

Total messages: 14 (8 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

Fady Samuel

Notes to self. https://codereview.chromium.org/1461243002/diff/40001/content/browser/renderer_host/render_process_host_impl.cc File content/browser/renderer_host/render_process_host_impl.cc (right): https://codereview.chromium.org/1461243002/diff/40001/content/browser/renderer_host/render_process_host_impl.cc#newcode534 content/browser/renderer_host/render_process_host_impl.cc:534: mojo_ready_(false), Revert this. https://codereview.chromium.org/1461243002/diff/40001/content/browser/renderer_host/render_process_host_impl.h File content/browser/renderer_host/render_process_host_impl.h ...

5 years ago (2015-11-24 04:42:38 UTC) #1

Fady Samuel

+ben@ for some early feedback. This just puts a green frame on screen currently. Should ...

5 years ago (2015-11-24 06:20:55 UTC) #3

Ben Goodger (Google)

i will have more thoughts on naming of the render_widget_view etc interfaces tomorrow morning https://codereview.chromium.org/1461243002/diff/80001/content/browser/render_widget_view_service_listener.cc ...

5 years ago (2015-11-24 06:42:40 UTC) #4

Fady Samuel

I've addressed your comments and I've also renamed some things to (hopefully) more sensible names. ...

5 years ago (2015-11-24 13:32:07 UTC) #5

Ben Goodger (Google)

https://codereview.chromium.org/1461243002/diff/100001/chrome/browser/ui/browser_navigator.cc File chrome/browser/ui/browser_navigator.cc (right): https://codereview.chromium.org/1461243002/diff/100001/chrome/browser/ui/browser_navigator.cc#newcode373 chrome/browser/ui/browser_navigator.cc:373: create_params.context->GetNativeWindowProperty("mus")); let's create a helper function somewhere and use ...

5 years ago (2015-11-24 15:30:06 UTC) #6

Fady Samuel

Description was changed from ========== mustash: Enable connections to mus from the Chrome renderer This ...

5 years ago (2015-11-24 16:38:53 UTC) #9

Description was changed from

==========
mustash: Enable connections to mus from the Chrome renderer

This CL introduces the initial plumbing to enable connections to mus from the
Chrome renderer.

The connection is established as follows:

1. RenderViewImpl creates a RenderWidgetMus.
2. RenderWidgetMus establishes a connection with the browser, requesting a
RenderProcessClient interface.
3. RenderProcessConnectionListener intercepts the request in the browser, and
creates a RenderProcessConnection with the process ID of the renderer.
4. RenderWidgetMus informs the RenderProcessClient that it has been created with
the specified routing ID, and provides a  WindowTreeClient interface.
5. RenderProcessConnection receives the routing ID and WindowTreeClient
interface ptr, and passes it along to the associated RenderWidgetHostViewMus.
6. RenderWidgetHostViewMus embeds the WindowTreeClient with its associated
mus::Window.
7. RenderWidgetMus sees the OnEmbed, and begins to put things on screen inside
its window.


, passing in the routing ID and the interfaceptr side of a WindowTreeClient. It
implements the request side.
BUG=
==========

to

==========
mustash: Enable connections to mus from the Chrome renderer

This CL introduces the initial plumbing to enable connections to mus from the
Chrome renderer.

The connection is established as follows:

1. RenderViewImpl creates a RenderWidgetMus.

2. RenderWidgetMus establishes a connection with the browser, 
    requesting a RenderProcessClient interface.

3. RenderProcessConnectionListener intercepts the request in the 
    browser, and creates a RenderProcessConnection with the process ID 
    of the renderer.

4. RenderWidgetMus informs the RenderProcessClient that it has been 
    created with the specified routing ID, and provides a  WindowTreeClient   
    interface.

5. RenderProcessConnection receives the routing ID and 
    WindowTreeClient interface ptr, and passes it along to the associated 
    RenderWidgetHostViewMus.

6. RenderWidgetHostViewMus embeds the WindowTreeClient with its 
    associated mus::Window.

7. RenderWidgetMus sees the OnEmbed, and begins to put things on
    screen inside its window.

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

Fady Samuel

Description was changed from ========== mustash: Enable connections to mus from the Chrome renderer This ...

5 years ago (2015-11-24 16:42:21 UTC) #10

Description was changed from

==========
mustash: Enable connections to mus from the Chrome renderer

This CL introduces the initial plumbing to enable connections to mus from the
Chrome renderer.

The connection is established as follows:

1. RenderViewImpl creates a RenderWidgetMus.

2. RenderWidgetMus establishes a connection with the browser, 
    requesting a RenderProcessClient interface.

3. RenderProcessConnectionListener intercepts the request in the 
    browser, and creates a RenderProcessConnection with the process ID 
    of the renderer.

4. RenderWidgetMus informs the RenderProcessClient that it has been 
    created with the specified routing ID, and provides a  WindowTreeClient   
    interface.

5. RenderProcessConnection receives the routing ID and 
    WindowTreeClient interface ptr, and passes it along to the associated 
    RenderWidgetHostViewMus.

6. RenderWidgetHostViewMus embeds the WindowTreeClient with its 
    associated mus::Window.

7. RenderWidgetMus sees the OnEmbed, and begins to put things on
    screen inside its window.

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

to

==========
mustash: Enable connections to mus from the Chrome renderer

This CL introduces the initial plumbing to enable connections to mus from the
Chrome renderer.

The connection is established as follows:

1. RenderViewImpl creates a RenderWidgetMus.

2. RenderWidgetMus establishes a connection with the browser, 
   requesting a RenderProcessClient interface.

3. RenderProcessConnectionListener intercepts the request in the 
   browser, and creates a RenderProcessConnection with the process ID 
   of the renderer.

4. RenderWidgetMus informs the RenderProcessClient that it has been 
   created with the specified routing ID, and provides a  WindowTreeClient      

   interface.

5. RenderProcessConnection receives the routing ID and 
   WindowTreeClient interface ptr, and passes it along to the associated 
   RenderWidgetHostViewMus.

6. RenderWidgetHostViewMus embeds the WindowTreeClient with its
   associated mus::Window.

7. RenderWidgetMus sees the OnEmbed, and begins to put things on
   screen inside its window.

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

Fady Samuel

5 years ago (2015-11-24 17:06:36 UTC) #11

PTAL Ben!

https://codereview.chromium.org/1461243002/diff/100001/chrome/browser/ui/brow...
File chrome/browser/ui/browser_navigator.cc (right):

https://codereview.chromium.org/1461243002/diff/100001/chrome/browser/ui/brow...
chrome/browser/ui/browser_navigator.cc:373:
create_params.context->GetNativeWindowProperty("mus"));
On 2015/11/24 15:30:05, Ben Goodger (Google) wrote:
> let's create a helper function somewhere and use that to obtain the window so
> the property can remain private.
> 
> furthermore I don't think you can safely add the mus_window parameter to
create
> params because of #define issues.
> 
> so I think you need to call this getter inside content at the point that you
> would have used mus_window

Done.

https://codereview.chromium.org/1461243002/diff/100001/content/common/mojo/mo...
File content/common/mojo/mojo_shell_connection_impl.cc (right):

https://codereview.chromium.org/1461243002/diff/100001/content/common/mojo/mo...
content/common/mojo/mojo_shell_connection_impl.cc:49:
STLDeleteElements(&listeners_);
On 2015/11/24 15:30:05, Ben Goodger (Google) wrote:
> document in the /public/ header that this object takes ownership of
non-removed
> listeners.

Done.

https://codereview.chromium.org/1461243002/diff/100001/content/public/browser...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/1461243002/diff/100001/content/public/browser...
content/public/browser/web_contents.h:144: mus::Window* mus_window;
On 2015/11/24 15:30:05, Ben Goodger (Google) wrote:
> this ifdef is going to cause you pain in targets not built with
> MOJO_SHELL_CLIENT

Done.

https://codereview.chromium.org/1461243002/diff/100001/content/renderer/rende...
File content/renderer/render_widget_view_mus.h (right):

https://codereview.chromium.org/1461243002/diff/100001/content/renderer/rende...
content/renderer/render_widget_view_mus.h:18: class RenderWidgetViewMus : public
mus::WindowTreeDelegate,
On 2015/11/24 15:30:05, Ben Goodger (Google) wrote:
> OK at this point maybe just call this RenderWidgetMus rather than
> RenderWidgetViewMus. I don't think we've thought through the abstraction well
> enough to modify the name much further.

Done.

Fady Samuel

Description was changed from ========== mustash: Enable connections to mus from the Chrome renderer This ...

5 years ago (2015-11-24 17:09:10 UTC) #12

Description was changed from

==========
mustash: Enable connections to mus from the Chrome renderer

This CL introduces the initial plumbing to enable connections to mus from the
Chrome renderer.

The connection is established as follows:

1. RenderViewImpl creates a RenderWidgetMus.

2. RenderWidgetMus establishes a connection with the browser, 
   requesting a RenderProcessClient interface.

3. RenderProcessConnectionListener intercepts the request in the 
   browser, and creates a RenderProcessConnection with the process ID 
   of the renderer.

4. RenderWidgetMus informs the RenderProcessClient that it has been 
   created with the specified routing ID, and provides a  WindowTreeClient      

   interface.

5. RenderProcessConnection receives the routing ID and 
   WindowTreeClient interface ptr, and passes it along to the associated 
   RenderWidgetHostViewMus.

6. RenderWidgetHostViewMus embeds the WindowTreeClient with its
   associated mus::Window.

7. RenderWidgetMus sees the OnEmbed, and begins to put things on
   screen inside its window.

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

to

==========
mustash: Enable connections to mus from the Chrome renderer

This CL introduces the initial plumbing to enable connections to mus from the
Chrome renderer.

The connection is established as follows:

1. RenderViewImpl creates a RenderWidgetMus.

2. RenderWidgetMus establishes a connection with the browser, 
   requesting a RenderProcessClient interface.

3. RenderProcessConnectionListener intercepts the request in the 
   browser, and creates a RenderProcessConnection with the process ID 
   of the renderer.

4. RenderWidgetMus informs the RenderProcessClient that it has been 
   created with the specified routing ID, and provides a  WindowTreeClient      

   interface.

5. RenderProcessConnection receives the routing ID and 
   WindowTreeClient interface ptr, and passes it along to the associated 
   RenderWidgetHostViewMus.

6. RenderWidgetHostViewMus embeds the WindowTreeClient with its
   associated mus::Window.

7. RenderWidgetMus sees the OnEmbed, and begins to put things on
   screen inside its window.

BUG=551250
==========

Fady Samuel

Description was changed from ========== mustash: Enable connections to mus from the Chrome renderer This ...

5 years ago (2015-11-24 17:09:22 UTC) #13

Description was changed from

==========
mustash: Enable connections to mus from the Chrome renderer

This CL introduces the initial plumbing to enable connections to mus from the
Chrome renderer.

The connection is established as follows:

1. RenderViewImpl creates a RenderWidgetMus.

2. RenderWidgetMus establishes a connection with the browser, 
   requesting a RenderProcessClient interface.

3. RenderProcessConnectionListener intercepts the request in the 
   browser, and creates a RenderProcessConnection with the process ID 
   of the renderer.

4. RenderWidgetMus informs the RenderProcessClient that it has been 
   created with the specified routing ID, and provides a  WindowTreeClient      

   interface.

5. RenderProcessConnection receives the routing ID and 
   WindowTreeClient interface ptr, and passes it along to the associated 
   RenderWidgetHostViewMus.

6. RenderWidgetHostViewMus embeds the WindowTreeClient with its
   associated mus::Window.

7. RenderWidgetMus sees the OnEmbed, and begins to put things on
   screen inside its window.

BUG=551250
==========

to

==========
mustash: Enable connections to mus from the Chrome renderer

This CL introduces the initial plumbing to enable connections to mus from 
the Chrome renderer.

The connection is established as follows:

1. RenderViewImpl creates a RenderWidgetMus.

2. RenderWidgetMus establishes a connection with the browser, 
   requesting a RenderProcessClient interface.

3. RenderProcessConnectionListener intercepts the request in the 
   browser, and creates a RenderProcessConnection with the process ID 
   of the renderer.

4. RenderWidgetMus informs the RenderProcessClient that it has been 
   created with the specified routing ID, and provides a  WindowTreeClient      

   interface.

5. RenderProcessConnection receives the routing ID and 
   WindowTreeClient interface ptr, and passes it along to the associated 
   RenderWidgetHostViewMus.

6. RenderWidgetHostViewMus embeds the WindowTreeClient with its
   associated mus::Window.

7. RenderWidgetMus sees the OnEmbed, and begins to put things on
   screen inside its window.

BUG=551250
==========

Fady Samuel

5 years ago (2015-11-25 03:18:57 UTC) #14

Description was changed from

==========
mustash: Enable connections to mus from the Chrome renderer

This CL introduces the initial plumbing to enable connections to mus from 
the Chrome renderer.

The connection is established as follows:

1. RenderViewImpl creates a RenderWidgetMus.

2. RenderWidgetMus establishes a connection with the browser, 
   requesting a RenderProcessClient interface.

3. RenderProcessConnectionListener intercepts the request in the 
   browser, and creates a RenderProcessConnection with the process ID 
   of the renderer.

4. RenderWidgetMus informs the RenderProcessClient that it has been 
   created with the specified routing ID, and provides a  WindowTreeClient      

   interface.

5. RenderProcessConnection receives the routing ID and 
   WindowTreeClient interface ptr, and passes it along to the associated 
   RenderWidgetHostViewMus.

6. RenderWidgetHostViewMus embeds the WindowTreeClient with its
   associated mus::Window.

7. RenderWidgetMus sees the OnEmbed, and begins to put things on
   screen inside its window.

BUG=551250
==========

to

==========
[OLD ATTEMPT, DO NOT REVIEW] mustash: Enable connections to mus from the Chrome
renderer

This CL introduces the initial plumbing to enable connections to mus from 
the Chrome renderer.

The connection is established as follows:

1. RenderViewImpl creates a RenderWidgetMus.

2. RenderWidgetMus establishes a connection with the browser, 
   requesting a RenderProcessClient interface.

3. RenderProcessConnectionListener intercepts the request in the 
   browser, and creates a RenderProcessConnection with the process ID 
   of the renderer.

4. RenderWidgetMus informs the RenderProcessClient that it has been 
   created with the specified routing ID, and provides a  WindowTreeClient      

   interface.

5. RenderProcessConnection receives the routing ID and 
   WindowTreeClient interface ptr, and passes it along to the associated 
   RenderWidgetHostViewMus.

6. RenderWidgetHostViewMus embeds the WindowTreeClient with its
   associated mus::Window.

7. RenderWidgetMus sees the OnEmbed, and begins to put things on
   screen inside its window.

BUG=551250
==========

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages