Add granular file tracing.

base/files/file.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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 {
 
 File::Info::Info()
     : size(0),
       is_directory(false),
       is_symbolic_link(false) {
 }
 
 File::Info::~Info() {
 }
 
 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
 
 File::File(PlatformFile platform_file)
     : file_(platform_file),
       error_details_(FILE_OK),
       created_(false),
       async_(false) {
 #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_;
   }
   return *this;
 }
 
 #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
 
 std::string File::ErrorToString(Error error) {
   switch (error) {
     case FILE_OK:
       return "FILE_OK";
     case FILE_ERROR_FAILED:
       return "FILE_ERROR_FAILED";
     case FILE_ERROR_IN_USE:
@@ skipping 27 matching lines @@
     case FILE_ERROR_IO:
       return "FILE_ERROR_IO";
     case FILE_ERROR_MAX:
       break;
   }
 
   NOTREACHED();
   return "";
 }
 
-bool File::Flush() {
-  ElapsedTimer timer;
-  bool return_value = DoFlush();
-  UMA_HISTOGRAM_TIMES("PlatformFile.FlushTime", timer.Elapsed());
-  return return_value;
-}
-
 }  // namespace base