Change IsPhysicalFile to DCHECK in ReadOnlyFile

Mojo serialization should not be blocking, but this CHECK included a
blocking call to Fstat. This prevents DisallowScopedBlocking consumers
from using this type. We now call fstat directly and while this
could block in practice this should be quick for already opened
physical files.

The remaining CHECK calls fcntl with F_GETFL which essentially just
copies an int from the kernel back to the process, so can be
considered non-blocking, and so left in place.

Bug: 1130762
Change-Id: Iced551675d5e8c74ac62b8bcbffc240dbde1aa95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466203Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817104}

mojo/public/cpp/base/file_unittest.cc

@@ -139,6 +139,7 @@ TEST(FileTest, ReadOnlyFileDeath) {
 
 // This should work on all platforms. This check might be relaxed in which case
 // this test can be removed.
+#if DCHECK_IS_ON()
 TEST(FileTest, NonPhysicalFileDeath) {
 #if defined(OFFICIAL_BUILD)
   const char kPhysicalFileCheckFailedRegex[] = "";
@@ -162,6 +163,7 @@ TEST(FileTest, NonPhysicalFileDeath) {
                                                                &file_out),
       kPhysicalFileCheckFailedRegex);
 }
+#endif  // DCHECK_IS_ON()
 
 }  // namespace file_unittest
 }  // namespace mojo_base

mojo/public/cpp/base/read_only_file_mojom_traits.cc

@@ -9,6 +9,8 @@
 
 #if defined(OS_POSIX) || defined(OS_FUCHSIA)
 #include <fcntl.h>
+#include <sys/stat.h>
+#include <unistd.h>
 #endif
 #if defined(OS_WIN)
 #include <windows.h>
@@ -64,10 +66,12 @@ bool IsPhysicalFile(base::File& file) {
   DWORD type = GetFileType(file.GetPlatformFile());
   return type == FILE_TYPE_DISK;
 #else
-  base::stat_wrapper_t stat;
-  if (base::File::Fstat(file.GetPlatformFile(), &stat) != 0)
+  // This may block but in practice this is unlikely for already opened
+  // physical files.
+  struct stat st;
+  if (fstat(file.GetPlatformFile(), &st) != 0)
     return false;
-  return S_ISREG(stat.st_mode);
+  return S_ISREG(st.st_mode);
 #endif
 }
 
@@ -78,8 +82,9 @@ mojo::PlatformHandle StructTraits<mojo_base::mojom::ReadOnlyFileDataView,
   DCHECK(file.IsValid());
   // For now we require real files as on some platforms it is too difficult to
   // be sure that more general handles cannot be written or made writable. This
-  // could be relaxed if an interface needs readonly pipes.
-  CHECK(IsPhysicalFile(file));
+  // could be relaxed if an interface needs readonly pipes. This check may block
+  // so cannot be enabled in release builds.
+  DCHECK(IsPhysicalFile(file));
   CHECK(IsReadOnlyFile(file));
 
   return mojo::PlatformHandle(