Add FILE* constructors to RtcEventLogOutputFile

And deprecate PlatformFile constructor.

Bug: webrtc:6463
Change-Id: I18cef28bcc78d776611494d17be992e1319194d6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/135120
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27858}

logging/BUILD.gn

@@ -239,6 +239,7 @@ rtc_source_set("rtc_event_log_impl_output") {
     "../api:libjingle_logging_api",
     "../rtc_base:checks",
     "../rtc_base:rtc_base_approved",
+    "../rtc_base/system:file_wrapper",
   ]
 }
 

logging/rtc_event_log/output/rtc_event_log_output_file.cc

@@ -9,6 +9,7 @@
  */
 
 #include <limits>
+#include <utility>
 
 #include "logging/rtc_event_log/output/rtc_event_log_output_file.h"
 #include "logging/rtc_event_log/rtc_event_log.h"
@@ -17,6 +18,17 @@
 
 namespace webrtc {
 
+namespace {
+
+FileWrapper FileWrapperFromPlatformFile(rtc::PlatformFile platform_file) {
+  if (platform_file == rtc::kInvalidPlatformFileValue)
+    return FileWrapper();
+
+  return FileWrapper(rtc::FdopenPlatformFileForWriting(platform_file));
+}
+
+}  // namespace
+
 // Together with the assumption of no single Write() would ever be called on
 // an input with length greater-than-or-equal-to (max(size_t) / 2), this
 // guarantees no overflow of the check for remaining file capacity in Write().
@@ -25,45 +37,33 @@ const size_t RtcEventLogOutputFile::kMaxReasonableFileSize =
     std::numeric_limits<size_t>::max() / 2;
 
 RtcEventLogOutputFile::RtcEventLogOutputFile(const std::string& file_name)
-    : RtcEventLogOutputFile(file_name, RtcEventLog::kUnlimitedOutput) {}
+    : RtcEventLogOutputFile(FileWrapper::OpenWriteOnly(file_name),
+                            RtcEventLog::kUnlimitedOutput) {}
 
 RtcEventLogOutputFile::RtcEventLogOutputFile(const std::string& file_name,
                                              size_t max_size_bytes)
 
-    // Unlike plain fopen, CreatePlatformFile takes care of filename utf8 ->
-    // wchar conversion on windows.
-    : RtcEventLogOutputFile(rtc::CreatePlatformFile(file_name),
+    // Unlike plain fopen, FileWrapper takes care of filename utf8 ->
+    // wchar conversion on Windows.
+    : RtcEventLogOutputFile(FileWrapper::OpenWriteOnly(file_name),
                             max_size_bytes) {}
 
-RtcEventLogOutputFile::RtcEventLogOutputFile(rtc::PlatformFile file)
-    : RtcEventLogOutputFile(file, RtcEventLog::kUnlimitedOutput) {}
+RtcEventLogOutputFile::RtcEventLogOutputFile(FILE* file, size_t max_size_bytes)
+    : RtcEventLogOutputFile(FileWrapper(file), max_size_bytes) {}
+
+RtcEventLogOutputFile::RtcEventLogOutputFile(FileWrapper file,
+                                             size_t max_size_bytes)
+    : max_size_bytes_(max_size_bytes), file_(std::move(file)) {
+  RTC_CHECK_LE(max_size_bytes_, kMaxReasonableFileSize);
+  if (!file_.is_open()) {
+    RTC_LOG(LS_ERROR) << "Invalid file. WebRTC event log not started.";
+  }
+}
 
 RtcEventLogOutputFile::RtcEventLogOutputFile(rtc::PlatformFile platform_file,
                                              size_t max_size_bytes)
-    : max_size_bytes_(max_size_bytes) {
-  RTC_CHECK_LE(max_size_bytes_, kMaxReasonableFileSize);
-
-  // Handle errors from the CreatePlatformFile call in above constructor.
-  if (platform_file == rtc::kInvalidPlatformFileValue) {
-    RTC_LOG(LS_ERROR) << "Invalid file. WebRTC event log not started.";
-    return;
-  }
-  file_ = rtc::FdopenPlatformFileForWriting(platform_file);
-  if (!file_) {
-    RTC_LOG(LS_ERROR) << "Can't open file. WebRTC event log not started.";
-    // Even though we failed to open a FILE*, the file is still open
-    // and needs to be closed.
-    if (!rtc::ClosePlatformFile(platform_file)) {
-      RTC_LOG(LS_ERROR) << "Can't close file.";
-    }
-  }
-}
-
-RtcEventLogOutputFile::~RtcEventLogOutputFile() {
-  if (file_) {
-    fclose(file_);
-  }
-}
+    : RtcEventLogOutputFile(FileWrapperFromPlatformFile(platform_file),
+                            max_size_bytes) {}
 
 bool RtcEventLogOutputFile::IsActive() const {
   return IsActiveInternal();
@@ -77,7 +77,7 @@ bool RtcEventLogOutputFile::Write(const std::string& output) {
 
   if (max_size_bytes_ == RtcEventLog::kUnlimitedOutput ||
       written_bytes_ + output.length() <= max_size_bytes_) {
-    if (fwrite(output.c_str(), 1, output.size(), file_) == output.size()) {
+    if (file_.Write(output.c_str(), output.size())) {
       written_bytes_ += output.size();
       return true;
     } else {
@@ -88,14 +88,13 @@ bool RtcEventLogOutputFile::Write(const std::string& output) {
   }
 
   // Failed, for one of above reasons. Close output file.
-  fclose(file_);
-  file_ = nullptr;
+  file_.Close();
   return false;
 }
 
 // Internal non-virtual method.
 bool RtcEventLogOutputFile::IsActiveInternal() const {
-  return file_ != nullptr;
+  return file_.is_open();
 }
 
 }  // namespace webrtc

logging/rtc_event_log/output/rtc_event_log_output_file.h

@@ -17,6 +17,7 @@
 
 #include "api/rtc_event_log_output.h"
 #include "rtc_base/platform_file.h"  // Can't neatly forward PlatformFile.
+#include "rtc_base/system/file_wrapper.h"
 
 namespace webrtc {
 
@@ -28,17 +29,23 @@ class RtcEventLogOutputFile final : public RtcEventLogOutput {
   explicit RtcEventLogOutputFile(const std::string& file_name);
   RtcEventLogOutputFile(const std::string& file_name, size_t max_size_bytes);
 
-  // Unlimited/limited-size output file (by file handle).
-  explicit RtcEventLogOutputFile(rtc::PlatformFile file);
+  // Limited-size output file (by FILE*). This class takes ownership
+  // of the FILE*, and closes it on destruction.
+  RtcEventLogOutputFile(FILE* file, size_t max_size_bytes);
+
+  // TODO(bugs.webrtc.org/6463): Deprecated, delete together with the
+  // corresponding PeerConnection::StartRtcEventLog override.
   RtcEventLogOutputFile(rtc::PlatformFile file, size_t max_size_bytes);
 
-  ~RtcEventLogOutputFile() override;
+  ~RtcEventLogOutputFile() override = default;
 
   bool IsActive() const override;
 
   bool Write(const std::string& output) override;
 
  private:
+  RtcEventLogOutputFile(FileWrapper file, size_t max_size_bytes);
+
   // IsActive() can be called either from outside or from inside, but we don't
   // want to incur the overhead of a virtual function call if called from inside
   // some other function of this class.
@@ -47,7 +54,7 @@ class RtcEventLogOutputFile final : public RtcEventLogOutput {
   // Maximum size, or zero for no limit.
   const size_t max_size_bytes_;
   size_t written_bytes_{0};
-  FILE* file_{nullptr};
+  FileWrapper file_;
 };
 
 }  // namespace webrtc