Check for PrintWebViewHelper validity when running nested message loops.

components/printing/renderer/print_web_view_helper.cc

Index: components/printing/renderer/print_web_view_helper.cc
diff --git a/components/printing/renderer/print_web_view_helper.cc b/components/printing/renderer/print_web_view_helper.cc
index e343eef72d49ef5518ae771d499f9e28bfa8d73e..df858dfc756af42ef4b7ea9c48266f367062f1f9 100644
--- a/components/printing/renderer/print_web_view_helper.cc
+++ b/components/printing/renderer/print_web_view_helper.cc
@@ -12,6 +12,7 @@
 #include <utility>
 
 #include "base/auto_reset.h"
+#include "base/callback_helpers.h"
 #include "base/json/json_writer.h"
 #include "base/location.h"
 #include "base/logging.h"
@@ -521,6 +522,33 @@ PrintMsg_Print_Params CalculatePrintParamsForCss(
   return result_params;
 }
 
+void SendScriptedPrint(
+    PrintWebViewHelper* print_helper,
+    const PrintHostMsg_ScriptedPrint_Params& params,
+    blink::WebPrintScalingOption scaling_option,
+    const PrintWebViewHelper::GetPrintSettingsFromUserCallback& done_callback) {
+  PrintMsg_PrintPages_Params print_settings;
+  auto msg = base::MakeUnique<PrintHostMsg_ScriptedPrint>(
+      print_helper->routing_id(), params, &print_settings);
+  msg->EnableMessagePumping();
+
+  // WARNING: |print_helper| may no longer be valid when Send() returns.
+  print_helper->Send(msg.release());
+  print_settings.params.print_scaling_option = scaling_option;
+  done_callback.Run(print_settings);
+}
+
+void SendSetupScriptedPrintPreview(PrintWebViewHelper* print_helper,
+                                   const base::Closure& done_callback) {
+  auto msg = base::MakeUnique<PrintHostMsg_SetupScriptedPrintPreview>(
+      print_helper->routing_id());
+  msg->EnableMessagePumping();
+
+  // WARNING: |print_helper| may no longer be valid when Send() returns.
+  print_helper->Send(msg.release());
+  done_callback.Run();
+}
+
 }  // namespace
 
 FrameReference::FrameReference(blink::WebLocalFrame* frame) {
@@ -964,12 +992,17 @@ void PrintWebViewHelper::ScriptedPrint(bool user_initiated) {
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
     print_preview_context_.InitWithFrame(web_frame);
     RequestPrintPreview(PRINT_PREVIEW_SCRIPTED);
+    // WARNING: |this| may be gone at this point. Do not do any more work here
+    // and just return.
 #endif
-  } else {
+    return;
+  }
 #if BUILDFLAG(ENABLE_BASIC_PRINTING)
-    Print(web_frame, blink::WebNode(), true /* is_scripted? */);
+  StartPrint(web_frame, blink::WebNode(), true /* is_scripted? */,
+             false /* reset_print_node_in_progress? */);
+  // WARNING: |this| may be gone at this point. Do not do any more work here and
+  // just return.
 #endif
-  }
 }
 
 bool PrintWebViewHelper::OnMessageReceived(const IPC::Message& message) {
@@ -1018,7 +1051,10 @@ void PrintWebViewHelper::OnPrintPages() {
   // If we are printing a PDF extension frame, find the plugin node and print
   // that instead.
   auto plugin = delegate_->GetPdfElement(frame);
-  Print(frame, plugin, false /* is_scripted? */);
+  StartPrint(frame, plugin, false /* is_scripted? */,
+             false /* reset_print_node_in_progress? */);
+  // WARNING: |this| may be gone at this point. Do not do any more work here and
+  // just return.
 }
 
 void PrintWebViewHelper::OnPrintForSystemDialog() {
@@ -1029,7 +1065,10 @@ void PrintWebViewHelper::OnPrintForSystemDialog() {
     NOTREACHED();
     return;
   }
-  Print(frame, print_preview_context_.source_node(), false);
+  StartPrint(frame, print_preview_context_.source_node(), false,
+             false /* reset_print_node_in_progress? */);
+  // WARNING: |this| may be gone at this point. Do not do any more work here and
+  // just return.
 }
 #endif  // BUILDFLAG(ENABLE_BASIC_PRINTING)
 
@@ -1418,32 +1457,44 @@ void PrintWebViewHelper::PrintNode(const blink::WebNode& node) {
 
   print_node_in_progress_ = true;
 
-  // Make a copy of the node, in case RenderView::OnContextMenuClosed resets
-  // its |context_menu_node_|.
   if (g_is_preview_enabled) {
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
     print_preview_context_.InitWithNode(node);
     RequestPrintPreview(PRINT_PREVIEW_USER_INITIATED_CONTEXT_NODE);
 #endif
-  } else {
-#if BUILDFLAG(ENABLE_BASIC_PRINTING)
-    blink::WebNode duplicate_node(node);
-    Print(duplicate_node.document().frame(), duplicate_node, false);
-#endif
+    print_node_in_progress_ = false;
+    return;
   }
 
-  print_node_in_progress_ = false;
+#if BUILDFLAG(ENABLE_BASIC_PRINTING)
+  // Make a copy of the node, in case RenderView::OnContextMenuClosed() resets
+  // its |context_menu_node_|.
+  blink::WebNode duplicate_node(node);
+  StartPrint(duplicate_node.document().frame(), duplicate_node,
+             false /* is_scripted? */,
+             true /* reset_print_node_in_progress? */);
+  // WARNING: |this| may be gone at this point. Do not do any more work here and
+  // just return.
+#endif
 }
 
 #if BUILDFLAG(ENABLE_BASIC_PRINTING)
-void PrintWebViewHelper::Print(blink::WebLocalFrame* frame,
-                               const blink::WebNode& node,
-                               bool is_scripted) {
+void PrintWebViewHelper::StartPrint(blink::WebLocalFrame* frame,
+                                    const blink::WebNode& node,
+                                    bool is_scripted,
+                                    bool reset_print_node_in_progress) {
+  std::unique_ptr<base::ScopedClosureRunner> on_finish_callback;
+  if (reset_print_node_in_progress) {
+    on_finish_callback = base::MakeUnique<base::ScopedClosureRunner>(
+        base::Bind(&PrintWebViewHelper::print_node_in_progress_finished,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+
   // If still not finished with earlier print request simply ignore.
   if (prep_frame_view_)
     return;
 
-  FrameReference frame_ref(frame);
+  auto frame_ref = base::MakeUnique<FrameReference>(frame);
 
   int expected_page_count = 0;
   if (!CalculateNumberOfPages(frame, node, &expected_page_count)) {
@@ -1457,16 +1508,23 @@ void PrintWebViewHelper::Print(blink::WebLocalFrame* frame,
     return;
   }
 
-  // Ask the browser to show UI to retrieve the final print settings.
-  if (delegate_->IsAskPrintSettingsEnabled() &&
-      !GetPrintSettingsFromUser(frame_ref.GetFrame(), node, expected_page_count,
-                                is_scripted)) {
-    DidFinishPrinting(OK);  // Release resources and fail silently.
+  // If the browser does not want to show the UI, then just go ahead and print.
+  if (!delegate_->IsAskPrintSettingsEnabled()) {
+    FinishPrint(std::move(frame_ref), node);
     return;
   }
 
+  // Ask the browser to show UI to retrieve the final print settings.
+  GetPrintSettingsFromUser(std::move(frame_ref), node, expected_page_count,
+                           is_scripted, std::move(on_finish_callback));
+  // WARNING: |this| may be gone at this point. Do not do any more work here and
+  // just return.
+}
+
+void PrintWebViewHelper::FinishPrint(std::unique_ptr<FrameReference> frame_ref,
+                                     const blink::WebNode& node) {
   // Render Pages for printing.
-  if (!RenderPagesForPrint(frame_ref.GetFrame(), node)) {
+  if (!RenderPagesForPrint(frame_ref->GetFrame(), node)) {
     LOG(ERROR) << "RenderPagesForPrint failed";
     DidFinishPrinting(FAIL_PRINT);
   }
@@ -1732,54 +1790,65 @@ bool PrintWebViewHelper::UpdatePrintSettings(
 
   SetPrintPagesParams(settings);
 
-  if (!PrintMsg_Print_Params_IsValid(settings.params)) {
-    if (!print_for_preview_)
-      print_preview_context_.set_error(PREVIEW_ERROR_INVALID_PRINTER_SETTINGS);
-    else
-      Send(new PrintHostMsg_ShowInvalidPrinterSettingsError(routing_id()));
-
-    return false;
-  }
+  if (PrintMsg_Print_Params_IsValid(settings.params))
+    return true;
 
-  return true;
+  if (print_for_preview_)
+    Send(new PrintHostMsg_ShowInvalidPrinterSettingsError(routing_id()));
+  else
+    print_preview_context_.set_error(PREVIEW_ERROR_INVALID_PRINTER_SETTINGS);
+  return false;
 }
 #endif  // BUILDFLAG(ENABLE_PRINT_PREVIEW)
 
 #if BUILDFLAG(ENABLE_BASIC_PRINTING)
-bool PrintWebViewHelper::GetPrintSettingsFromUser(blink::WebLocalFrame* frame,
-                                                  const blink::WebNode& node,
-                                                  int expected_pages_count,
-                                                  bool is_scripted) {
+void PrintWebViewHelper::GetPrintSettingsFromUser(
+    std::unique_ptr<FrameReference> frame_ref,
+    const blink::WebNode& node,
+    int expected_pages_count,
+    bool is_scripted,
+    std::unique_ptr<base::ScopedClosureRunner> on_finish_callback) {
+  blink::WebLocalFrame* frame = frame_ref->GetFrame();
   PrintHostMsg_ScriptedPrint_Params params;
-  PrintMsg_PrintPages_Params print_settings;
-
   params.cookie = print_pages_params_->params.document_cookie;
   params.has_selection = frame->hasSelection();
   params.expected_pages_count = expected_pages_count;
   MarginType margin_type = DEFAULT_MARGINS;
-  if (PrintingNodeOrPdfFrame(frame, node)) {
-    margin_type =
-        GetMarginsForPdf(frame, node, print_pages_params_->params);
-  }
+  if (PrintingNodeOrPdfFrame(frame, node))
+    margin_type = GetMarginsForPdf(frame, node, print_pages_params_->params);
   params.margin_type = margin_type;
   params.is_scripted = is_scripted;
   params.is_modifiable = !PrintingNodeOrPdfFrame(frame, node);
 
   Send(new PrintHostMsg_DidShowPrintDialog(routing_id()));
 
-  // PrintHostMsg_ScriptedPrint will reset print_scaling_option, so we save the
-  // value before and restore it afterwards.
+  // PrintHostMsg_ScriptedPrint in SendScriptedPrint() will reset
+  // |print_scaling_option|, so save the value before and restore it afterwards.
   blink::WebPrintScalingOption scaling_option =
       print_pages_params_->params.print_scaling_option;
 
   print_pages_params_.reset();
-  IPC::SyncMessage* msg =
-      new PrintHostMsg_ScriptedPrint(routing_id(), params, &print_settings);
-  msg->EnableMessagePumping();
-  Send(msg);
-  print_settings.params.print_scaling_option = scaling_option;
+
+  GetPrintSettingsFromUserCallback ask_user_done_callback =
+      base::Bind(&PrintWebViewHelper::OnGetPrintSettingsFromUser,
+                 weak_ptr_factory_.GetWeakPtr(), base::Passed(&frame_ref), node,
+                 base::Passed(&on_finish_callback));
+  SendScriptedPrint(this, params, scaling_option, ask_user_done_callback);
+  // WARNING: |this| may be gone at this point. Do not do any more work here and
+  // just return. Put any work that needs to be done in |done_callback| above.
+}
+
+void PrintWebViewHelper::OnGetPrintSettingsFromUser(
+    std::unique_ptr<FrameReference> frame_ref,
+    const blink::WebNode& node,
+    std::unique_ptr<base::ScopedClosureRunner> on_finish_callback,
+    const PrintMsg_PrintPages_Params& print_settings) {
   SetPrintPagesParams(print_settings);
-  return (print_settings.params.dpi && print_settings.params.document_cookie);
+  if (print_settings.params.dpi && print_settings.params.document_cookie) {
+    FinishPrint(std::move(frame_ref), node);
+    return;
+  }
+  DidFinishPrinting(OK);  // Release resources and fail silently on failure.
 }
 
 bool PrintWebViewHelper::RenderPagesForPrint(blink::WebLocalFrame* frame,
@@ -1943,11 +2012,13 @@ void PrintWebViewHelper::RequestPrintPreview(PrintPreviewRequestType type) {
             FROM_HERE, base::Bind(&PrintWebViewHelper::ShowScriptedPrintPreview,
                                   weak_ptr_factory_.GetWeakPtr()));
       }
-      IPC::SyncMessage* msg =
-          new PrintHostMsg_SetupScriptedPrintPreview(routing_id());
-      msg->EnableMessagePumping();
-      Send(msg);

[lfg, 2016/11/24 05:25:47]

Wouldn't it be simpler to do:

auto ptr = weak_ptr_factory_.GetWeakPtr();
Send();
if(!ptr)
  return;

[Lei Zhang, 2016/11/24 07:46:34]

Yes, thank you.

-      is_scripted_preview_delayed_ = false;
+      base::Closure done_callback =
+          base::Bind(&PrintWebViewHelper::reset_scripted_preview_delayed,
+                     weak_ptr_factory_.GetWeakPtr());
+      SendSetupScriptedPrintPreview(this, done_callback);
+      // WARNING: |this| may be gone at this point. Do not do any more work
+      // here and just return. Put any work that needs to be done in
+      // |done_callback| above.
       return;
     }
     case PRINT_PREVIEW_USER_INITIATED_ENTIRE_FRAME: {