Fix plugin placeholders not loading

I have made a truly wonderful mess of WebViewPlugin, via:
https://chromium.googlesource.com/chromium/src/+/8489f1bf78c2c6f296e0df6c529751d29e8a0188
https://chromium.googlesource.com/chromium/src/+/8ed4bf69eca5e2696aeab731a8e9a15e8f379f2b

My fundamental misunderstanding (I think!) was regarding what
WebViewPlugin is listening to via the various interfaces it subclasses.
It uses WebFrameClient and WebViewClient to observe its internal
WebView, while it uses WebPlugin and RenderViewObserver to interact
with the real external page. In the patches above, I mixed and matched
messages from those 2 sources because I thought they were coming from
the same context.

This patch reverts the damage done in those patches, and restores
ReplayReceivedData, more-or-less the way it was before I
started breaking stuff. It then continues the work I started in
https://chromium.googlesource.com/chromium/src/+/8e1badd5fbc054cf4ffc9303e8cb0aa22ce55ca6
and moves the subclassing of WebViewClient to a private helper class.
This helper, renamed to WebViewHelper, now handles all callbacks coming
from the internal WebView (i.e., the "WebView" in "WebViewPlugin"), and
WebViewPlugin continues subclassing WebPlugin and RenderViewObserver
to observe the outside world. This will hopefully make it harder to
screw things up in the same way in the future.

BUG=675585

