Reduce the amount of included header files. Vast change like in "Oh God! This...

Reduce the amount of included header files. Vast change like in "Oh God! This revision changes half of the source files!".

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

[M-A Ruel, 2009-02-14 02:14:37]

After the snapshot, I added in memory_details.cc(9):
#include "chrome/browser/browser_process.h"
to fix windows build. Mac build fine.

On linux, it still fails with:
________Linking Hammer/dbg/obj/webkit/glue/plugins/test/libnpapi_test_plugin.so
Hammer/dbg/lib/libnet.a(ssl_client_socket_nss.o):(.rodata._ZTVN3net18SSLClientSocketNSSE[vtable
for net::SSLClientSocketNSS]+0x28): undefined reference to
`net::ClientSocket::GetPeerName(sockaddr*, unsigned int*)'
Hammer/dbg/lib/libnet.a(ssl_client_socket_nss.o):(.rodata._ZTVN3net15SSLClientSocketE[vtable
for net::SSLClientSocket]+0x28): undefined reference to
`net::ClientSocket::GetPeerName(sockaddr*, unsigned int*)'

Any idea why? It correctly compiles client_socket.cc.

[Lei Zhang, 2009-02-17 19:09:41]

Review in progress. I wish this was split up into smaller pieces.

http://codereview.chromium.org/20378/diff/1/34
File base/task.h (right):

http://codereview.chromium.org/20378/diff/1/34#newcode10
Line 10: #include "base/logging.h"
Do we need logging.h and time.h?

http://codereview.chromium.org/20378/diff/1/63
File chrome/browser/chrome_plugin_host.cc (left):

http://codereview.chromium.org/20378/diff/1/63#oldcode43
Line 43: #include "base/string_util.h"
needed for StringToLowerASCII().

http://codereview.chromium.org/20378/diff/1/63
File chrome/browser/chrome_plugin_host.cc (right):

http://codereview.chromium.org/20378/diff/1/63#newcode13
Line 13: #include "base/logging.h"
I don't see logging being used.

http://codereview.chromium.org/20378/diff/1/41
File chrome/browser/debugger/debugger_contents.cc (right):

http://codereview.chromium.org/20378/diff/1/41#newcode19
Line 19: #include "net/base/upload_data.h"
Why do we need upload_data.h?

http://codereview.chromium.org/20378/diff/1/42
File chrome/browser/debugger/debugger_node.cc (right):

http://codereview.chromium.org/20378/diff/1/42#newcode17
Line 17: #include "net/base/upload_data.h"
Do we need upload_data.h?

http://codereview.chromium.org/20378/diff/1/120
File chrome/browser/memory_details.cc (left):

http://codereview.chromium.org/20378/diff/1/120#oldcode9
Line 9: #include "base/histogram.h"
needed for #define UMA_HISTOGRAM_MEMORY_KB

http://codereview.chromium.org/20378/diff/1/120#oldcode11
Line 11: #include "base/message_loop.h"
Aren't we using MessageLoop?

http://codereview.chromium.org/20378/diff/1/120#oldcode13
Line 13: #include "base/scoped_ptr.h"
We use scoped_ptr in this file.

http://codereview.chromium.org/20378/diff/1/120#oldcode20
Line 20: #include "chrome/browser/tab_contents/web_contents.h"
Shouldn't we get rid of web_contents.h (not used) and keep tab_contents.h?

http://codereview.chromium.org/20378/diff/1/64
File chrome/browser/net/dns_global.cc (left):

http://codereview.chromium.org/20378/diff/1/64#oldcode23
Line 23: #include "googleurl/src/gurl.h"
We use GURL on line 455.

http://codereview.chromium.org/20378/diff/1/44
File chrome/browser/net/url_fetcher.cc (right):

http://codereview.chromium.org/20378/diff/1/44#newcode93
Line 93: DISALLOW_EVIL_CONSTRUCTORS(Core);
Change to DISALLOW_COPY_AND_ASSIGN.

http://codereview.chromium.org/20378/diff/1/121
File chrome/browser/profile.cc (left):

http://codereview.chromium.org/20378/diff/1/121#oldcode34
Line 34: #include "chrome/common/pref_service.h"
We use PrefService in this file.

