Base: A few more files using BASE_API (for base.dll)

Base: A few more files using BASE_API (for base.dll)

BUG=76996
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=79426

[wtc, 2011-03-25 17:35:42]

LGTM.  Please note my comment marked with "IMPORTANT" below.

http://codereview.chromium.org/6733031/diff/1/base/base_paths.h
File base/base_paths.h (right):

http://codereview.chromium.org/6733031/diff/1/base/base_paths.h#newcode12
base/base_paths.h:12: #include "build/build_config.h"
I suspect that basictypes.h already includes build_config.h.
Also, the current recommendation is to list build_config.h
in normal alphabetical order, and then list the platform-
specific headers.  So we should say

#include "base/basictypes.h"
#include "build/build_config.h"
#if defined(OS_WIN)
#include "base/base_paths_win.h"
#elif defined(OS_MACOSX)
#include "base/base_paths_mac.h"
#endif

http://codereview.chromium.org/6733031/diff/1/base/cpu.h
File base/cpu.h (right):

http://codereview.chromium.org/6733031/diff/1/base/cpu.h#newcode12
base/cpu.h:12: #include <string>
This header should not need to include build_config.h.
So we should say

#include <string>

#include "base/base_api.h"

http://codereview.chromium.org/6733031/diff/1/base/file_util_unittest.cc
File base/file_util_unittest.cc (right):

http://codereview.chromium.org/6733031/diff/1/base/file_util_unittest.cc#newc...
base/file_util_unittest.cc:165: std::wofstream file;
IMPORTANT: this seems like a bug fix.  Since there is no
BASE DLL related change to this file, I suggest you separate
this bug fix into another CL.

http://codereview.chromium.org/6733031/diff/1/base/file_util_unittest.cc#newc...
base/file_util_unittest.cc:178: file.getline(contents, 64);
Nit: we should replace 64 with arraysize(contents).

http://codereview.chromium.org/6733031/diff/1/base/platform_file.h
File base/platform_file.h (right):

http://codereview.chromium.org/6733031/diff/1/base/platform_file.h#newcode21
base/platform_file.h:21: class FilePath;
Remove this forward declaration of FilePath because we
already include "base/file_path.h".

[rvargas (doing something else), 2011-03-25 17:59:56]

Done, thanks.

http://codereview.chromium.org/6733031/diff/1/base/base_paths.h
File base/base_paths.h (right):

http://codereview.chromium.org/6733031/diff/1/base/base_paths.h#newcode12
base/base_paths.h:12: #include "build/build_config.h"
On 2011/03/25 17:35:42, wtc wrote:
> I suspect that basictypes.h already includes build_config.h.
> Also, the current recommendation is to list build_config.h
> in normal alphabetical order, and then list the platform-
> specific headers.  So we should say
> 
> #include "base/basictypes.h"
> #include "build/build_config.h"
> #if defined(OS_WIN)
> #include "base/base_paths_win.h"
> #elif defined(OS_MACOSX)
> #include "base/base_paths_mac.h"
> #endif

I guess I should not be touching this file at all. I guess I changed it before
realizing that basictypes depends on build_config. I'll just leave it alone (it
needs either build_config or basic_types, but not both).

http://codereview.chromium.org/6733031/diff/1/base/cpu.h
File base/cpu.h (right):

http://codereview.chromium.org/6733031/diff/1/base/cpu.h#newcode12
base/cpu.h:12: #include <string>
On 2011/03/25 17:35:42, wtc wrote:
> This header should not need to include build_config.h.
> So we should say
> 
> #include <string>
> 
> #include "base/base_api.h"

Done.