Review-Url: https://codereview.chromium.org/2600253003
Cr-Commit-Position: refs/heads/master@{#441401}

components/plugins/renderer/loadable_plugin_placeholder.cc

@@ -129,9 +129,8 @@ void LoadablePluginPlaceholder::ReplacePlugin(blink::WebPlugin* new_plugin) {
 
   container->invalidate();
   container->reportGeometry();
-  if (plugin()->focused())
-    new_plugin->updateFocus(true, blink::WebFocusTypeNone);
   container->element().setAttribute("title", plugin()->old_title());
+  plugin()->ReplayReceivedData(new_plugin);
   plugin()->destroy();
 }
 

components/plugins/renderer/webview_plugin.cc

@@ -53,21 +53,12 @@ WebViewPlugin::WebViewPlugin(content::RenderView* render_view,
     : content::RenderViewObserver(render_view),
       delegate_(delegate),
       container_(nullptr),
-      web_view_(WebView::create(this, blink::WebPageVisibilityStateVisible)),
+      finished_loading_(false),
       focused_(false),
       is_painting_(false),
       is_resizing_(false),
-      web_frame_client_(this),
+      web_view_helper_(this, preferences),
       weak_factory_(this) {
-  // ApplyWebPreferences before making a WebLocalFrame so that the frame sees a
-  // consistent view of our preferences.
-  content::RenderView::ApplyWebPreferences(preferences, web_view_);
-  WebLocalFrame* web_frame = WebLocalFrame::create(
-      blink::WebTreeScopeType::Document, &web_frame_client_);
-  web_view_->setMainFrame(web_frame);
-  // TODO(dcheng): The main frame widget currently has a special case.
-  // Eliminate this once WebView is no longer a WebWidget.
-  WebFrameWidget::create(this, web_view_, web_frame);
 }
 
 // static
@@ -84,7 +75,26 @@ WebViewPlugin* WebViewPlugin::Create(content::RenderView* render_view,
 
 WebViewPlugin::~WebViewPlugin() {
   DCHECK(!weak_factory_.HasWeakPtrs());
-  web_view_->close();
+}
+
+void WebViewPlugin::ReplayReceivedData(WebPlugin* plugin) {
+  if (!response_.isNull()) {
+    plugin->didReceiveResponse(response_);
+    size_t total_bytes = 0;
+    for (std::list<std::string>::iterator it = data_.begin(); it != data_.end();
+         ++it) {
+      plugin->didReceiveData(
+          it->c_str(), base::checked_cast<int>(it->length()));
+      total_bytes += it->length();
+    }
+  }
+  // We need to transfer the |focused_| to new plugin after it loaded.
+  if (focused_)
+    plugin->updateFocus(true, blink::WebFocusTypeNone);
+  if (finished_loading_)
+    plugin->didFinishLoading();
+  if (error_)
+    plugin->didFailLoading(*error_);
 }
 
 WebPluginContainer* WebViewPlugin::container() const { return container_; }
@@ -106,8 +116,8 @@ bool WebViewPlugin::initialize(WebPluginContainer* container) {
   old_title_ = container_->element().getAttribute("title");
 
   // Propagate device scale and zoom level to inner webview.
-  web_view_->setDeviceScaleFactor(container_->deviceScaleFactor());
-  web_view_->setZoomLevel(
+  web_view()->setDeviceScaleFactor(container_->deviceScaleFactor());
+  web_view()->setZoomLevel(
       blink::WebView::zoomFactorToZoomLevel(container_->pageZoomFactor()));
 
   return true;
@@ -133,7 +143,7 @@ v8::Local<v8::Object> WebViewPlugin::v8ScriptableObject(v8::Isolate* isolate) {
 }
 
 void WebViewPlugin::updateAllLifecyclePhases() {
-  web_view_->updateAllLifecyclePhases();
+  web_view()->updateAllLifecyclePhases();
 }
 
 void WebViewPlugin::paint(WebCanvas* canvas, const WebRect& rect) {
@@ -155,7 +165,7 @@ void WebViewPlugin::paint(WebCanvas* canvas, const WebRect& rect) {
       SkFloatToScalar(1.0 / container_->deviceScaleFactor());
   canvas->scale(inverse_scale, inverse_scale);
 
-  web_view_->paint(canvas, paint_rect);
+  web_view()->paint(canvas, paint_rect);
 
   canvas->restore();
 }
@@ -172,7 +182,7 @@ void WebViewPlugin::updateGeometry(const WebRect& window_rect,
 
   if (static_cast<gfx::Rect>(window_rect) != rect_) {
     rect_ = window_rect;
-    web_view_->resize(rect_.size());
+    web_view()->resize(rect_.size());
   }
 
   // Plugin updates are forbidden during Blink layout. Therefore,
@@ -209,77 +219,102 @@ blink::WebInputEventResult WebViewPlugin::handleInputEvent(
     return blink::WebInputEventResult::HandledSuppressed;
   }
   current_cursor_ = cursor;
-  blink::WebInputEventResult handled = web_view_->handleInputEvent(event);
+  blink::WebInputEventResult handled = web_view()->handleInputEvent(event);
   cursor = current_cursor_;
 
   return handled;
 }
 
 void WebViewPlugin::didReceiveResponse(const WebURLResponse& response) {
-  NOTREACHED();
+  DCHECK(response_.isNull());
+  response_ = response;
 }
 
 void WebViewPlugin::didReceiveData(const char* data, int data_length) {
-  NOTREACHED();
+  data_.push_back(std::string(data, data_length));
 }
 
 void WebViewPlugin::didFinishLoading() {
-  NOTREACHED();
+  DCHECK(!finished_loading_);
+  finished_loading_ = true;
 }
 
 void WebViewPlugin::didFailLoading(const WebURLError& error) {
-  NOTREACHED();
+  DCHECK(!error_.get());
+  error_.reset(new WebURLError(error));
 }
 
-bool WebViewPlugin::acceptsLoadDrops() { return false; }
+WebViewPlugin::WebViewHelper::WebViewHelper(
+    WebViewPlugin* plugin,
+    const WebPreferences& preferences) : plugin_(plugin) {
+  web_view_ =
+      WebView::create(this, blink::WebPageVisibilityStateVisible);
+  // ApplyWebPreferences before making a WebLocalFrame so that the frame sees a
+  // consistent view of our preferences.
+  content::RenderView::ApplyWebPreferences(preferences, web_view_);
+  WebLocalFrame* web_frame = WebLocalFrame::create(
+      blink::WebTreeScopeType::Document, this);
+  web_view_->setMainFrame(web_frame);
+  // TODO(dcheng): The main frame widget currently has a special case.
+  // Eliminate this once WebView is no longer a WebWidget.
+  WebFrameWidget::create(this, web_view_, web_frame);
+}
 
-void WebViewPlugin::setToolTipText(const WebString& text,
-                                   blink::WebTextDirection hint) {
-  if (container_)
-    container_->element().setAttribute("title", text);
+WebViewPlugin::WebViewHelper::~WebViewHelper() {
+  web_view_->close();
+}
+
+bool WebViewPlugin::WebViewHelper::acceptsLoadDrops() { return false; }
+
+void WebViewPlugin::WebViewHelper::setToolTipText(
+    const WebString& text,
+    blink::WebTextDirection hint) {
+  if (plugin_->container_)
+    plugin_->container_->element().setAttribute("title", text);
 }
 
-void WebViewPlugin::startDragging(blink::WebReferrerPolicy,
-                                  const WebDragData&,
-                                  WebDragOperationsMask,
-                                  const WebImage&,
-                                  const WebPoint&) {
+void WebViewPlugin::WebViewHelper::startDragging(blink::WebReferrerPolicy,
+                                                 const WebDragData&,
+                                                 WebDragOperationsMask,
+                                                 const WebImage&,
+                                                 const WebPoint&) {
   // Immediately stop dragging.
   DCHECK(web_view_->mainFrame()->isWebLocalFrame());
   web_view_->mainFrame()->toWebLocalFrame()->frameWidget()->
       dragSourceSystemDragEnded();
 }
 
-bool WebViewPlugin::allowsBrokenNullLayerTreeView() const {
+bool WebViewPlugin::WebViewHelper::allowsBrokenNullLayerTreeView() const {
   return true;
 }
 
-void WebViewPlugin::didInvalidateRect(const WebRect& rect) {
-  if (container_)
-    container_->invalidateRect(rect);
+void WebViewPlugin::WebViewHelper::didInvalidateRect(const WebRect& rect) {
+  if (plugin_->container_)
+    plugin_->container_->invalidateRect(rect);
 }
 
-void WebViewPlugin::didChangeCursor(const WebCursorInfo& cursor) {
-  current_cursor_ = cursor;
+void WebViewPlugin::WebViewHelper::didChangeCursor(
+    const WebCursorInfo& cursor) {
+  plugin_->current_cursor_ = cursor;
 }
 
-void WebViewPlugin::scheduleAnimation() {
+void WebViewPlugin::WebViewHelper::scheduleAnimation() {
   // Resizes must be self-contained: any lifecycle updating must
   // be triggerd from within the WebView or this WebViewPlugin.
   // This is because this WebViewPlugin is contained in another
   // Web View which may be in the middle of updating its lifecycle,
   // but after layout is done, and it is illegal to dirty earlier
   // lifecycle stages during later ones.
-  if (is_resizing_)
+  if (plugin_->is_resizing_)
     return;
-  if (container_) {
+  if (plugin_->container_) {
     // This should never happen; see also crbug.com/545039 for context.
-    CHECK(!is_painting_);
-    container_->scheduleAnimation();
+    DCHECK(!plugin_->is_painting_);
+    plugin_->container_->scheduleAnimation();
   }
 }
 
-void WebViewPlugin::PluginWebFrameClient::didClearWindowObject(
+void WebViewPlugin::WebViewHelper::didClearWindowObject(
     WebLocalFrame* frame) {
   if (!plugin_->delegate_)
     return;
@@ -296,11 +331,9 @@ void WebViewPlugin::PluginWebFrameClient::didClearWindowObject(
               plugin_->delegate_->GetV8Handle(isolate));
 }
 
-void WebViewPlugin::OnDestruct() {}
-
 void WebViewPlugin::OnZoomLevelChanged() {
   if (container_) {
-    web_view_->setZoomLevel(
+    web_view()->setZoomLevel(
       blink::WebView::zoomFactorToZoomLevel(container_->pageZoomFactor()));
   }
 }
@@ -317,5 +350,5 @@ void WebViewPlugin::UpdatePluginForNewGeometry(
   // The delegate may have dirtied style and layout of the WebView.
   // See for example the resizePoster function in plugin_poster.html.
   // Run the lifecycle now so that it is clean.
-  web_view_->updateAllLifecyclePhases();
+  web_view()->updateAllLifecyclePhases();
 }

components/plugins/renderer/webview_plugin.h

@@ -36,7 +36,6 @@ struct WebPreferences;
 // chrome:// URL as origin.
 
 class WebViewPlugin : public blink::WebPlugin,
-                      public blink::WebViewClient,
                       public content::RenderViewObserver {
  public:
   class Delegate {
@@ -67,11 +66,15 @@ class WebViewPlugin : public blink::WebPlugin,
                                const std::string& html_data,
                                const GURL& url);
 
-  blink::WebView* web_view() { return web_view_; }
+  blink::WebView* web_view() { return web_view_helper_.web_view(); }
 
-  bool focused() const { return focused_; }
   const blink::WebString& old_title() const { return old_title_; }
 
+  // When loading a plugin document (i.e. a full page plugin not embedded in
+  // another page), we save all data that has been received, and replay it with
+  // this method on the actual plugin.
+  void ReplayReceivedData(blink::WebPlugin* plugin);
+
   // WebPlugin methods:
   blink::WebPluginContainer* container() const override;
   // The WebViewPlugin, by design, never fails to initialize. It's used to
@@ -103,27 +106,6 @@ class WebViewPlugin : public blink::WebPlugin,
   void didFinishLoading() override;
   void didFailLoading(const blink::WebURLError& error) override;
 
-  // WebViewClient methods:
-  bool acceptsLoadDrops() override;
-
-  void setToolTipText(const blink::WebString&,
-                      blink::WebTextDirection) override;
-
-  void startDragging(blink::WebReferrerPolicy,
-                     const blink::WebDragData&,
-                     blink::WebDragOperationsMask,
-                     const blink::WebImage&,
-                     const blink::WebPoint&) override;
-
-  // TODO(ojan): Remove this override and have this class use a non-null
-  // layerTreeView.
-  bool allowsBrokenNullLayerTreeView() const override;
-
-  // WebWidgetClient methods:
-  void didInvalidateRect(const blink::WebRect&) override;
-  void didChangeCursor(const blink::WebCursorInfo& cursor) override;
-  void scheduleAnimation() override;
-
  private:
   friend class base::DeleteHelper<WebViewPlugin>;
   WebViewPlugin(content::RenderView* render_view,
@@ -132,7 +114,7 @@ class WebViewPlugin : public blink::WebPlugin,
   ~WebViewPlugin() override;
 
   // content::RenderViewObserver methods:
-  void OnDestruct() override;
+  void OnDestruct() override {}
   void OnZoomLevelChanged() override;
 
   void UpdatePluginForNewGeometry(const blink::WebRect& window_rect,
@@ -146,27 +128,55 @@ class WebViewPlugin : public blink::WebPlugin,
   // Owns us.
   blink::WebPluginContainer* container_;
 
-  // Owned by us, deleted via |close()|.
-  blink::WebView* web_view_;
-
   gfx::Rect rect_;
 
+  blink::WebURLResponse response_;
+  std::list<std::string> data_;
+  std::unique_ptr<blink::WebURLError> error_;
   blink::WebString old_title_;
+  bool finished_loading_;
   bool focused_;
   bool is_painting_;
   bool is_resizing_;
 
-  // A helper needed to create a WebLocalFrame.
-  class PluginWebFrameClient : public blink::WebFrameClient {
+  // A helper that handles interaction from WebViewPlugin's internal WebView.
+  class WebViewHelper : public blink::WebViewClient,
+                        public blink::WebFrameClient {
    public:
-    PluginWebFrameClient(WebViewPlugin* plugin) : plugin_(plugin) {}
-    ~PluginWebFrameClient() override {}
+    WebViewHelper(WebViewPlugin* plugin,
+                  const content::WebPreferences& preferences);
+    ~WebViewHelper() override;
+
+    blink::WebView* web_view() { return web_view_; }
+
+    // WebViewClient methods:
+    bool acceptsLoadDrops() override;
+
+    // WebWidgetClient methods:
+    void setToolTipText(const blink::WebString&,
+                        blink::WebTextDirection) override;
+    void startDragging(blink::WebReferrerPolicy,
+                       const blink::WebDragData&,
+                       blink::WebDragOperationsMask,
+                       const blink::WebImage&,
+                       const blink::WebPoint&) override;
+    // TODO(ojan): Remove this override and have this class use a non-null
+    // layerTreeView.
+    bool allowsBrokenNullLayerTreeView() const override;
+    void didInvalidateRect(const blink::WebRect&) override;
+    void didChangeCursor(const blink::WebCursorInfo& cursor) override;
+    void scheduleAnimation() override;
+
+    // WebFrameClient methods:
     void didClearWindowObject(blink::WebLocalFrame* frame) override;
 
    private:
     WebViewPlugin* plugin_;
+
+    // Owned by us, deleted via |close()|.
+    blink::WebView* web_view_;
   };
-  PluginWebFrameClient web_frame_client_;
+  WebViewHelper web_view_helper_;
 
   // Should be invalidated when destroy() is called.
   base::WeakPtrFactory<WebViewPlugin> weak_factory_;