http://codereview.chromium.org/20378/diff/1/97
File chrome/browser/profile_manager.cc (right):

http://codereview.chromium.org/20378/diff/1/97#newcode22
Line 22: #include "net/http/http_transaction_factory.h"
Why do we need this?

[Lei Zhang, 2009-02-17 19:50:48]

http://codereview.chromium.org/20378/diff/1/47
File chrome/browser/renderer_host/resource_dispatcher_host.h (left):

http://codereview.chromium.org/20378/diff/1/47#oldcode19
Line 19: #include "base/logging.h"
We use vector and logging.h in this file.

http://codereview.chromium.org/20378/diff/1/47#oldcode22
Line 22: #include "base/ref_counted.h"
Do we need ref_counted.h?

http://codereview.chromium.org/20378/diff/1/48
File chrome/browser/renderer_host/resource_request_details.h (left):

http://codereview.chromium.org/20378/diff/1/48#oldcode14
Line 14: #include "base/basictypes.h"
DISALLOW_COPY_AND_ASSIGN is from basictypes.h

http://codereview.chromium.org/20378/diff/1/46
File chrome/browser/safe_browsing/protocol_manager.h (left):

http://codereview.chromium.org/20378/diff/1/46#oldcode15
Line 15: #include <vector>
We use string and vector here.

http://codereview.chromium.org/20378/diff/1/51
File chrome/common/net/url_request_intercept_job.h (left):

http://codereview.chromium.org/20378/diff/1/51#oldcode8
Line 8: #include "base/basictypes.h"
needed for DISALLOW_EVIL_CONSTRUCTORS.

http://codereview.chromium.org/20378/diff/1/124
File chrome/plugin/plugin.vcproj (right):

http://codereview.chromium.org/20378/diff/1/124#newcode212
Line 212: Name="VCCLCompilerTool"
I don't understand vcproj files, can you get someone else to look at this part?

http://codereview.chromium.org/20378/diff/1/21
File media/base/buffers.h (left):

http://codereview.chromium.org/20378/diff/1/21#oldcode30
Line 30: #include "base/basictypes.h"
needed for DISALLOW_COPY_AND_ASSIGN.

http://codereview.chromium.org/20378/diff/1/29
File net/base/upload_data.h (left):

http://codereview.chromium.org/20378/diff/1/29#oldcode11
Line 11: #include "base/basictypes.h"
needed for uint64.

http://codereview.chromium.org/20378/diff/1/32
File net/disk_cache/addr.h (left):

http://codereview.chromium.org/20378/diff/1/32#oldcode11
Line 11: #include "base/basictypes.h"
needed for uint32.

http://codereview.chromium.org/20378/diff/1/26
File net/url_request/url_request.cc (right):

http://codereview.chromium.org/20378/diff/1/26#newcode12
Line 12: #include "googleurl/src/gurl.h"
remove gurl.h, already in url_request.h

http://codereview.chromium.org/20378/diff/1/25
File net/url_request/url_request.h (left):

http://codereview.chromium.org/20378/diff/1/25#oldcode11
Line 11: #include "base/basictypes.h"
needed for DISALLOW_EVIL_CONSTRUCTORS.

http://codereview.chromium.org/20378/diff/1/25#oldcode17
Line 17: #include "net/base/ssl_info.h"
Needed around line 306.

http://codereview.chromium.org/20378/diff/1/87
File net/url_request/url_request_http_job.h (left):

http://codereview.chromium.org/20378/diff/1/87#oldcode8
Line 8: #include <string>
don't we need this?

http://codereview.chromium.org/20378/diff/1/87
File net/url_request/url_request_http_job.h (right):

http://codereview.chromium.org/20378/diff/1/87#newcode12
Line 12: #include "net/base/completion_callback.h"
typo?

http://codereview.chromium.org/20378/diff/1/39
File net/url_request/url_request_job.h (left):

http://codereview.chromium.org/20378/diff/1/39#oldcode11
Line 11: #include "base/basictypes.h"
needed for DISALLOW_COPY_AND_ASSIGN.

[Lei Zhang, 2009-02-17 20:16:36]

http://codereview.chromium.org/20378/diff/1/59
File webkit/glue/alt_error_page_resource_fetcher.cc (right):

