Issue 8440027: Change renderer_client interface for registering PPAPI Factories

Issue 8440027: Change renderer_client interface for registering PPAPI Factories (Closed)

Created:
9 years, 1 month ago by Dirk Pranke

Modified:
9 years, 1 month ago

Reviewers:
jam

CC:
chromium-reviews, joi+watch-content_chromium.org, jam, dpranke-watch+content_chromium.org, darin-cc_chromium.org, brettw-cc_chromium.org, darin (slow to review), brettw

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

In order to make sure that we only have one list of registered PPAPI factories with the content component build, we need to change the renderer_client interface so that we tell the client which ppapi factory manager to use, rather than relying on a static global. R=jam@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=108549

Patch Set 1 #

Patch Set 2 : revamp fix #

Patch Set 3 : remove changes from content_client.h #

Patch Set 4 : clean up renderer_main slightly #

Total comments: 6

Patch Set 5 : update other renderer_clients #

Created: 9 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+42 lines, -18 lines)			Patch
M	chrome/renderer/chrome_content_renderer_client.h	View	1 2 3 4	1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/renderer/chrome_content_renderer_client.cc	View	1	3 chunks	+6 lines, -2 lines	0 comments	Download
M	chrome/renderer/chrome_ppapi_interfaces.h	View	1	1 chunk	+4 lines, -4 lines	0 comments	Download
M	chrome/renderer/chrome_ppapi_interfaces.cc	View	1 2 3 4	1 chunk	+0 lines, -12 lines	0 comments	Download
M	content/public/renderer/content_renderer_client.h	View	1	2 chunks	+9 lines, -0 lines	0 comments	Download
M	content/renderer/mock_content_renderer_client.h	View	1 2 3 4	1 chunk	+3 lines, -0 lines	0 comments	Download
M	content/renderer/mock_content_renderer_client.cc	View	1 2 3 4	1 chunk	+4 lines, -0 lines	0 comments	Download
M	content/renderer/renderer_main.cc	View	1 2 3	2 chunks	+7 lines, -0 lines	0 comments	Download
M	content/shell/shell_content_renderer_client.h	View	1 2 3 4	1 chunk	+2 lines, -0 lines	0 comments	Download
M	content/shell/shell_content_renderer_client.cc	View	1 2 3 4	1 chunk	+4 lines, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Dirk Pranke

okay, I've revamped this based on our discussion, and I think something like this could ...

9 years, 1 month ago (2011-11-02 20:31:46 UTC) #2

jam

lgtm http://codereview.chromium.org/8440027/diff/6001/chrome/renderer/chrome_content_renderer_client.h File chrome/renderer/chrome_content_renderer_client.h (right): http://codereview.chromium.org/8440027/diff/6001/chrome/renderer/chrome_content_renderer_client.h#newcode109 chrome/renderer/chrome_content_renderer_client.h:109: webkit::ppapi::PpapiInterfaceFactoryManager* factory_manager) OVERRIDE; nit: spacing http://codereview.chromium.org/8440027/diff/6001/chrome/renderer/chrome_ppapi_interfaces.cc File chrome/renderer/chrome_ppapi_interfaces.cc ...

9 years, 1 month ago (2011-11-02 21:42:41 UTC) #3

Dirk Pranke

9 years, 1 month ago (2011-11-02 22:09:25 UTC) #4

turns out I need to update the other renderer_clients, natch.

http://codereview.chromium.org/8440027/diff/6001/chrome/renderer/chrome_conte...
File chrome/renderer/chrome_content_renderer_client.h (right):

http://codereview.chromium.org/8440027/diff/6001/chrome/renderer/chrome_conte...
chrome/renderer/chrome_content_renderer_client.h:109:
webkit::ppapi::PpapiInterfaceFactoryManager* factory_manager) OVERRIDE;
On 2011/11/02 21:42:41, John Abd-El-Malek wrote:
> nit: spacing

Done.

http://codereview.chromium.org/8440027/diff/6001/chrome/renderer/chrome_ppapi...
File chrome/renderer/chrome_ppapi_interfaces.cc (right):

http://codereview.chromium.org/8440027/diff/6001/chrome/renderer/chrome_ppapi...
chrome/renderer/chrome_ppapi_interfaces.cc:13: #include
"content/public/common/content_client.h"
On 2011/11/02 21:42:41, John Abd-El-Malek wrote:
> nit: not needed

Done.

http://codereview.chromium.org/8440027/diff/6001/content/renderer/renderer_ma...
File content/renderer/renderer_main.cc (right):

http://codereview.chromium.org/8440027/diff/6001/content/renderer/renderer_ma...
content/renderer/renderer_main.cc:159: factory_manager);
On 2011/11/02 20:31:46, Dirk Pranke wrote:
> It's not clear to me that this is the right place to call this; WDYT?

Will leave it here for now, should be fine.

Expand Messages | Collapse Messages