Require WebLocalFrame to be created with a non-null client

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 7fa310fbb04278eb36ee355b8e1e7079046d808b..7e9df81aac57f41ec92577098edb65509a6baf92 100644
--- a/components/printing/renderer/print_web_view_helper.cc
+++ b/components/printing/renderer/print_web_view_helper.cc
@@ -527,11 +527,11 @@ void PrintWebViewHelper::PrintHeaderAndFooter(
       blink::WebView::create(nullptr, blink::WebPageVisibilityStateVisible);
   web_view->settings()->setJavaScriptEnabled(true);
 
-  blink::WebLocalFrame* frame =
-      blink::WebLocalFrame::create(blink::WebTreeScopeType::Document, NULL);
+  blink::WebFrameClient frame_client;

[esprehn, 2016/09/29 04:28:42]

This scares me, the WebLocalFrame(Impl) below is garbage collected, so it can
live longer than this function if someone passes a reference to it, but the
client on the stack dies immediately after this function returns. What makes us
think this is safe? Should there be a static default WebFrameClient?

[dcheng, 2016/09/29 04:42:38]

When a frame is detached, it nulls out the client pointer, so there won't be a
dangling pointer.

We may want to consider using the static default WebFrameClient in the future,
but that's a non-trivial change.

+  blink::WebLocalFrame* frame = blink::WebLocalFrame::create(
+      blink::WebTreeScopeType::Document, &frame_client);
   web_view->setMainFrame(frame);
-  blink::WebFrameWidget* widget =
-      blink::WebFrameWidget::create(nullptr, web_view, frame);
+  blink::WebFrameWidget::create(nullptr, web_view, frame);
 
   base::StringValue html(ResourceBundle::GetSharedInstance().GetLocalizedString(
       IDR_PRINT_PREVIEW_PAGE));
@@ -561,9 +561,7 @@ void PrintWebViewHelper::PrintHeaderAndFooter(
   frame->printPage(0, canvas);
   frame->printEnd();
 
-  widget->close();
   web_view->close();
-  frame->close();
 }
 #endif  // defined(ENABLE_PRINT_PREVIEW)
 
@@ -620,14 +618,13 @@ class PrepareFrameAndViewForPrint : public blink::WebViewClient,
   bool allowsBrokenNullLayerTreeView() const override;
 
   // blink::WebFrameClient:
-  blink::WebFrame* createChildFrame(
+  blink::WebLocalFrame* createChildFrame(
       blink::WebLocalFrame* parent,
       blink::WebTreeScopeType scope,
       const blink::WebString& name,
       const blink::WebString& unique_name,
       blink::WebSandboxFlags sandbox_flags,
       const blink::WebFrameOwnerProperties& frame_owner_properties) override;
-  void frameDetached(blink::WebLocalFrame* frame, DetachType type) override;
 
   void CallOnReady();
   void ResizeForPrinting();
@@ -775,26 +772,18 @@ void PrepareFrameAndViewForPrint::didStopLoading() {
                             weak_ptr_factory_.GetWeakPtr()));
 }
 
-blink::WebFrame* PrepareFrameAndViewForPrint::createChildFrame(
+blink::WebLocalFrame* PrepareFrameAndViewForPrint::createChildFrame(
     blink::WebLocalFrame* parent,
     blink::WebTreeScopeType scope,
     const blink::WebString& name,
     const blink::WebString& unique_name,
     blink::WebSandboxFlags sandbox_flags,
     const blink::WebFrameOwnerProperties& frame_owner_properties) {
-  blink::WebFrame* frame = blink::WebLocalFrame::create(scope, this);
+  blink::WebLocalFrame* frame = blink::WebLocalFrame::create(scope, this);
   parent->appendChild(frame);
   return frame;
 }
 
-void PrepareFrameAndViewForPrint::frameDetached(blink::WebLocalFrame* frame,
-                                                DetachType type) {
-  DCHECK(type == DetachType::Remove);
-  if (frame->parent())
-    frame->parent()->removeChild(frame);
-  frame->close();
-}
-
 void PrepareFrameAndViewForPrint::CallOnReady() {
   return on_ready_.Run();  // Can delete |this|.
 }