http://codereview.chromium.org/20378/diff/1/59#newcode17
Line 17: #include "webkit/glue/webdatasource.h"
We don't use WebDataSource here.

http://codereview.chromium.org/20378/diff/1/114
File webkit/glue/alt_error_page_resource_fetcher.h (left):

http://codereview.chromium.org/20378/diff/1/114#oldcode16
Line 16: #include "base/basictypes.h"
needed for DISALLOW_EVIL_CONSTRUCTORS

http://codereview.chromium.org/20378/diff/1/10
File webkit/glue/password_form_dom_manager.h (left):

http://codereview.chromium.org/20378/diff/1/10#oldcode12
Line 12: #include <vector>
We use these in this header file.

http://codereview.chromium.org/20378/diff/1/36
File webkit/glue/resource_loader_bridge.h (left):

http://codereview.chromium.org/20378/diff/1/36#oldcode22
Line 22: #include "base/basictypes.h"
needed for DISALLOW_EVIL_CONSTRUCTORS.

http://codereview.chromium.org/20378/diff/1/17
File webkit/glue/webdatasource_impl.h (left):

http://codereview.chromium.org/20378/diff/1/17#oldcode32
Line 32: #include "base/basictypes.h"
needed for DISALLOW_EVIL_CONSTRUCTORS.

http://codereview.chromium.org/20378/diff/1/11
File webkit/glue/webframe.h (left):

http://codereview.chromium.org/20378/diff/1/11#oldcode10
Line 10: #include "base/basictypes.h"
needed for DISALLOW_COPY_AND_ASSIGN.

http://codereview.chromium.org/20378/diff/1/12
File webkit/glue/webframe_impl.h (left):

http://codereview.chromium.org/20378/diff/1/12#oldcode31
Line 31: #include "base/basictypes.h"
needed for DISALLOW_COPY_AND_ASSIGN.

http://codereview.chromium.org/20378/diff/1/6
File webkit/glue/webhistoryitem.h (left):

http://codereview.chromium.org/20378/diff/1/6#oldcode10
Line 10: #include "base/basictypes.h"
needed for DISALLOW_EVIL_CONSTRUCTORS.

http://codereview.chromium.org/20378/diff/1/6#oldcode11
Line 11: #include "base/ref_counted.h"
Don't we use it on line 16?

http://codereview.chromium.org/20378/diff/1/4
File webkit/glue/webhistoryitem_impl.h (left):

http://codereview.chromium.org/20378/diff/1/4#oldcode8
Line 8: #include "base/basictypes.h"
needed for DISALLOW_EVIL_CONSTRUCTORS.

http://codereview.chromium.org/20378/diff/1/7
File webkit/glue/webplugin_delegate.h (left):

http://codereview.chromium.org/20378/diff/1/7#oldcode11
Line 11: #include "base/basictypes.h"
needed for DISALLOW_EVIL_CONSTRUCTORS.

http://codereview.chromium.org/20378/diff/1/15
File webkit/glue/weburlrequest_impl.h (left):

http://codereview.chromium.org/20378/diff/1/15#oldcode66
Line 66: scoped_refptr<ExtraData> extra_data_;
Include "base/ref_counted.h"

http://codereview.chromium.org/20378/diff/1/18
File webkit/glue/webview_delegate.h (left):

http://codereview.chromium.org/20378/diff/1/18#oldcode32
Line 32: #include "base/basictypes.h"
Needed for DISALLOW_COPY_AND_ASSIGN.

http://codereview.chromium.org/20378/diff/1/18#oldcode34
Line 34: #include "googleurl/src/gurl.h"
Forward declare GURL and WindowOpenDisposition?

[Lei Zhang, 2009-02-18 03:51:11]

Amazingly, the mega patch still applies after 4 days. I found the fix for the
link error.

http://codereview.chromium.org/20378/diff/1/31
File net/base/client_socket.h (right):

http://codereview.chromium.org/20378/diff/1/31#newcode9
Line 9: #include "net/base/socket.h"
to get it to compile on Linux, you need to add:

#if defined(OS_LINUX)
#include <sys/socket.h>
#endif

