Require WebLocalFrame to be created with a non-null client

components/printing/renderer/print_web_view_helper.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/printing/renderer/print_web_view_helper.h"
 
 #include <stddef.h>
 #include <stdint.h>
 #include <string>
 #include <utility>
@@ skipping 509 matching lines @@
 
   blink::WebSize page_size(page_layout.margin_left + page_layout.margin_right +
                                page_layout.content_width,
                            page_layout.margin_top + page_layout.margin_bottom +
                                page_layout.content_height);
 
   blink::WebView* web_view =
       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));
   // Load page with script to avoid async operations.
   ExecuteScript(frame, kPageLoadScriptFormat, html);
 
   std::unique_ptr<base::DictionaryValue> options(new base::DictionaryValue());
   options.reset(new base::DictionaryValue());
   options->SetDouble(kSettingHeaderFooterDate, base::Time::Now().ToJsTime());
   options->SetDouble("width", page_size.width);
   options->SetDouble("height", page_size.height);
   options->SetDouble("topMargin", page_layout.margin_top);
   options->SetDouble("bottomMargin", page_layout.margin_bottom);
   options->SetString("pageNumber",
                      base::StringPrintf("%d/%d", page_number, total_pages));
 
   options->SetString("url", params.url);
   base::string16 title = source_frame.document().title();
   options->SetString("title", title.empty() ? params.title : title);
 
   ExecuteScript(frame, kPageSetupScriptFormat, *options);
 
   blink::WebPrintParams webkit_params(page_size);
   webkit_params.printerDPI = GetDPI(&params);
 
   frame->printBegin(webkit_params);
   frame->printPage(0, canvas);
   frame->printEnd();
 
-  widget->close();
   web_view->close();
-  frame->close();
 }
 #endif  // defined(ENABLE_PRINT_PREVIEW)
 
 // static - Not anonymous so that platform implementations can use it.
 float PrintWebViewHelper::RenderPageContent(blink::WebFrame* frame,
                                             int page_number,
                                             const gfx::Rect& canvas_area,
                                             const gfx::Rect& content_area,
                                             double scale_factor,
                                             blink::WebCanvas* canvas) {
@@ skipping 36 matching lines @@
   }
 
  private:
   // blink::WebViewClient:
   void didStopLoading() override;
   // TODO(ojan): Remove this override and have this class use a non-null
   // layerTreeView.
   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();
   void RestoreSize();
   void CopySelection(const WebPreferences& preferences);
 
   FrameReference frame_;
   blink::WebNode node_to_print_;
   bool owns_web_view_;
   blink::WebPrintParams web_print_params_;
@@ skipping 127 matching lines @@
 
 void PrepareFrameAndViewForPrint::didStopLoading() {
   DCHECK(!on_ready_.is_null());
   // Don't call callback here, because it can delete |this| and WebView that is
   // called didStopLoading.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(&PrepareFrameAndViewForPrint::CallOnReady,
                             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|.
 }
 
 void PrepareFrameAndViewForPrint::RestoreSize() {
   if (frame()) {
     blink::WebView* web_view = frame_.GetFrame()->view();
     web_view->resize(prev_view_size_);
     if (blink::WebFrame* web_frame = web_view->mainFrame())
       web_frame->setScrollOffset(prev_scroll_offset_);
@@ skipping 1453 matching lines @@
       blink::WebConsoleMessage::LevelWarning, message));
   return false;
 }
 
 void PrintWebViewHelper::ScriptingThrottler::Reset() {
   // Reset counter on successful print.
   count_ = 0;
 }
 
 }  // namespace printing