Update/improve base::File comments.

base/files/file.h

Index: base/files/file.h
diff --git a/base/files/file.h b/base/files/file.h
index 7b6366c1c2edaa2c6ccb695f2f075ff83f0d002b..8648e93ffcebe28fccf3811ea9401359776b0a9c 100644
--- a/base/files/file.h
+++ b/base/files/file.h
@@ -128,9 +128,8 @@ class BASE_EXPORT File {
 
   // Used to hold information about a given file.
   // If you add more fields to this structure (platform-specific fields are OK),
-  // make sure to update all functions that use it in file_util_{win|posix}.cc
-  // too, and the ParamTraits<base::PlatformFileInfo> implementation in
-  // chrome/common/common_param_traits.cc.

[rvargas (doing something else), 2015/03/13 18:37:01]

This is now ParamTraits<base::File::Info>... but I *think* there's nothing to be
updated over there (depends directly in this code), so sure, let's remove this.

[Thiemo Nagel, 2015/03/16 16:28:39]

I couldn't find it and thought it had been removed entirely, but now that you
say that it still exists, I'd just update the reference.

+  // make sure to update all functions that use it in file_util_{win|posix}.cc,
+  // too.
   struct BASE_EXPORT Info {
     Info();
     ~Info();
@@ -142,10 +141,11 @@ class BASE_EXPORT File {
     // The size of the file in bytes.  Undefined when is_directory is true.
     int64 size;
 
-    // True if the file corresponds to a directory.
+    // True if the file corresponds to a directory.  Always false on Windows.

[rvargas (doing something else), 2015/03/13 18:37:01]

I don't think this is true.

[rvargas (doing something else), 2015/03/13 18:44:20]

To be clear, if the File points to a directory, GetInfo should return true. I
think it does, it doesn't, it sounds like a bug.

[Thiemo Nagel, 2015/03/16 16:28:39]

I thought you couldn't even open a directory in Windows, i.e. IsValid() should
be false for base::File("c:\windows")?  So you can't even call GetInfo() on a
directory on Windows?

At least when I tried opening a directory on a Windows bot, it failed.  Also I
can't find code in file_win.cc which sets is_directory to true.  But maybe I'm
missing something crucial here?

[rvargas (doing something else), 2015/03/16 19:57:14]

It requires File::Flags::FLAG_BACKUP_SEMANTICS (to be renamed and made
multi-platform)
File::GetInfo() (line 287)

[Thiemo Nagel, 2015/03/17 19:48:35]

I have overlooked that.  :(

     bool is_directory;
 
-    // True if the file corresponds to a symbolic link.
+    // True if the file corresponds to a symbolic link.  Always false on
+    // Windows.

[rvargas (doing something else), 2015/03/13 18:37:00]

Reflects the current implementation, but may not be what we want from the
interface. I don't have a use case to fix the implementation though (which is
why I have never bothered). Where does it matter? (just curious)

[rvargas (doing something else), 2015/03/13 18:44:20]

(and I'm basically fine either way)

[Thiemo Nagel, 2015/03/16 16:28:39]

There is no specific use case.  It's just me trying to understand the properties
of base::File.  When an OS-independent abstraction contains parts that are
specific to only a subset of all supported OSes, I thought it might be useful to
document that.

[rvargas (doing something else), 2015/03/16 19:57:14]

Could you make the comment state that this is an implementation detail rather
than a contract (as in, it's easy enough to fix for whoever actually needs that
info in Windows). I'm not sure if what I'm saying makes sense... but if we ever
fix this I don't want to have to worry about current callers assuming it is
false because the doc says so.

[Thiemo Nagel, 2015/03/17 19:48:35]

Done.

     bool is_symbolic_link;
 
     // The last modified time of a file.