Add PrintedDocument::HasDebugDumpPath().

Unlike the current checks for the debug dump path, HasDebugDumpPath()
is truly read only and won't create the base::FilePath when accessing
the |g_debug_dump_info| base::LazyInstance.

Update code to use HasDebugDumpPath() instead of accessing
|g_debug_dump_info| directly, and only do that when getting / setting
the path value.

Change-Id: I200664a9cb70289883627c8dc7f85c6f02edd45b
Reviewed-on: https://chromium-review.googlesource.com/821370
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523356}

chrome/browser/chrome_browser_main.cc

@@ -1708,7 +1708,8 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   if (parsed_command_line().HasSwitch(switches::kDebugPrint)) {
     base::FilePath path =
         parsed_command_line().GetSwitchValuePath(switches::kDebugPrint);
-    printing::PrintedDocument::set_debug_dump_path(path);
+    if (!path.empty())
+      printing::PrintedDocument::SetDebugDumpPath(path);
   }
 #endif  // BUILDFLAG(ENABLE_PRINT_PREVIEW) && !defined(OFFICIAL_BUILD)
 

chrome/browser/printing/print_view_manager_base.cc

@@ -219,7 +219,8 @@ void PrintViewManagerBase::UpdateForPrintedPage(
         reinterpret_cast<const unsigned char*>(shared_buf->memory()),
         shared_buf->mapped_size()));
 
