Upstream misc changes for android.

Upstream misc changes for android.


BUG=
TEST=

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

[michaelbai, 2011-08-23 22:51:30]

Hi Brett,

Please help to review it.

[brettw, 2011-08-24 20:46:58]

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc
File base/path_service_unittest.cc (right):

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc#n...
base/path_service_unittest.cc:59: for (int key = base::DIR_CURRENT; key <
base::PATH_END; ++key) {
Why doesn't this code work on Android? It seems like this existing loop handles
all of your above cases.

http://codereview.chromium.org/7701014/diff/1/base/process_util.cc
File base/process_util.cc (right):

http://codereview.chromium.org/7701014/diff/1/base/process_util.cc#newcode69
base/process_util.cc:69: if (executable_name_.size() > 15)
Use {} for multiple-line conditionals.

http://codereview.chromium.org/7701014/diff/1/base/third_party/nspr/prtime.cc
File base/third_party/nspr/prtime.cc (right):

http://codereview.chromium.org/7701014/diff/1/base/third_party/nspr/prtime.cc...
base/third_party/nspr/prtime.cc:76: #include "base/time.h"  // For timegm()
I'm surprised you have to include Chrome's time.h from this shared code, and I
don't see anything in our time.h that defines timegm.

[michaelbai, 2011-08-24 23:02:18]

PTAL

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc
File base/path_service_unittest.cc (right):

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc#n...
base/path_service_unittest.cc:59: for (int key = base::DIR_CURRENT; key <
base::PATH_END; ++key) {
On 2011/08/24 20:46:58, brettw wrote:
> Why doesn't this code work on Android? It seems like this existing loop
handles
> all of your above cases.

The FILE_MODULE is not implemented in Android.

http://codereview.chromium.org/7701014/diff/1/base/process_util.cc
File base/process_util.cc (right):

http://codereview.chromium.org/7701014/diff/1/base/process_util.cc#newcode69
base/process_util.cc:69: if (executable_name_.size() > 15)
On 2011/08/24 20:46:58, brettw wrote:
> Use {} for multiple-line conditionals.

Done.

http://codereview.chromium.org/7701014/diff/1/base/third_party/nspr/prtime.cc
File base/third_party/nspr/prtime.cc (right):

http://codereview.chromium.org/7701014/diff/1/base/third_party/nspr/prtime.cc...
base/third_party/nspr/prtime.cc:76: #include "base/time.h"  // For timegm()
On 2011/08/24 20:46:58, brettw wrote:
> I'm surprised you have to include Chrome's time.h from this shared code, and I
> don't see anything in our time.h that defines timegm.

Sorry, it seemed I should upload more files, timegm() is actually in
base/os_compat_android.h which was included in base/time.h. But I don't think it
was right. I remvoed the os_compat_android.h from base/time.h and directly
include it here.

Let me know whether it LGTY

[brettw, 2011-08-24 23:17:13]

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc
File base/path_service_unittest.cc (right):

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc#n...
base/path_service_unittest.cc:59: for (int key = base::DIR_CURRENT; key <
base::PATH_END; ++key) {
It seems like the correct solution then is to implement the FILE_MODULE getter.

http://codereview.chromium.org/7701014/diff/5001/base/third_party/nspr/prtime.cc
File base/third_party/nspr/prtime.cc (right):

http://codereview.chromium.org/7701014/diff/5001/base/third_party/nspr/prtime...
base/third_party/nspr/prtime.cc:76: #include "base/time.h"  // For timegm()
I don't think you actually updated this like you said you did. Maybe you forgot
to upload?

[michaelbai, 2011-08-25 17:26:16]

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc
File base/path_service_unittest.cc (right):

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc#n...
base/path_service_unittest.cc:59: for (int key = base::DIR_CURRENT; key <
base::PATH_END; ++key) {
On 2011/08/24 23:17:13, brettw wrote:
> It seems like the correct solution then is to implement the FILE_MODULE
getter.

I could implement the FILE_MODULE, but the issue is that we used the different
directory in test. We set the DIR_MODULE to /data/local/tmp directory in test,
but I can't set FILE_MODULE to a file in /data/local/tmp which could be empty.

Another way, I could set FILE_MODULE to a file which doesn't locate in
DIR_MODULE. Do we have assumption that FILE_MODULE is in DIR_MODULE directory?

http://codereview.chromium.org/7701014/diff/5001/base/third_party/nspr/prtime.cc
File base/third_party/nspr/prtime.cc (right):

http://codereview.chromium.org/7701014/diff/5001/base/third_party/nspr/prtime...
base/third_party/nspr/prtime.cc:76: #include "base/time.h"  // For timegm()
On 2011/08/24 23:17:13, brettw wrote:
> I don't think you actually updated this like you said you did. Maybe you
forgot
> to upload?

Here it is, thanks

[brettw, 2011-08-25 17:49:10]

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc
File base/path_service_unittest.cc (right):

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc#n...
base/path_service_unittest.cc:59: for (int key = base::DIR_CURRENT; key <
base::PATH_END; ++key) {
I guess we can leave FILE_MODULE unimplemented. In that case, I think it would
be better to explicitly filter that one out in the loop:
  #if defined(OS_ANDROID)
    if (key == base::FILE_MODULE)
      continue;  // Android doesn't implement FILE_MODULE;
  #endif

[michaelbai, 2011-08-25 18:09:52]

PTAL

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc
File base/path_service_unittest.cc (right):

http://codereview.chromium.org/7701014/diff/1/base/path_service_unittest.cc#n...
base/path_service_unittest.cc:59: for (int key = base::DIR_CURRENT; key <
base::PATH_END; ++key) {
On 2011/08/25 17:49:10, brettw wrote:
> I guess we can leave FILE_MODULE unimplemented. In that case, I think it would
> be better to explicitly filter that one out in the loop:
>   #if defined(OS_ANDROID)
>     if (key == base::FILE_MODULE)
>       continue;  // Android doesn't implement FILE_MODULE;
>   #endif

Done.

[brettw, 2011-08-25 19:23:06]

LGTM