Issue 10449063: Create default implementations of ContentRendererClient, ContentUtilityClient, and ContentPluginCli…

Issue 10449063: Create default implementations of ContentRendererClient, ContentUtilityClient, and ContentPluginCli… (Closed)

Created:
8 years, 6 months ago by jam

Modified:
8 years, 6 months ago

Reviewers:
Jói

CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, jochen+watch-content_chromium.org, tfarina

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

More Reviews

Description

Create default implementations of ContentRendererClient, ContentUtilityClient, and ContentPluginClient to make it easier to embed content. BUG=98716 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=139432

Patch Set 1 : #

Patch Set 2 : #

Total comments: 6

Created: 8 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+153 lines, -227 lines)			Patch
M	content/content_renderer.gypi	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	content/content_utility.gypi	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	content/public/plugin/content_plugin_client.h	View		1 chunk	+2 lines, -2 lines	4 comments	Download
M	content/public/renderer/content_renderer_client.h	View		5 chunks	+26 lines, -30 lines	2 comments	Download
A	content/public/renderer/content_renderer_client.cc	View		1 chunk	+106 lines, -0 lines	0 comments	Download
M	content/public/utility/content_utility_client.h	View		1 chunk	+3 lines, -3 lines	0 comments	Download
A	content/public/utility/content_utility_client.cc	View		1 chunk	+13 lines, -0 lines	0 comments	Download
M	content/public/utility/utility_thread.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	content/shell/shell_content_plugin_client.h	View	1	1 chunk	+0 lines, -1 line	0 comments	Download
M	content/shell/shell_content_plugin_client.cc	View		1 chunk	+0 lines, -4 lines	0 comments	Download
M	content/shell/shell_content_renderer_client.h	View		1 chunk	+0 lines, -62 lines	0 comments	Download
M	content/shell/shell_content_renderer_client.cc	View		1 chunk	+0 lines, -124 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

tfarina

https://chromiumcodereview.appspot.com/10449063/diff/2004/content/public/renderer/content_renderer_client.h File content/public/renderer/content_renderer_client.h (left): https://chromiumcodereview.appspot.com/10449063/diff/2004/content/public/renderer/content_renderer_client.h#oldcode52 content/public/renderer/content_renderer_client.h:52: template<class T> class Handle; why not keep these forward ...

8 years, 6 months ago (2012-05-30 00:10:33 UTC) #3

tfarina

https://chromiumcodereview.appspot.com/10449063/diff/2004/content/public/plugin/content_plugin_client.h File content/public/plugin/content_plugin_client.h (right): https://chromiumcodereview.appspot.com/10449063/diff/2004/content/public/plugin/content_plugin_client.h#newcode10 content/public/plugin/content_plugin_client.h:10: #include "content/public/common/content_client.h" looks like this include can be removed ...

8 years, 6 months ago (2012-05-30 00:26:49 UTC) #4

jam

8 years, 6 months ago (2012-05-30 14:50:21 UTC) #5

https://chromiumcodereview.appspot.com/10449063/diff/2004/content/public/plug...
File content/public/plugin/content_plugin_client.h (right):

https://chromiumcodereview.appspot.com/10449063/diff/2004/content/public/plug...
content/public/plugin/content_plugin_client.h:10: #include
"content/public/common/content_client.h"
On 2012/05/30 00:26:49, tfarina wrote:
> looks like this include can be removed and content_export.h should be included
> instead.

by convention, all content_foo_client.h files include content_client.h so that
cc files don't have to include both of them

https://chromiumcodereview.appspot.com/10449063/diff/2004/content/public/plug...
content/public/plugin/content_plugin_client.h:15: class CONTENT_EXPORT
ContentPluginClient {
On 2012/05/30 00:26:49, tfarina wrote:
> does this need a virtual destructor?

yep according to the style guide, i'll add one.

https://chromiumcodereview.appspot.com/10449063/diff/2004/content/public/rend...
File content/public/renderer/content_renderer_client.h (left):

https://chromiumcodereview.appspot.com/10449063/diff/2004/content/public/rend...
content/public/renderer/content_renderer_client.h:52: template<class T> class
Handle;
On 2012/05/30 00:10:33, tfarina wrote:
> why not keep these forward declarations? I guess it doesn't hurt. :/ I like
them
> more than including v8.h in header files.

I had to include v8.h after I inlined some methods

Expand Messages | Collapse Messages