RenderWidget/RenderView: encapsulate ViewHostMsg_Show, etc, in a callback

content/renderer/render_view_impl.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 "content/renderer/render_view_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <memory>
 
@@ skipping 652 matching lines @@
                    blink::WebPopupTypeNone,
                    params.initial_size.screen_info,
                    params.swapped_out,
                    params.hidden,
                    params.never_visible),
       webkit_preferences_(params.web_preferences),
       send_content_state_immediately_(false),
       enabled_bindings_(0),
       send_preferred_size_changes_(false),
       navigation_gesture_(NavigationGestureUnknown),
-      opened_by_user_gesture_(true),
       history_list_offset_(-1),
       history_list_length_(0),
       frames_in_progress_(0),
       target_url_status_(TARGET_NONE),
       uses_temporary_zoom_level_(false),
 #if defined(OS_ANDROID)
       top_controls_constraints_(BROWSER_CONTROLS_STATE_BOTH),
 #endif
       browser_controls_shrink_blink_size_(false),
       top_controls_height_(0.f),
       webview_(nullptr),
       has_scrolled_focused_editable_node_into_rect_(false),
       page_zoom_level_(params.page_zoom_level),
       main_render_frame_(nullptr),
       frame_widget_(nullptr),
       speech_recognition_dispatcher_(NULL),
 #if defined(OS_ANDROID)
       expected_content_intent_id_(0),
+      was_created_by_renderer_(false),
 #endif
       enumeration_completion_id_(0),
       session_storage_namespace_id_(params.session_storage_namespace_id),
       has_added_input_handler_(false) {
   GetWidget()->set_owner_delegate(this);
 }
 
-void RenderViewImpl::Initialize(const mojom::CreateViewParams& params,
-                                bool was_created_by_renderer) {
-  int opener_view_routing_id = MSG_ROUTING_NONE;
+void RenderViewImpl::Initialize(
+    const mojom::CreateViewParams& params,
+    const RenderWidget::ShowCallback& show_callback) {
+  bool was_created_by_renderer = !show_callback.is_null();
+#if defined(OS_ANDROID)
+  // TODO(sgurun): crbug.com/325351 Needed only for android webview's deprecated
+  // HandleNavigation.
+  was_created_by_renderer_ = was_created_by_renderer;
+#endif
   WebFrame* opener_frame = RenderFrameImpl::ResolveOpener(
-      params.opener_frame_route_id, &opener_view_routing_id);
-  if (!was_created_by_renderer)
-    opener_view_routing_id = MSG_ROUTING_NONE;
+      params.opener_frame_route_id, nullptr);
 
   display_mode_ = params.initial_size.display_mode;
 
   webview_ =
       WebView::create(this, is_hidden() ? blink::WebPageVisibilityStateHidden
                                         : blink::WebPageVisibilityStateVisible);
-  RenderWidget::Init(opener_view_routing_id, webview_->widget());
+  RenderWidget::Init(show_callback, webview_->widget());
 
   g_view_map.Get().insert(std::make_pair(webview(), this));
   g_routing_id_view_map.Get().insert(std::make_pair(GetRoutingID(), this));
 
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
 
   if (command_line.HasSwitch(switches::kStatsCollectionController))
     stats_collection_observer_.reset(new StatsCollectionObserver(this));
 
@@ skipping 71 matching lines @@
   content_detectors_.push_back(base::MakeUnique<AddressDetector>());
   const std::string& contry_iso =
       params.renderer_preferences.network_contry_iso;
   if (!contry_iso.empty()) {
     content_detectors_.push_back(
         base::MakeUnique<PhoneNumberDetector>(contry_iso));
   }
   content_detectors_.push_back(base::MakeUnique<EmailDetector>());
 #endif
 
-  // If this is a popup, we must wait for the CreatingNew_ACK message before
-  // completing initialization.  Otherwise, we can finish it now.
-  if (opener_id_ == MSG_ROUTING_NONE)
+  // If this is a popup, we won't be visible until we ask our opener (via

[alexmos, 2016/11/17 09:51:09]

nit: is it really correct to say "If this is a popup", since we need to call
show_callback in all the renderer-initiated cases, including fullscreen widgets
which aren't popups?  Perhaps enumerate the three cases that need this?

[ncarter (slow), 2016/11/17 19:33:11]

I amended the comment.

There are three widget-show cases, but when we go through
RenderViewImpl::Initialize we know we are in the popup case. The other two
widget-show cases are not RenderViews and don't go through
RenderView::Initialize; however, there are also browser-initiated
RenderView-creation cases that go through RenderView::Initialize with a null
show callback, so they aren't widget-show cases. This did_show_ assignment is
for the latter group.

Making it even more confusing, the other two widget-show cases do interact with
their opener RenderView, via the callback. It will hopefully be somewhat clearer
once these callbacks are moved to RenderFrame.

[alexmos, 2016/11/17 22:27:43]

Acknowledged.

+  // show_callback) to make us visible. Otherwise, we expect that show() will
+  // never be called.
+  if (!was_created_by_renderer)
     did_show_ = true;
 
   // Set the main frame's name.  Only needs to be done for WebLocalFrames,
   // since the remote case was handled as part of SetReplicatedState on the
   // proxy above.
   if (!params.replicated_frame_state.name.empty() &&
       webview()->mainFrame()->isWebLocalFrame()) {
     webview()->mainFrame()->setName(
         blink::WebString::fromUTF8(params.replicated_frame_state.name));
   }
@@ skipping 366 matching lines @@
   settings->setDoubleTapToZoomEnabled(true);
   web_view->setMaximumLegibleScale(prefs.default_maximum_page_scale_factor);
 #endif
 
 #if defined(OS_WIN)
   WebRuntimeFeatures::enableMiddleClickAutoscroll(true);
 #endif
 }
 
 /*static*/
