Set needsLayout on WebViewPlugin's container when initialized

components/plugins/renderer/webview_plugin.cc

 // Copyright 2013 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/plugins/renderer/webview_plugin.h"
 
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/numerics/safe_conversions.h"
 #include "content/public/common/web_preferences.h"
@@ skipping 93 matching lines @@
 
 void WebViewPlugin::RestoreTitleText() {
   if (container_)
     container_->element().setAttribute("title", old_title_);
 }
 
 WebPluginContainer* WebViewPlugin::container() const { return container_; }
 
 bool WebViewPlugin::initialize(WebPluginContainer* container) {
   container_ = container;
   if (container_) {

[chrishtr, 2015/07/21 20:43:58]

Please add a comment summarizing why setNeedsLayout needs to be called here.

[Stephen Chennney, 2015/07/21 21:03:11]

Done.

+    container_->setNeedsLayout();

[chrishtr, 2015/07/21 20:43:58]

This call is definitely needed. Here is what can happen otherwise:

1. The plugin WebView is created in WebViewPlugin::Create.
2. Lifecycle methods on the plugin WebView happen that cause it to call
WebWidget::scheduleAnimation() on the WebWidget which contains the WebView. In
this case the WebWidget is the WebViewPlugin class, so
WebViewPlugin::scheduleAnimation is called.
3. However, when WebViewPlugin::scheduleAnimation is called, container_ is still
null since initialize has not yet been called. So the scheduleAnimation is
dropped on the floor [very bad!]
4. WebPlugin::initialize() is called on the WebViewPlugin by the parent frame of
the plugin. This sets container_, and now we can propagate the scheduleAnimation
upward to the parent frame, so we need to do that at this time, hence this CL.

These steps above can happen in various orders, since the steps to initialize a
document involve timers that exist in Blink and don't have a specified order.
e.g. initialize() is called from FrameView::updateWidgets, which is in turn
called by a timer callback.

Unfortunately I was not able to get things to happen in the right order to cause
state to be invalid, but definitely dropping a scheduleAnimate on the floor is
incorrect.

[Stephen Chennney, 2015/07/21 21:03:11]

Acknowledged.

     old_title_ = container_->element().getAttribute("title");
 
     // Propagate device scale to inner webview to load the correct resource
     // when images have a "srcset" attribute.
     web_view_->setDeviceScaleFactor(container_->deviceScaleFactor());
   }
   return true;
 }
 
 void WebViewPlugin::destroy() {
@@ skipping 154 matching lines @@
 
   global->Set(gin::StringToV8(isolate, "plugin"),
               delegate_->GetV8Handle(isolate));
 }
 
 void WebViewPlugin::didReceiveResponse(WebLocalFrame* frame,
                                        unsigned identifier,
                                        const WebURLResponse& response) {
   WebFrameClient::didReceiveResponse(frame, identifier, response);
 }