Add granular file tracing.

base/files/file.cc

Index: base/files/file.cc
diff --git a/base/files/file.cc b/base/files/file.cc
index 8030bf13734e8a6b20bfe4f8f45ccc738885e075..f5bd406f3b41fc305100bdcabbd3f6bc6ba016ad 100644
--- a/base/files/file.cc
+++ b/base/files/file.cc
@@ -4,8 +4,8 @@
 
 #include "base/files/file.h"
 #include "base/files/file_path.h"
+#include "base/files/file_tracing.h"
 #include "base/metrics/histogram.h"
-#include "base/timer/elapsed_timer.h"
 
 namespace base {
 
@@ -22,14 +22,14 @@ File::File()
     : error_details_(FILE_ERROR_FAILED),
       created_(false),
       async_(false) {
+  FILE_TRACING_BEGIN();

[oystein (OOO til 10th of July), 2015/05/01 18:21:05]

It could be cleaner to do these BEGIN/END calls in a class member instead of
duplicating them in all the constructors.

[Dan Beam, 2015/05/01 23:29:04]

Done.

 }
 
 #if !defined(OS_NACL)
-File::File(const FilePath& name, uint32 flags)
-    : error_details_(FILE_OK),
-      created_(false),
-      async_(false) {
-  Initialize(name, flags);
+File::File(const FilePath& path, uint32 flags)
+    : error_details_(FILE_OK), created_(false), async_(false) {
+  FILE_TRACING_BEGIN();
+  Initialize(path, flags);
 }
 #endif
 
@@ -41,30 +41,36 @@ File::File(PlatformFile platform_file)
 #if defined(OS_POSIX)
   DCHECK_GE(platform_file, -1);
 #endif
+  FILE_TRACING_BEGIN();
 }
 
 File::File(Error error_details)
     : error_details_(error_details),
       created_(false),
       async_(false) {
+  FILE_TRACING_BEGIN();
 }
 
 File::File(RValue other)
     : file_(other.object->TakePlatformFile()),
+      unsafe_path_(other.object->unsafe_path_),
       error_details_(other.object->error_details()),
       created_(other.object->created()),
       async_(other.object->async_) {
+  FILE_TRACING_BEGIN();
 }
 
 File::~File() {
   // Go through the AssertIOAllowed logic.
   Close();
+  FILE_TRACING_END();
 }
 
 File& File::operator=(RValue other) {
   if (this != other.object) {
     Close();
     SetPlatformFile(other.object->TakePlatformFile());
+    unsafe_path_ = other.object->unsafe_path_;
     error_details_ = other.object->error_details();
     created_ = other.object->created();
     async_ = other.object->async_;
@@ -73,12 +79,14 @@ File& File::operator=(RValue other) {
 }
 
 #if !defined(OS_NACL)
-void File::Initialize(const FilePath& name, uint32 flags) {
-  if (name.ReferencesParent()) {
+void File::Initialize(const FilePath& path, uint32 flags) {
+  unsafe_path_ = path;

[oystein (OOO til 10th of July), 2015/05/01 18:15:31]

What's the reason for setting this here, instead of setting it DoInitialize or
at least after the ReferencesParent() check? I'm worried about codepaths popping
up where the path gets used despite of Initialize() failing, not setting the
member at all seems like a good layered defense.

[Dan Beam, 2015/05/01 23:29:04]

Done. (originally my rationale was: it'd be weird to do thing.Initialize(path)
-> thing.GetPath() be different, but path_ is never exposed)

+  if (unsafe_path_.ReferencesParent()) {
     error_details_ = FILE_ERROR_ACCESS_DENIED;
     return;
   }
-  DoInitialize(name, flags);
+  SCOPED_FILE_TRACE("Initialize");
+  DoInitialize(flags);
 }
 #endif
 
@@ -126,11 +134,4 @@ std::string File::ErrorToString(Error error) {
   return "";
 }
 
-bool File::Flush() {
-  ElapsedTimer timer;
-  bool return_value = DoFlush();
-  UMA_HISTOGRAM_TIMES("PlatformFile.FlushTime", timer.Elapsed());
-  return return_value;
-}
-
 }  // namespace base