Otherwise gcc will think sockaddr refers to net::sockaddr.

[M-A Ruel, 2009-02-18 15:35:22]

Updated snapshot. I didn't comment on the file that I removed and that you said
that should be kept because they were already included by dependencies. Due to
the fact that we don't have real distributed builds and we don't do strict
includes anyway, I don't think it's worth keeping "dangling unneeded includes".

I renamed a few DISALLOW_EVIL_CONTRUCTORS to DISALLOW_COPY_AND_ASSIGN. I added 3
new files.

http://codereview.chromium.org/20378/diff/1/63
File chrome/browser/chrome_plugin_host.cc (left):

http://codereview.chromium.org/20378/diff/1/63#oldcode43
Line 43: #include "base/string_util.h"
On 2009/02/17 19:09:41, Lei Zhang wrote:
> needed for StringToLowerASCII().

It was there 2 times (!) I just sorted the includes.

http://codereview.chromium.org/20378/diff/1/120
File chrome/browser/memory_details.cc (left):

http://codereview.chromium.org/20378/diff/1/120#oldcode20
Line 20: #include "chrome/browser/tab_contents/web_contents.h"
On 2009/02/17 19:09:41, Lei Zhang wrote:
> Shouldn't we get rid of web_contents.h (not used) and keep tab_contents.h?

It's used.

http://codereview.chromium.org/20378/diff/1/97
File chrome/browser/profile_manager.cc (right):

http://codereview.chromium.org/20378/diff/1/97#newcode22
Line 22: #include "net/http/http_transaction_factory.h"
On 2009/02/17 19:09:41, Lei Zhang wrote:
> Why do we need this?

Line 265.

http://codereview.chromium.org/20378/diff/1/124
File chrome/plugin/plugin.vcproj (right):

http://codereview.chromium.org/20378/diff/1/124#newcode212
Line 212: Name="VCCLCompilerTool"
On 2009/02/17 19:50:48, Lei Zhang wrote:
> I don't understand vcproj files, can you get someone else to look at this
part?

It says that for the Release configuration, it is Excluded From Build. This file
is solely to generate the precompiled header.

http://codereview.chromium.org/20378/diff/1/59
File webkit/glue/alt_error_page_resource_fetcher.cc (right):

http://codereview.chromium.org/20378/diff/1/59#newcode17
Line 17: #include "webkit/glue/webdatasource.h"
On 2009/02/17 20:16:37, Lei Zhang wrote:
> We don't use WebDataSource here.

Line 35.

http://codereview.chromium.org/20378/diff/1/18
File webkit/glue/webview_delegate.h (left):

http://codereview.chromium.org/20378/diff/1/18#oldcode34
Line 34: #include "googleurl/src/gurl.h"
On 2009/02/17 20:16:37, Lei Zhang wrote:
> Forward declare GURL and WindowOpenDisposition?

Already forward declared/included.

[Lei Zhang, 2009-02-18 19:32:05]

LGTM

[Dean McNamee, 2009-02-19 11:50:31]

Seems like in an effort to remove includes, we also now have some cases where
instead of the header including what it uses, it happens to work because some
random header it includes includes what it uses...  I'm not sure that's a great
thing.

http://codereview.chromium.org/20378/diff/1186/434
File chrome/browser/memory_details.h (left):

http://codereview.chromium.org/20378/diff/1186/434#oldcode9
Line 9: #include <string>
why did you remove string when it uses std::wstring, etc?

http://codereview.chromium.org/20378/diff/1186/379
File chrome/browser/net/dns_global.cc (right):

http://codereview.chromium.org/20378/diff/1186/379#newcode455
Line 455: GURL gurl = tab_start_pref.urls[i];
GURL is used here, but you removed the include.  Are you depending on an include
including GURL?  I tend to prefer including what you use in a .cc file, and not
depending on a .h to include it.

http://codereview.chromium.org/20378/diff/1186/446
File chrome/browser/renderer_host/render_process_host.h (right):

http://codereview.chromium.org/20378/diff/1186/446#newcode201
Line 201: DISALLOW_COPY_AND_ASSIGN(RenderProcessHost);
this requires basictypes.h, but you removed that include, now it just happens
it's getting pulled in by one of the others?