-RenderViewImpl* RenderViewImpl::Create(CompositorDependencies* compositor_deps,
-                                       const mojom::CreateViewParams& params,
-                                       bool was_created_by_renderer) {
+RenderViewImpl* RenderViewImpl::Create(
+    CompositorDependencies* compositor_deps,
+    const mojom::CreateViewParams& params,
+    const RenderWidget::ShowCallback& show_callback) {
   DCHECK(params.view_id != MSG_ROUTING_NONE);
   RenderViewImpl* render_view = NULL;
   if (g_create_render_view_impl)
     render_view = g_create_render_view_impl(compositor_deps, params);
   else
     render_view = new RenderViewImpl(compositor_deps, params);
 
-  render_view->Initialize(params, was_created_by_renderer);
+  render_view->Initialize(params, show_callback);
   return render_view;
 }
 
 // static
 void RenderViewImpl::InstallCreateHook(RenderViewImpl* (
     *create_render_view_impl)(CompositorDependencies* compositor_deps,
                               const mojom::CreateViewParams&)) {
   CHECK(!g_create_render_view_impl);
   g_create_render_view_impl = create_render_view_impl;
 }
@@ skipping 245 matching lines @@
   DCHECK(!SiteIsolationPolicy::UseSubframeNavigationEntries());
 
   HistoryEntry* entry = history_controller_->GetCurrentEntry();
   if (!entry)
     return;
 
   Send(new ViewHostMsg_UpdateState(GetRoutingID(),
                                    HistoryEntryToPageState(entry)));
 }
 