-    document->DebugDumpData(bytes.get(), FILE_PATH_LITERAL(".pdf"));
+    if (PrintedDocument::HasDebugDumpPath())
+      document->DebugDumpData(bytes.get(), FILE_PATH_LITERAL(".pdf"));
 
     const auto& settings = document->settings();
     if (settings.printer_is_textonly()) {

printing/printed_document.cc

@@ -50,7 +50,7 @@ void DebugDumpPageTask(const base::string16& doc_name,
                        const PrintedPage* page) {
   base::AssertBlockingAllowed();
 
-  DCHECK(!g_debug_dump_info.Get().empty());
+  DCHECK(PrintedDocument::HasDebugDumpPath());
 
   static constexpr base::FilePath::CharType kExtension[] =
       FILE_PATH_LITERAL(".emf");
@@ -67,7 +67,7 @@ void DebugDumpTask(const base::string16& doc_name,
                    const MetafilePlayer* metafile) {
   base::AssertBlockingAllowed();
 
-  DCHECK(!g_debug_dump_info.Get().empty());
+  DCHECK(PrintedDocument::HasDebugDumpPath());
 
   static constexpr base::FilePath::CharType kExtension[] =
       FILE_PATH_LITERAL(".pdf");
@@ -124,7 +124,7 @@ PrintedDocument::PrintedDocument(const PrintSettings& settings,
     }
   }
 
-  if (!g_debug_dump_info.Get().empty())
+  if (HasDebugDumpPath())
     DebugDumpSettings(name, settings);
 }
 
@@ -146,7 +146,7 @@ void PrintedDocument::SetPage(int page_number,
     mutable_.pages_[page_number] = page;
   }
 
-  if (!g_debug_dump_info.Get().empty()) {
+  if (HasDebugDumpPath()) {
     base::PostTaskWithTraits(
         FROM_HERE, {base::TaskPriority::BACKGROUND, base::MayBlock()},
         base::BindOnce(&DebugDumpPageTask, name(), base::RetainedRef(page)));
@@ -177,7 +177,7 @@ void PrintedDocument::SetDocument(std::unique_ptr<MetafilePlayer> metafile,
 #endif
   }
 
-  if (!g_debug_dump_info.Get().empty()) {
+  if (HasDebugDumpPath()) {
     base::PostTaskWithTraits(
         FROM_HERE, {base::TaskPriority::BACKGROUND, base::MayBlock()},
         base::BindOnce(&DebugDumpTask, name(), mutable_.metafile_.get()));
@@ -235,16 +235,22 @@ int PrintedDocument::expected_page_count() const {
   return mutable_.expected_page_count_;
 }
 
-void PrintedDocument::set_debug_dump_path(
-    const base::FilePath& debug_dump_path) {
+// static
+void PrintedDocument::SetDebugDumpPath(const base::FilePath& debug_dump_path) {
+  DCHECK(!debug_dump_path.empty());
   g_debug_dump_info.Get() = debug_dump_path;
 }
 
+// static
+bool PrintedDocument::HasDebugDumpPath() {
+  return g_debug_dump_info.IsCreated();
+}
+
+// static
 base::FilePath PrintedDocument::CreateDebugDumpPath(
     const base::string16& document_name,
     const base::FilePath::StringType& extension) {
-  if (!g_debug_dump_info.Get().empty())
-    return base::FilePath();
+  DCHECK(HasDebugDumpPath());
 
   // Create a filename.
   base::string16 filename;
@@ -259,15 +265,15 @@ base::FilePath PrintedDocument::CreateDebugDumpPath(
   system_filename = base::UTF16ToUTF8(filename);
 #endif  // OS_WIN
   base::i18n::ReplaceIllegalCharactersInPath(&system_filename, '_');
-  return g_debug_dump_info.Get().Append(system_filename).AddExtension(
-      extension);
+  const auto& dump_path = g_debug_dump_info.Get();
+  DCHECK(!dump_path.empty());
+  return dump_path.Append(system_filename).AddExtension(extension);
 }
 
 void PrintedDocument::DebugDumpData(
     const base::RefCountedMemory* data,
     const base::FilePath::StringType& extension) {
-  if (g_debug_dump_info.Get().empty())
-    return;
+  DCHECK(HasDebugDumpPath());
   base::PostTaskWithTraits(FROM_HERE,
                            {base::TaskPriority::BACKGROUND, base::MayBlock()},
                            base::BindOnce(&DebugDumpDataTask, name(), extension,

printing/printed_document.h

@@ -101,18 +101,22 @@ class PRINTING_EXPORT PrintedDocument
   const base::string16& name() const { return immutable_.name_; }
   int cookie() const { return immutable_.cookie_; }
 
-  // Sets a path where to dump printing output files for debugging. If never set
-  // no files are generated.
-  static void set_debug_dump_path(const base::FilePath& debug_dump_path);
+  // Sets a path where to dump printing output files for debugging. If never
+  // set, no files are generated. |debug_dump_path| must not be empty.
+  static void SetDebugDumpPath(const base::FilePath& debug_dump_path);
+
+  // Returns true if SetDebugDumpPath() has been called.
+  static bool HasDebugDumpPath();
 
   // Creates debug file name from given |document_name| and |extension|.
-  // |extension| should include '.', example ".pdf"
-  // Returns empty |base::FilePath| if debug dumps is not enabled.
+  // |extension| should include the leading dot. e.g. ".pdf"
+  // Should only be called when debug dumps are enabled.
   static base::FilePath CreateDebugDumpPath(
       const base::string16& document_name,
       const base::FilePath::StringType& extension);
 
-  // Dump data on blocking task runner if debug dumps enabled.
+  // Dump data on blocking task runner.
+  // Should only be called when debug dumps are enabled.
   void DebugDumpData(const base::RefCountedMemory* data,
                      const base::FilePath::StringType& extension);
 

printing/printing_context_win.cc

@@ -263,11 +263,12 @@ PrintingContext::Result PrintingContextWin::NewDocument(
   di.lpszDocName = document_name.c_str();
 
   // Is there a debug dump directory specified? If so, force to print to a file.
-  base::string16 debug_dump_path =
-      PrintedDocument::CreateDebugDumpPath(document_name,
-                                           FILE_PATH_LITERAL(".prn")).value();
-  if (!debug_dump_path.empty())
-    di.lpszOutput = debug_dump_path.c_str();
+  if (PrintedDocument::HasDebugDumpPath()) {
+    base::FilePath debug_dump_path = PrintedDocument::CreateDebugDumpPath(
+        document_name, FILE_PATH_LITERAL(".prn"));
+    if (!debug_dump_path.empty())
+      di.lpszOutput = debug_dump_path.value().c_str();
+  }
 
   // No message loop running in unit tests.
   DCHECK(!base::MessageLoop::current() ||