Make an empty content browser test work.

Make an empty content browser test work.





BUG=90448
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=104181

[jam, 2011-10-04 22:21:13]

http://codereview.chromium.org/8137012/diff/1/content/content_shell.gypi
File content/content_shell.gypi (right):

http://codereview.chromium.org/8137012/diff/1/content/content_shell.gypi#newc...
content/content_shell.gypi:86: 'resource_include_dirs': [
do you need this block (86-105) twice, here and below?

http://codereview.chromium.org/8137012/diff/1/content/content_tests.gypi
File content/content_tests.gypi (right):

http://codereview.chromium.org/8137012/diff/1/content/content_tests.gypi#newc...
content/content_tests.gypi:256:
'<(SHARED_INTERMEDIATE_DIR)/net/net_resources.rc',
why are all these added? are they necessary?

http://codereview.chromium.org/8137012/diff/1/content/shell/shell_main_delega...
File content/shell/shell_main_delegate.cc (right):

http://codereview.chromium.org/8137012/diff/1/content/shell/shell_main_delega...
content/shell/shell_main_delegate.cc:38: void
ShellMainDelegate::InitializeShellContentClient(const std::string& process_type)
{
nit: 80 chars

http://codereview.chromium.org/8137012/diff/1/content/shell/shell_main_delega...
File content/shell/shell_main_delegate.h (right):

http://codereview.chromium.org/8137012/diff/1/content/shell/shell_main_delega...
content/shell/shell_main_delegate.h:12: #include
"content/shell/shell_content_client.h"
nit: don't need this, can forward declare

http://codereview.chromium.org/8137012/diff/1/content/test/content_test_launc...
File content/test/content_test_launcher.cc (right):

http://codereview.chromium.org/8137012/diff/1/content/test/content_test_launc...
content/test/content_test_launcher.cc:49: #if defined(OS_WIN)
we shouldn't copy all this code from chrome_test_launcher, but should share it
instead (i.e. what we agreed on in
http://codereview.chromium.org/8036044/diff/13001/chrome/test/base/chrome_tes...
:) )

[Paweł Hajdan Jr., 2011-10-04 23:19:48]

http://codereview.chromium.org/8137012/diff/1/content/content_shell.gypi
File content/content_shell.gypi (right):

http://codereview.chromium.org/8137012/diff/1/content/content_shell.gypi#newc...
content/content_shell.gypi:86: 'resource_include_dirs': [
On 2011/10/04 22:21:13, John Abd-El-Malek wrote:
> do you need this block (86-105) twice, here and below?

Done.

http://codereview.chromium.org/8137012/diff/1/content/content_tests.gypi
File content/content_tests.gypi (right):

http://codereview.chromium.org/8137012/diff/1/content/content_tests.gypi#newc...
content/content_tests.gypi:256:
'<(SHARED_INTERMEDIATE_DIR)/net/net_resources.rc',
On 2011/10/04 22:21:13, John Abd-El-Malek wrote:
> why are all these added? are they necessary?

No idea, they were there.

http://codereview.chromium.org/8137012/diff/1/content/shell/shell_main_delega...
File content/shell/shell_main_delegate.cc (right):

http://codereview.chromium.org/8137012/diff/1/content/shell/shell_main_delega...
content/shell/shell_main_delegate.cc:38: void
ShellMainDelegate::InitializeShellContentClient(const std::string& process_type)
{
On 2011/10/04 22:21:13, John Abd-El-Malek wrote:
> nit: 80 chars

Done.

http://codereview.chromium.org/8137012/diff/1/content/shell/shell_main_delega...
File content/shell/shell_main_delegate.h (right):

http://codereview.chromium.org/8137012/diff/1/content/shell/shell_main_delega...
content/shell/shell_main_delegate.h:12: #include
"content/shell/shell_content_client.h"
On 2011/10/04 22:21:13, John Abd-El-Malek wrote:
> nit: don't need this, can forward declare

Do you want me to convert the member variable below to a scoped_ptr? Just making
sure.

http://codereview.chromium.org/8137012/diff/1/content/test/content_test_launc...
File content/test/content_test_launcher.cc (right):

http://codereview.chromium.org/8137012/diff/1/content/test/content_test_launc...
content/test/content_test_launcher.cc:49: #if defined(OS_WIN)
On 2011/10/04 22:21:13, John Abd-El-Malek wrote:
> we shouldn't copy all this code from chrome_test_launcher, but should share it
> instead (i.e. what we agreed on in
>
http://codereview.chromium.org/8036044/diff/13001/chrome/test/base/chrome_tes...
> :) )

Done.

[jam, 2011-10-05 00:54:01]

http://codereview.chromium.org/8137012/diff/4001/content/test/content_test_la...
File content/test/content_test_launcher.cc (right):

http://codereview.chromium.org/8137012/diff/4001/content/test/content_test_la...
content/test/content_test_launcher.cc:19: #include
"content/app/startup_helper_win.h"
don't think you need all these includes

http://codereview.chromium.org/8137012/diff/4001/content/test/test_launcher.cc
File content/test/test_launcher.cc (right):

http://codereview.chromium.org/8137012/diff/4001/content/test/test_launcher.c...
content/test/test_launcher.cc:561: // TODO(pkasting): This "single_process vs.
single-process" design is
this code was duplicated from chrome_test_launcher.cc. now it just moved from
one file to another in content, but we need it to be shared (i.e. remove the
chrome copy)

[Paweł Hajdan Jr., 2011-10-05 18:34:26]

PTAL

http://codereview.chromium.org/8137012/diff/4001/content/test/content_test_la...
File content/test/content_test_launcher.cc (right):

http://codereview.chromium.org/8137012/diff/4001/content/test/content_test_la...
content/test/content_test_launcher.cc:19: #include
"content/app/startup_helper_win.h"
On 2011/10/05 00:54:01, John Abd-El-Malek wrote:
> don't think you need all these includes

Done.

http://codereview.chromium.org/8137012/diff/4001/content/test/test_launcher.cc
File content/test/test_launcher.cc (right):

http://codereview.chromium.org/8137012/diff/4001/content/test/test_launcher.c...
content/test/test_launcher.cc:561: // TODO(pkasting): This "single_process vs.
single-process" design is
On 2011/10/05 00:54:01, John Abd-El-Malek wrote:
> this code was duplicated from chrome_test_launcher.cc. now it just moved from
> one file to another in content, but we need it to be shared (i.e. remove the
> chrome copy)

Done (just forgot to gcl add those files).

[jam, 2011-10-05 19:36:04]

lgtm

[Dmitry Azaraev, 2011-11-06 23:02:08]

This commit breaks content_shell project.
Still now (src@108805) content_shell.exe doesn't contain *ANY* win32 resources.
So, no menu, and other resources initially added - missed.

[Dmitry Azaraev, 2011-11-06 23:10:59]

Sorry, missed: resource.h, shell.rc, other .rc files moved out from
content_shell.gypi, and now landed at content_tests.gypi, so, as i'm understand
no have chance to get resources.