+void RenderViewImpl::ShowCreatedPopupWidget(RenderWidget* popup_widget,
+                                            WebNavigationPolicy policy,
+                                            const gfx::Rect& initial_rect) {
+  Send(new ViewHostMsg_ShowWidget(GetRoutingID(), popup_widget->routing_id(),
+                                  initial_rect));
+}
+
+void RenderViewImpl::ShowCreatedFullscreenWidget(
+    RenderWidget* fullscreen_widget,
+    WebNavigationPolicy policy,
+    const gfx::Rect& initial_rect) {
+  Send(new ViewHostMsg_ShowFullscreenWidget(GetRoutingID(),
+                                            fullscreen_widget->routing_id()));
+}
+
+void RenderViewImpl::ShowCreatedViewWidget(bool opened_by_user_gesture,
+                                           RenderWidget* render_view_to_show,
+                                           WebNavigationPolicy policy,
+                                           const gfx::Rect& initial_rect) {
+  // |render_view_to_show| represents a pending view opened (e.g. a popup opened
+  // via window.open()) by this object, but not yet shown (it's offscreen, and
+  // still owned by the opener). Sending |ViewHostMsg_ShowView| will move it off
+  // the opener's pending list, into e.g. its own tab or window.
+  Send(new ViewHostMsg_ShowView(GetRoutingID(),
+                                render_view_to_show->routing_id(),
+                                NavigationPolicyToDisposition(policy),
+                                initial_rect, opened_by_user_gesture));
+}
+
 void RenderViewImpl::SendFrameStateUpdates() {
   // We only use this path in OOPIF-enabled modes.
   DCHECK(SiteIsolationPolicy::UseSubframeNavigationEntries());
 
   // Tell each frame with pending state to send its UpdateState message.
   for (int render_frame_routing_id : frames_with_pending_state_) {
     RenderFrameImpl* frame =
         RenderFrameImpl::FromRoutingID(render_frame_routing_id);
     if (frame)
       frame->SendUpdateState();
@@ skipping 112 matching lines @@
   // WebCore will take care of setting the correct name.
   view_params.replicated_frame_state = FrameReplicationState();
   view_params.hidden = is_background_tab;
   view_params.never_visible = never_visible;
   view_params.initial_size = initial_size;
   view_params.enable_auto_resize = false;
   view_params.min_size = gfx::Size();
   view_params.max_size = gfx::Size();
   view_params.page_zoom_level = page_zoom_level_;
 
+  RenderWidget::ShowCallback show_callback =
+      base::Bind(&RenderViewImpl::ShowCreatedViewWidget, this->AsWeakPtr(),
+                 opened_by_user_gesture);
+
   RenderViewImpl* view =
-      RenderViewImpl::Create(compositor_deps_, view_params, true);
-  view->opened_by_user_gesture_ = opened_by_user_gesture;
+      RenderViewImpl::Create(compositor_deps_, view_params, show_callback);
 
   return view->webview();
 }
 
 WebWidget* RenderViewImpl::createPopupMenu(blink::WebPopupType popup_type) {
-  RenderWidget* widget = RenderWidget::Create(GetRoutingID(), compositor_deps_,
-                                              popup_type, screen_info_);
+  RenderWidget* widget = RenderWidget::CreateForPopup(this, compositor_deps_,
+                                                      popup_type, screen_info_);
   if (!widget)
     return NULL;
   if (screen_metrics_emulator_) {
     widget->SetPopupOriginAdjustmentsForEmulation(
         screen_metrics_emulator_.get());
   }
   return widget->GetWebWidget();
 }
 
 WebStorageNamespace* RenderViewImpl::createSessionStorageNamespace() {
@@ skipping 280 matching lines @@
   // TODO(jcivelli): when https://bugs.webkit.org/show_bug.cgi?id=33389 is fixed
   //                 we won't have to test for user gesture anymore and we can
   //                 move that code back to render_widget.cc
   if (WebUserGestureIndicator::isProcessingUserGesture() &&
       !RenderThreadImpl::current()->layout_test_mode()) {
     Send(new ViewHostMsg_Focus(GetRoutingID()));
   }
 }
 
 // We are supposed to get a single call to Show for a newly created RenderView
 // that was created via RenderViewImpl::CreateWebView.  So, we wait until this

[alexmos, 2016/11/17 09:51:09]

Perhaps update this while you're here?

[ncarter (slow), 2016/11/17 19:33:11]

Done.

 // point to dispatch the ShowView message.
 //
 // This method provides us with the information about how to display the newly
 // created RenderView (i.e., as a blocked popup or as a new tab).
 //
 void RenderViewImpl::show(WebNavigationPolicy policy) {
-  if (did_show_) {
+  if (did_show_ && !webkit_preferences_.supports_multiple_windows) {

[alexmos, 2016/11/17 09:51:09]

Just to confirm, the NOTREACHED is gone because if we get here with did_show_
and supports_multiple_windows both being true, we'll hit the DCHECK in
RenderWidget::show instead?

[ncarter (slow), 2016/11/17 19:33:11]

Correct.

     // When supports_multiple_windows is disabled, popups are reusing
     // the same view. In some scenarios, this makes WebKit to call show() twice.
-    if (webkit_preferences_.supports_multiple_windows)
-      NOTREACHED() << "received extraneous Show call";
     return;
   }
-  did_show_ = true;
 
-  DCHECK(opener_id_ != MSG_ROUTING_NONE);
-
-  // NOTE: initial_rect_ may still have its default values at this point, but
-  // that's okay.  It'll be ignored if disposition is not NEW_POPUP, or the
-  // browser process will impose a default position otherwise.
-  Send(new ViewHostMsg_ShowView(opener_id_, GetRoutingID(),
-                                NavigationPolicyToDisposition(policy),
-                                initial_rect_, opened_by_user_gesture_));
-  SetPendingWindowRect(initial_rect_);
+  RenderWidget::show(policy);
 }
 
 void RenderViewImpl::onMouseDown(const WebNode& mouse_down_node) {
   for (auto& observer : observers_)
     observer.OnMouseDown(mouse_down_node);
 }
 
 void RenderViewImpl::didHandleGestureEvent(
     const WebGestureEvent& event,
     bool event_cancelled) {
@@ skipping 1022 matching lines @@
                             INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
   }
 
   std::unique_ptr<InputEventAck> ack(
       new InputEventAck(InputEventAckSource::MAIN_THREAD, input_event->type,
                         INPUT_EVENT_ACK_STATE_NOT_CONSUMED));
   OnInputEventAck(std::move(ack));
 }
 
 }  // namespace content