Browse Source

fix: use OOPIF for webview tag (#13869) (#14156)

* fix: use OOIF for webview tag

* fix: do not call GetNativeView for webview

* fix: OOIPF webview's WebContents is managed by embedder frame

* fix: guest view can not be focused

* fix: clear zoom controller when guest is destroyed

* fix: implement the webview resize event

The webview is no longer a browser plugin with the resize event, use
ResizeObserver instead.

* test: disable failed tests due to OOPIF webview

* fix: embedder can be destroyed earlier than guest

This happens when embedder is manually destroyed.

* fix: don't double attach

* fix: recreate iframe when webview is reattached

* fix: resize event may happen very early

* test: some tests are working after OOPIF webview

* chore: remove unused browser plugin webview code

* fix: get embedder via closure

When the "destroyed" event is emitted, the entry in guestInstances would be
cleared.

* chore: rename browserPluginNode to internalElement

* test: make the visibilityState test more robust

* chore: guestinstance can not work with OOPIF webview

* fix: element could be detached before got response from browser
Cheng Zhao 6 years ago
parent
commit
44b0245ac4

+ 32 - 58
atom/browser/api/atom_api_web_contents.cc

@@ -120,28 +120,6 @@ struct PrintSettings {
 
 namespace mate {
 
-template <>
-struct Converter<atom::SetSizeParams> {
-  static bool FromV8(v8::Isolate* isolate,
-                     v8::Local<v8::Value> val,
-                     atom::SetSizeParams* out) {
-    mate::Dictionary params;
-    if (!ConvertFromV8(isolate, val, &params))
-      return false;
-    bool autosize;
-    if (params.Get("enableAutoSize", &autosize))
-      out->enable_auto_size.reset(new bool(autosize));
-    gfx::Size size;
-    if (params.Get("min", &size))
-      out->min_size.reset(new gfx::Size(size));
-    if (params.Get("max", &size))
-      out->max_size.reset(new gfx::Size(size));
-    if (params.Get("normal", &size))
-      out->normal_size.reset(new gfx::Size(size));
-    return true;
-  }
-};
-
 template <>
 struct Converter<PrintSettings> {
   static bool FromV8(v8::Isolate* isolate,
@@ -396,7 +374,8 @@ WebContents::WebContents(v8::Isolate* isolate,
                                             GURL("chrome-guest://fake-host"));
     content::WebContents::CreateParams params(session->browser_context(),
                                               site_instance);
-    guest_delegate_.reset(new WebViewGuestDelegate);
+    guest_delegate_.reset(
+        new WebViewGuestDelegate(embedder_->web_contents(), this));
     params.guest_delegate = guest_delegate_.get();
 
 #if defined(ENABLE_OSR)
@@ -448,7 +427,7 @@ void WebContents::InitWithSessionAndOptions(v8::Isolate* isolate,
                                             mate::Handle<api::Session> session,
                                             const mate::Dictionary& options) {
   Observe(web_contents);
-  InitWithWebContents(web_contents, session->browser_context());
+  InitWithWebContents(web_contents, session->browser_context(), IsGuest());
 
   managed_web_contents()->GetView()->SetDelegate(this);
 
@@ -481,8 +460,6 @@ void WebContents::InitWithSessionAndOptions(v8::Isolate* isolate,
   web_contents->SetUserAgentOverride(GetBrowserContext()->GetUserAgent());
 
   if (IsGuest()) {
-    guest_delegate_->Initialize(this);
-
     NativeWindow* owner_window = nullptr;
     if (embedder_) {
       // New WebContents's owner_window is the embedder's owner_window.
@@ -504,27 +481,18 @@ WebContents::~WebContents() {
   if (managed_web_contents()) {
     managed_web_contents()->GetView()->SetDelegate(nullptr);
 
-    // For webview we need to tell content module to do some cleanup work before
-    // destroying it.
-    if (type_ == WEB_VIEW)
-      guest_delegate_->Destroy();
-
     RenderViewDeleted(web_contents()->GetRenderViewHost());
 
-    if (type_ == WEB_VIEW) {
-      DestroyWebContents(false /* async */);
+    if (type_ == BROWSER_WINDOW && owner_window()) {
+      for (ExtendedWebContentsObserver& observer : observers_)
+        observer.OnCloseContents();
     } else {
-      if (type_ == BROWSER_WINDOW && owner_window()) {
-        for (ExtendedWebContentsObserver& observer : observers_)
-          observer.OnCloseContents();
-      } else {
-        DestroyWebContents(true /* async */);
-      }
-      // The WebContentsDestroyed will not be called automatically because we
-      // destroy the webContents in the next tick. So we have to manually
-      // call it here to make sure "destroyed" event is emitted.
-      WebContentsDestroyed();
+      DestroyWebContents(!IsGuest() /* async */);
     }
+    // The WebContentsDestroyed will not be called automatically because we
+    // destroy the webContents in the next tick. So we have to manually
+    // call it here to make sure "destroyed" event is emitted.
+    WebContentsDestroyed();
   }
 }
 
@@ -784,13 +752,6 @@ content::JavaScriptDialogManager* WebContents::GetJavaScriptDialogManager(
   return dialog_manager_.get();
 }
 
-void WebContents::ResizeDueToAutoResize(content::WebContents* web_contents,
-                                        const gfx::Size& new_size) {
-  if (IsGuest()) {
-    guest_delegate_->ResizeDueToAutoResize(new_size);
-  }
-}
-
 void WebContents::BeforeUnloadFired(const base::TimeTicks& proceed_time) {
   // Do nothing, we override this method just to avoid compilation error since
   // there are two virtual functions named BeforeUnloadFired.
@@ -935,6 +896,8 @@ void WebContents::DidFinishNavigation(
         Emit("did-navigate", url, http_response_code, http_status_text);
       }
     }
+    if (IsGuest())
+      Emit("load-commit", url, is_main_frame);
   } else {
     auto url = navigation_handle->GetURL();
     int code = navigation_handle->GetNetErrorCode();
@@ -1075,7 +1038,8 @@ bool WebContents::OnMessageReceived(const IPC::Message& message,
 // 1. call webContents.destroy();
 // 2. garbage collection;
 // 3. user closes the window of webContents;
-// For webview only #1 will happen, for BrowserWindow both #1 and #3 may
+// 4. the embedder detaches the frame.
+// For webview only #4 will happen, for BrowserWindow both #1 and #3 may
 // happen. The #2 should never happen for webContents, because webview is
 // managed by GuestViewManager, and BrowserWindow's webContents is managed
 // by api::BrowserWindow.
@@ -1083,6 +1047,7 @@ bool WebContents::OnMessageReceived(const IPC::Message& message,
 // sure "destroyed" event is emitted. For #3, the content::WebContents will
 // be destroyed on close, and WebContentsDestroyed would be called for it, so
 // we need to make sure the api::WebContents is also deleted.
+// For #4, the WebContents will be destroyed by embedder.
 void WebContents::WebContentsDestroyed() {
   // Cleanup relationships with other parts.
   RemoveFromWeakMap();
@@ -1093,6 +1058,13 @@ void WebContents::WebContentsDestroyed() {
 
   Emit("destroyed");
 
+  // For guest view based on OOPIF, the WebContents is released by the embedder
+  // frame, and we need to clear the reference to the memory.
+  if (IsGuest() && managed_web_contents()) {
+    managed_web_contents()->ReleaseWebContents();
+    ResetManagedWebContents(false);
+  }
+
   // Destroy the native class in next tick.
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, GetDestroyClosure());
 }
@@ -1140,10 +1112,6 @@ void WebContents::LoadURL(const GURL& url, const mate::Dictionary& options) {
     return;
   }
 
-  if (guest_delegate_ && !guest_delegate_->IsAttached()) {
-    return;
-  }
-
   content::NavigationController::LoadURLParams params(url);
 
   if (!options.Get("httpReferrer", &params.referrer)) {
@@ -1754,15 +1722,20 @@ void WebContents::OnCursorChange(const content::WebCursor& cursor) {
   }
 }
 
-void WebContents::SetSize(const SetSizeParams& params) {
-  if (guest_delegate_)
-    guest_delegate_->SetSize(params);
+void WebContents::SetSize(v8::Local<v8::Value>) {
+  // TODO(zcbenz): Remove this method in 4.0.
 }
 
 bool WebContents::IsGuest() const {
   return type_ == WEB_VIEW;
 }
 
+void WebContents::AttachToIframe(content::WebContents* embedder_web_contents,
+                                 int embedder_frame_id) {
+  if (guest_delegate_)
+    guest_delegate_->AttachToIframe(embedder_web_contents, embedder_frame_id);
+}
+
 bool WebContents::IsOffScreen() const {
 #if defined(ENABLE_OSR)
   return type_ == OFF_SCREEN;
@@ -2051,6 +2024,7 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("startDrag", &WebContents::StartDrag)
       .SetMethod("setSize", &WebContents::SetSize)
       .SetMethod("isGuest", &WebContents::IsGuest)
+      .SetMethod("attachToIframe", &WebContents::AttachToIframe)
       .SetMethod("isOffscreen", &WebContents::IsOffScreen)
       .SetMethod("startPainting", &WebContents::StartPainting)
       .SetMethod("stopPainting", &WebContents::StopPainting)

+ 3 - 4
atom/browser/api/atom_api_web_contents.h

@@ -42,7 +42,6 @@ class ResourceRequestBody;
 
 namespace atom {
 
-struct SetSizeParams;
 class AtomBrowserContext;
 class AtomJavaScriptDialogManager;
 class WebContentsZoomController;
@@ -199,8 +198,10 @@ class WebContents : public mate::TrackableObject<WebContents>,
   void CapturePage(mate::Arguments* args);
 
   // Methods for creating <webview>.
-  void SetSize(const SetSizeParams& params);
+  void SetSize(v8::Local<v8::Value>);
   bool IsGuest() const;
+  void AttachToIframe(content::WebContents* embedder_web_contents,
+                      int embedder_frame_id);
 
   // Methods for offscreen rendering
   bool IsOffScreen() const;
@@ -342,8 +343,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
       const content::BluetoothChooser::EventHandler& handler) override;
   content::JavaScriptDialogManager* GetJavaScriptDialogManager(
       content::WebContents* source) override;
-  void ResizeDueToAutoResize(content::WebContents* web_contents,
-                             const gfx::Size& new_size) override;
 
   // content::WebContentsObserver:
   void BeforeUnloadFired(const base::TimeTicks& proceed_time) override;

+ 1 - 0
atom/browser/api/atom_api_web_view_manager.cc

@@ -10,6 +10,7 @@
 #include "atom/common/options_switches.h"
 #include "content/public/browser/browser_context.h"
 #include "native_mate/dictionary.h"
+
 // Must be the last in the includes list.
 // See https://github.com/electron/electron/issues/10363
 #include "atom/common/node_includes.h"

+ 4 - 2
atom/browser/common_web_contents_delegate.cc

@@ -152,7 +152,8 @@ CommonWebContentsDelegate::~CommonWebContentsDelegate() {}
 
 void CommonWebContentsDelegate::InitWithWebContents(
     content::WebContents* web_contents,
-    AtomBrowserContext* browser_context) {
+    AtomBrowserContext* browser_context,
+    bool is_guest) {
   browser_context_ = browser_context;
   web_contents->SetDelegate(this);
 
@@ -165,7 +166,8 @@ void CommonWebContentsDelegate::InitWithWebContents(
       !web_preferences || web_preferences->IsEnabled(options::kOffscreen);
 
   // Create InspectableWebContents.
-  web_contents_.reset(brightray::InspectableWebContents::Create(web_contents));
+  web_contents_.reset(
+      brightray::InspectableWebContents::Create(web_contents, is_guest));
   web_contents_->SetDelegate(this);
 }
 

+ 2 - 1
atom/browser/common_web_contents_delegate.h

@@ -42,7 +42,8 @@ class CommonWebContentsDelegate
   // Creates a InspectableWebContents object and takes onwership of
   // |web_contents|.
   void InitWithWebContents(content::WebContents* web_contents,
-                           AtomBrowserContext* browser_context);
+                           AtomBrowserContext* browser_context,
+                           bool is_guest);
 
   // Set the window as owner window.
   void SetOwnerWindow(NativeWindow* owner_window);

+ 3 - 0
atom/browser/web_contents_zoom_controller.cc

@@ -228,6 +228,9 @@ void WebContentsZoomController::DidFinishNavigation(
 }
 
 void WebContentsZoomController::WebContentsDestroyed() {
+  for (Observer& observer : observers_)
+    observer.OnZoomControllerWebContentsDestroyed();
+
   observers_.Clear();
   embedder_zoom_controller_ = nullptr;
 }

+ 1 - 0
atom/browser/web_contents_zoom_controller.h

@@ -24,6 +24,7 @@ class WebContentsZoomController
     virtual void OnZoomLevelChanged(content::WebContents* web_contents,
                                     double level,
                                     bool is_temporary) {}
+    virtual void OnZoomControllerWebContentsDestroyed() {}
 
    protected:
     virtual ~Observer() {}

+ 28 - 131
atom/browser/web_view_guest_delegate.cc

@@ -7,142 +7,58 @@
 #include "atom/browser/api/atom_api_web_contents.h"
 #include "atom/common/native_mate_converters/gurl_converter.h"
 #include "content/browser/web_contents/web_contents_impl.h"
-#include "content/public/browser/guest_host.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 
 namespace atom {
 
-namespace {
+WebViewGuestDelegate::WebViewGuestDelegate(content::WebContents* embedder,
+                                           api::WebContents* api_web_contents)
+    : embedder_web_contents_(embedder), api_web_contents_(api_web_contents) {}
 
-const int kDefaultWidth = 300;
-const int kDefaultHeight = 300;
-
-}  // namespace
-
-SetSizeParams::SetSizeParams() = default;
-SetSizeParams::~SetSizeParams() = default;
-
-WebViewGuestDelegate::WebViewGuestDelegate() {}
-
-WebViewGuestDelegate::~WebViewGuestDelegate() {}
-
-void WebViewGuestDelegate::Initialize(api::WebContents* api_web_contents) {
-  api_web_contents_ = api_web_contents;
-  Observe(api_web_contents->GetWebContents());
-}
-
-void WebViewGuestDelegate::Destroy() {
-  // Give the content module an opportunity to perform some cleanup.
+WebViewGuestDelegate::~WebViewGuestDelegate() {
   ResetZoomController();
-  guest_host_->WillDestroy();
-  guest_host_ = nullptr;
 }
 
-void WebViewGuestDelegate::SetSize(const SetSizeParams& params) {
-  bool enable_auto_size =
-      params.enable_auto_size ? *params.enable_auto_size : auto_size_enabled_;
-  gfx::Size min_size = params.min_size ? *params.min_size : min_auto_size_;
-  gfx::Size max_size = params.max_size ? *params.max_size : max_auto_size_;
-
-  if (params.normal_size)
-    normal_size_ = *params.normal_size;
-
-  min_auto_size_ = min_size;
-  min_auto_size_.SetToMin(max_size);
-  max_auto_size_ = max_size;
-  max_auto_size_.SetToMax(min_size);
-
-  enable_auto_size &= !min_auto_size_.IsEmpty() && !max_auto_size_.IsEmpty();
-
-  auto* rvh = web_contents()->GetRenderViewHost();
-  if (enable_auto_size) {
-    // Autosize is being enabled.
-    rvh->EnableAutoResize(min_auto_size_, max_auto_size_);
-    normal_size_.SetSize(0, 0);
-  } else {
-    // Autosize is being disabled.
-    // Use default width/height if missing from partially defined normal size.
-    if (normal_size_.width() && !normal_size_.height())
-      normal_size_.set_height(GetDefaultSize().height());
-    if (!normal_size_.width() && normal_size_.height())
-      normal_size_.set_width(GetDefaultSize().width());
-
-    gfx::Size new_size;
-    if (!normal_size_.IsEmpty()) {
-      new_size = normal_size_;
-    } else if (!guest_size_.IsEmpty()) {
-      new_size = guest_size_;
-    } else {
-      new_size = GetDefaultSize();
-    }
-
-    bool changed_due_to_auto_resize = false;
-    if (auto_size_enabled_) {
-      // Autosize was previously enabled.
-      rvh->DisableAutoResize(new_size);
-      changed_due_to_auto_resize = true;
-    } else {
-      // Autosize was already disabled.
-      guest_host_->SizeContents(new_size);
-    }
-
-    UpdateGuestSize(new_size, changed_due_to_auto_resize);
-  }
+void WebViewGuestDelegate::AttachToIframe(
+    content::WebContents* embedder_web_contents,
+    int embedder_frame_id) {
+  embedder_web_contents_ = embedder_web_contents;
 
-  auto_size_enabled_ = enable_auto_size;
-}
+  int embedder_process_id =
+      embedder_web_contents_->GetMainFrame()->GetProcess()->GetID();
+  auto* embedder_frame =
+      content::RenderFrameHost::FromID(embedder_process_id, embedder_frame_id);
+  DCHECK_EQ(embedder_web_contents_,
+            content::WebContents::FromRenderFrameHost(embedder_frame));
 
-void WebViewGuestDelegate::ResizeDueToAutoResize(const gfx::Size& new_size) {
-  UpdateGuestSize(new_size, auto_size_enabled_);
-}
-
-void WebViewGuestDelegate::DidFinishNavigation(
-    content::NavigationHandle* navigation_handle) {
-  if (navigation_handle->HasCommitted() && !navigation_handle->IsErrorPage()) {
-    auto is_main_frame = navigation_handle->IsInMainFrame();
-    auto url = navigation_handle->GetURL();
-    api_web_contents_->Emit("load-commit", url, is_main_frame);
-  }
-}
-
-void WebViewGuestDelegate::DidDetach() {
-  attached_ = false;
-  ResetZoomController();
-}
-
-void WebViewGuestDelegate::DidAttach(int guest_proxy_routing_id) {
-  attached_ = true;
-  api_web_contents_->Emit("did-attach");
+  // Attach this inner WebContents |guest_web_contents| to the outer
+  // WebContents |embedder_web_contents|. The outer WebContents's
+  // frame |embedder_frame| hosts the inner WebContents.
+  api_web_contents_->web_contents()->AttachToOuterWebContentsFrame(
+      embedder_web_contents_, embedder_frame);
 
   ResetZoomController();
 
   embedder_zoom_controller_ =
       WebContentsZoomController::FromWebContents(embedder_web_contents_);
-  auto* zoom_controller = api_web_contents_->GetZoomController();
   embedder_zoom_controller_->AddObserver(this);
+  auto* zoom_controller = api_web_contents_->GetZoomController();
   zoom_controller->SetEmbedderZoomController(embedder_zoom_controller_);
-}
 
-content::WebContents* WebViewGuestDelegate::GetOwnerWebContents() const {
-  return embedder_web_contents_;
+  api_web_contents_->Emit("did-attach");
 }
 
-void WebViewGuestDelegate::SetGuestHost(content::GuestHost* guest_host) {
-  guest_host_ = guest_host;
+void WebViewGuestDelegate::DidDetach() {
+  ResetZoomController();
 }
 
-void WebViewGuestDelegate::WillAttach(
-    content::WebContents* embedder_web_contents,
-    int element_instance_id,
-    bool is_full_page_plugin,
-    const base::Closure& completion_callback) {
-  embedder_web_contents_ = embedder_web_contents;
-  is_full_page_plugin_ = is_full_page_plugin;
-  completion_callback.Run();
+content::WebContents* WebViewGuestDelegate::GetOwnerWebContents() const {
+  return embedder_web_contents_;
 }
 
 void WebViewGuestDelegate::OnZoomLevelChanged(
@@ -161,23 +77,8 @@ void WebViewGuestDelegate::OnZoomLevelChanged(
   }
 }
 
-void WebViewGuestDelegate::UpdateGuestSize(const gfx::Size& new_size,
-                                           bool due_to_auto_resize) {
-  if (due_to_auto_resize)
-    api_web_contents_->Emit("size-changed", guest_size_.width(),
-                            guest_size_.height(), new_size.width(),
-                            new_size.height());
-  guest_size_ = new_size;
-}
-
-gfx::Size WebViewGuestDelegate::GetDefaultSize() const {
-  if (is_full_page_plugin_) {
-    // Full page plugins default to the size of the owner's viewport.
-    return embedder_web_contents_->GetRenderWidgetHostView()
-        ->GetVisibleViewportSize();
-  } else {
-    return gfx::Size(kDefaultWidth, kDefaultHeight);
-  }
+void WebViewGuestDelegate::OnZoomControllerWebContentsDestroyed() {
+  ResetZoomController();
 }
 
 void WebViewGuestDelegate::ResetZoomController() {
@@ -187,10 +88,6 @@ void WebViewGuestDelegate::ResetZoomController() {
   }
 }
 
-bool WebViewGuestDelegate::CanBeEmbeddedInsideCrossProcessFrames() {
-  return true;
-}
-
 content::RenderWidgetHost* WebViewGuestDelegate::GetOwnerRenderWidgetHost() {
   return embedder_web_contents_->GetRenderViewHost()->GetWidget();
 }

+ 9 - 85
atom/browser/web_view_guest_delegate.h

@@ -7,7 +7,6 @@
 
 #include "atom/browser/web_contents_zoom_controller.h"
 #include "content/public/browser/browser_plugin_guest_delegate.h"
-#include "content/public/browser/web_contents_observer.h"
 
 namespace atom {
 
@@ -15,80 +14,33 @@ namespace api {
 class WebContents;
 }
 
-// A struct of parameters for SetSize(). The parameters are all declared as
-// scoped pointers since they are all optional. Null pointers indicate that the
-// parameter has not been provided, and the last used value should be used. Note
-// that when |enable_auto_size| is true, providing |normal_size| is not
-// meaningful. This is because the normal size of the guestview is overridden
-// whenever autosizing occurs.
-struct SetSizeParams {
-  SetSizeParams();
-  ~SetSizeParams();
-
-  std::unique_ptr<bool> enable_auto_size;
-  std::unique_ptr<gfx::Size> min_size;
-  std::unique_ptr<gfx::Size> max_size;
-  std::unique_ptr<gfx::Size> normal_size;
-};
-
 class WebViewGuestDelegate : public content::BrowserPluginGuestDelegate,
-                             public content::WebContentsObserver,
                              public WebContentsZoomController::Observer {
  public:
-  WebViewGuestDelegate();
+  WebViewGuestDelegate(content::WebContents* embedder,
+                       api::WebContents* api_web_contents);
   ~WebViewGuestDelegate() override;
 
-  void Initialize(api::WebContents* api_web_contents);
-
-  // Called when the WebContents is going to be destroyed.
-  void Destroy();
-
-  // Used to toggle autosize mode for this GuestView, and set both the automatic
-  // and normal sizes.
-  void SetSize(const SetSizeParams& params);
-
-  // Invoked when the contents auto-resized and the container should match it.
-  void ResizeDueToAutoResize(const gfx::Size& new_size);
-
-  // Return true if attached.
-  bool IsAttached() const { return attached_; }
+  // Attach to the iframe.
+  void AttachToIframe(content::WebContents* embedder_web_contents,
+                      int embedder_frame_id);
 
  protected:
-  // content::WebContentsObserver:
-  void DidFinishNavigation(
-      content::NavigationHandle* navigation_handle) override;
-
   // content::BrowserPluginGuestDelegate:
-  void DidAttach(int guest_proxy_routing_id) final;
   void DidDetach() final;
   content::WebContents* GetOwnerWebContents() const final;
-  void SetGuestHost(content::GuestHost* guest_host) final;
-  void WillAttach(content::WebContents* embedder_web_contents,
-                  int element_instance_id,
-                  bool is_full_page_plugin,
-                  const base::Closure& completion_callback) final;
-  bool CanBeEmbeddedInsideCrossProcessFrames() override;
-  content::RenderWidgetHost* GetOwnerRenderWidgetHost() override;
-  content::SiteInstance* GetOwnerSiteInstance() override;
+  content::RenderWidgetHost* GetOwnerRenderWidgetHost() final;
+  content::SiteInstance* GetOwnerSiteInstance() final;
   content::WebContents* CreateNewGuestWindow(
-      const content::WebContents::CreateParams& create_params) override;
+      const content::WebContents::CreateParams& create_params) final;
 
   // WebContentsZoomController::Observer:
   void OnZoomLevelChanged(content::WebContents* web_contents,
                           double level,
                           bool is_temporary) override;
+  void OnZoomControllerWebContentsDestroyed() override;
 
  private:
-  // This method is invoked when the contents auto-resized to give the container
-  // an opportunity to match it if it wishes.
-  //
-  // This gives the derived class an opportunity to inform its container element
-  // or perform other actions.
-  void UpdateGuestSize(const gfx::Size& new_size, bool due_to_auto_resize);
-
-  // Returns the default size of the guestview.
-  gfx::Size GetDefaultSize() const;
-
   void ResetZoomController();
 
   // The WebContents that attaches this guest view.
@@ -98,34 +50,6 @@ class WebViewGuestDelegate : public content::BrowserPluginGuestDelegate,
   // to subscribe for zoom changes.
   WebContentsZoomController* embedder_zoom_controller_ = nullptr;
 
-  // The size of the container element.
-  gfx::Size element_size_;
-
-  // The size of the guest content. Note: In autosize mode, the container
-  // element may not match the size of the guest.
-  gfx::Size guest_size_;
-
-  // A pointer to the guest_host.
-  content::GuestHost* guest_host_ = nullptr;
-
-  // Indicates whether autosize mode is enabled or not.
-  bool auto_size_enabled_ = false;
-
-  // The maximum size constraints of the container element in autosize mode.
-  gfx::Size max_auto_size_;
-
-  // The minimum size constraints of the container element in autosize mode.
-  gfx::Size min_auto_size_;
-
-  // The size that will be used when autosize mode is disabled.
-  gfx::Size normal_size_;
-
-  // Whether the guest view is inside a plugin document.
-  bool is_full_page_plugin_ = false;
-
-  // Whether attached.
-  bool attached_ = false;
-
   api::WebContents* api_web_contents_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(WebViewGuestDelegate);

+ 40 - 16
atom/renderer/api/atom_api_web_frame.cc

@@ -13,6 +13,7 @@
 #include "atom/renderer/api/atom_api_spell_check_client.h"
 #include "base/memory/memory_pressure_listener.h"
 #include "content/public/renderer/render_frame.h"
+#include "content/public/renderer/render_frame_observer.h"
 #include "content/public/renderer/render_frame_visitor.h"
 #include "content/public/renderer/render_view.h"
 #include "native_mate/dictionary.h"
@@ -62,6 +63,29 @@ namespace api {
 
 namespace {
 
+content::RenderFrame* GetRenderFrame(v8::Local<v8::Value> value) {
+  v8::Local<v8::Context> context =
+      v8::Local<v8::Object>::Cast(value)->CreationContext();
+  if (context.IsEmpty())
+    return nullptr;
+  blink::WebLocalFrame* frame = blink::WebLocalFrame::FrameForContext(context);
+  if (!frame)
+    return nullptr;
+  return content::RenderFrame::FromWebFrame(frame);
+}
+
+class RenderFrameStatus : public content::RenderFrameObserver {
+ public:
+  explicit RenderFrameStatus(content::RenderFrame* render_frame)
+      : content::RenderFrameObserver(render_frame) {}
+  ~RenderFrameStatus() final {}
+
+  bool is_ok() { return render_frame() != nullptr; }
+
+  // RenderFrameObserver implementation.
+  void OnDestruct() final {}
+};
+
 class ScriptExecutionCallback : public blink::WebScriptExecutionCallback {
  public:
   using CompletionCallback =
@@ -170,20 +194,23 @@ v8::Local<v8::Value> WebFrame::RegisterEmbedderCustomElement(
       blink::WebString::FromUTF16(name), options);
 }
 
-void WebFrame::RegisterElementResizeCallback(
-    int element_instance_id,
-    const GuestViewContainer::ResizeCallback& callback) {
-  auto* guest_view_container = GuestViewContainer::FromID(element_instance_id);
-  if (guest_view_container)
-    guest_view_container->RegisterElementResizeCallback(callback);
-}
+int WebFrame::GetWebFrameId(v8::Local<v8::Value> content_window) {
+  // Get the WebLocalFrame before (possibly) executing any user-space JS while
+  // getting the |params|. We track the status of the RenderFrame via an
+  // observer in case it is deleted during user code execution.
+  content::RenderFrame* render_frame = GetRenderFrame(content_window);
+  RenderFrameStatus render_frame_status(render_frame);
 
-void WebFrame::AttachGuest(int id) {
-  content::RenderFrame::FromWebFrame(web_frame_)->AttachGuest(id);
-}
+  if (!render_frame_status.is_ok())
+    return -1;
+
+  blink::WebLocalFrame* frame = render_frame->GetWebFrame();
+  // Parent must exist.
+  blink::WebFrame* parent_frame = frame->Parent();
+  DCHECK(parent_frame);
+  DCHECK(parent_frame->IsWebLocalFrame());
 
-void WebFrame::DetachGuest(int id) {
-  content::RenderFrame::FromWebFrame(web_frame_)->DetachGuest(id);
+  return render_frame->GetRoutingID();
 }
 
 void WebFrame::SetSpellCheckProvider(mate::Arguments* args,
@@ -464,10 +491,7 @@ void WebFrame::BuildPrototype(v8::Isolate* isolate,
                  &WebFrame::SetLayoutZoomLevelLimits)
       .SetMethod("registerEmbedderCustomElement",
                  &WebFrame::RegisterEmbedderCustomElement)
-      .SetMethod("registerElementResizeCallback",
-                 &WebFrame::RegisterElementResizeCallback)
-      .SetMethod("attachGuest", &WebFrame::AttachGuest)
-      .SetMethod("detachGuest", &WebFrame::DetachGuest)
+      .SetMethod("getWebFrameId", &WebFrame::GetWebFrameId)
       .SetMethod("setSpellCheckProvider", &WebFrame::SetSpellCheckProvider)
       .SetMethod("registerURLSchemeAsBypassingCSP",
                  &WebFrame::RegisterURLSchemeAsBypassingCSP)

+ 1 - 6
atom/renderer/api/atom_api_web_frame.h

@@ -9,7 +9,6 @@
 #include <string>
 #include <vector>
 
-#include "atom/renderer/guest_view_container.h"
 #include "native_mate/handle.h"
 #include "native_mate/wrappable.h"
 #include "third_party/WebKit/public/platform/WebCache.h"
@@ -54,11 +53,7 @@ class WebFrame : public mate::Wrappable<WebFrame> {
   v8::Local<v8::Value> RegisterEmbedderCustomElement(
       const base::string16& name,
       v8::Local<v8::Object> options);
-  void RegisterElementResizeCallback(
-      int element_instance_id,
-      const GuestViewContainer::ResizeCallback& callback);
-  void AttachGuest(int element_instance_id);
-  void DetachGuest(int element_instance_id);
+  int GetWebFrameId(v8::Local<v8::Value> content_window);
 
   // Set the provider that will be used by SpellCheckClient for spell check.
   void SetSpellCheckProvider(mate::Arguments* args,

+ 1 - 12
atom/renderer/renderer_client_base.cc

@@ -14,7 +14,6 @@
 #include "atom/renderer/atom_render_frame_observer.h"
 #include "atom/renderer/atom_render_view_observer.h"
 #include "atom/renderer/content_settings_observer.h"
-#include "atom/renderer/guest_view_container.h"
 #include "atom/renderer/preferences_manager.h"
 #include "base/command_line.h"
 #include "base/process/process_handle.h"
@@ -25,6 +24,7 @@
 #include "chrome/renderer/printing/print_web_view_helper.h"
 #include "chrome/renderer/tts_dispatcher.h"
 #include "content/public/common/content_constants.h"
+#include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_view.h"
 #include "native_mate/dictionary.h"
 #include "third_party/WebKit/Source/platform/weborigin/SchemeRegistry.h"
@@ -219,17 +219,6 @@ bool RendererClientBase::OverrideCreatePlugin(
   return true;
 }
 
-content::BrowserPluginDelegate* RendererClientBase::CreateBrowserPluginDelegate(
-    content::RenderFrame* render_frame,
-    const std::string& mime_type,
-    const GURL& original_url) {
-  if (mime_type == content::kBrowserPluginMimeType) {
-    return new GuestViewContainer(render_frame);
-  } else {
-    return nullptr;
-  }
-}
-
 void RendererClientBase::AddSupportedKeySystems(
     std::vector<std::unique_ptr<::media::KeySystemProperties>>* key_systems) {
   AddChromeKeySystems(key_systems);

+ 0 - 4
atom/renderer/renderer_client_base.h

@@ -46,10 +46,6 @@ class RendererClientBase : public content::ContentRendererClient {
   bool OverrideCreatePlugin(content::RenderFrame* render_frame,
                             const blink::WebPluginParams& params,
                             blink::WebPlugin** plugin) override;
-  content::BrowserPluginDelegate* CreateBrowserPluginDelegate(
-      content::RenderFrame* render_frame,
-      const std::string& mime_type,
-      const GURL& original_url) override;
   void AddSupportedKeySystems(
       std::vector<std::unique_ptr<::media::KeySystemProperties>>* key_systems)
       override;

+ 1 - 3
brightray/browser/browser_main_parts.cc

@@ -186,10 +186,8 @@ void BrowserMainParts::InitializeFeatureList() {
   auto disable_features =
       cmd_line->GetSwitchValueASCII(switches::kDisableFeatures);
 
-  // TODO(deepak1556): Disable guest webcontents based on OOPIF feature.
   // Disable surface synchronization and async wheel events to make OSR work.
-  disable_features +=
-      ",GuestViewCrossProcessFrames,SurfaceSynchronization,AsyncWheelEvents";
+  disable_features += ",SurfaceSynchronization,AsyncWheelEvents";
 
   auto feature_list = std::make_unique<base::FeatureList>();
   feature_list->InitializeFromCommandLine(enable_features, disable_features);

+ 3 - 8
brightray/browser/inspectable_web_contents.cc

@@ -5,14 +5,9 @@
 namespace brightray {
 
 InspectableWebContents* InspectableWebContents::Create(
-    const content::WebContents::CreateParams& create_params) {
-  auto* contents = content::WebContents::Create(create_params);
-  return Create(contents);
-}
-
-InspectableWebContents* InspectableWebContents::Create(
-    content::WebContents* web_contents) {
-  return new InspectableWebContentsImpl(web_contents);
+    content::WebContents* web_contents,
+    bool is_guest) {
+  return new InspectableWebContentsImpl(web_contents, is_guest);
 }
 
 }  // namespace brightray

+ 4 - 4
brightray/browser/inspectable_web_contents.h

@@ -20,12 +20,10 @@ class InspectableWebContentsView;
 
 class InspectableWebContents {
  public:
-  static InspectableWebContents* Create(
-      const content::WebContents::CreateParams&);
-
   // The returned InspectableWebContents takes ownership of the passed-in
   // WebContents.
-  static InspectableWebContents* Create(content::WebContents*);
+  static InspectableWebContents* Create(content::WebContents* web_contents,
+                                        bool is_guest);
 
   virtual ~InspectableWebContents() {}
 
@@ -37,6 +35,8 @@ class InspectableWebContents {
   virtual void SetDelegate(InspectableWebContentsDelegate* delegate) = 0;
   virtual InspectableWebContentsDelegate* GetDelegate() const = 0;
 
+  virtual bool IsGuest() const = 0;
+  virtual void ReleaseWebContents() = 0;
   virtual void SetDevToolsWebContents(content::WebContents* devtools) = 0;
   virtual void SetDockState(const std::string& state) = 0;
   virtual void ShowDevTools() = 0;

+ 19 - 7
brightray/browser/inspectable_web_contents_impl.cc

@@ -202,15 +202,20 @@ void InspectableWebContentsImpl::RegisterPrefs(PrefRegistrySimple* registry) {
 }
 
 InspectableWebContentsImpl::InspectableWebContentsImpl(
-    content::WebContents* web_contents)
+    content::WebContents* web_contents,
+    bool is_guest)
     : frontend_loaded_(false),
       can_dock_(true),
       delegate_(nullptr),
+      pref_service_(
+          static_cast<BrowserContext*>(web_contents->GetBrowserContext())
+              ->prefs()),
       web_contents_(web_contents),
+      is_guest_(is_guest),
+      view_(CreateInspectableContentsView(this)),
       weak_factory_(this) {
-  auto* context =
-      static_cast<BrowserContext*>(web_contents_->GetBrowserContext());
-  pref_service_ = context->prefs();
+  if (is_guest)
+    return;
   auto* bounds_dict = pref_service_->GetDictionary(kDevToolsBoundsPref);
   if (bounds_dict) {
     DictionaryToRect(*bounds_dict, &devtools_bounds_);
@@ -235,8 +240,6 @@ InspectableWebContentsImpl::InspectableWebContentsImpl(
           display.y() + (display.height() - devtools_bounds_.height()) / 2);
     }
   }
-
-  view_.reset(CreateInspectableContentsView(this));
 }
 
 InspectableWebContentsImpl::~InspectableWebContentsImpl() {
@@ -282,6 +285,14 @@ InspectableWebContentsDelegate* InspectableWebContentsImpl::GetDelegate()
   return delegate_;
 }
 
+bool InspectableWebContentsImpl::IsGuest() const {
+  return is_guest_;
+}
+
+void InspectableWebContentsImpl::ReleaseWebContents() {
+  web_contents_.release();
+}
+
 void InspectableWebContentsImpl::SetDockState(const std::string& state) {
   if (state == "detach") {
     can_dock_ = false;
@@ -332,7 +343,8 @@ void InspectableWebContentsImpl::CloseDevTools() {
       managed_devtools_web_contents_.reset();
     }
     embedder_message_dispatcher_.reset();
-    web_contents_->Focus();
+    if (!IsGuest())
+      web_contents_->Focus();
   }
 }
 

+ 4 - 1
brightray/browser/inspectable_web_contents_impl.h

@@ -39,7 +39,7 @@ class InspectableWebContentsImpl
  public:
   static void RegisterPrefs(PrefRegistrySimple* pref_registry);
 
-  explicit InspectableWebContentsImpl(content::WebContents*);
+  InspectableWebContentsImpl(content::WebContents* web_contents, bool is_guest);
   ~InspectableWebContentsImpl() override;
 
   InspectableWebContentsView* GetView() const override;
@@ -48,6 +48,8 @@ class InspectableWebContentsImpl
 
   void SetDelegate(InspectableWebContentsDelegate* delegate) override;
   InspectableWebContentsDelegate* GetDelegate() const override;
+  bool IsGuest() const override;
+  void ReleaseWebContents() override;
   void SetDevToolsWebContents(content::WebContents* devtools) override;
   void SetDockState(const std::string& state) override;
   void ShowDevTools() override;
@@ -214,6 +216,7 @@ class InspectableWebContentsImpl
   // The external devtools assigned by SetDevToolsWebContents.
   content::WebContents* external_devtools_web_contents_ = nullptr;
 
+  bool is_guest_;
   std::unique_ptr<InspectableWebContentsView> view_;
 
   using ExtensionsAPIs = std::map<std::string, std::string>;

+ 1 - 0
brightray/browser/mac/bry_inspectable_web_contents_view.h

@@ -14,6 +14,7 @@ using brightray::InspectableWebContentsViewMac;
  @private
   brightray::InspectableWebContentsViewMac* inspectableWebContentsView_;
 
+  base::scoped_nsobject<NSView> fake_view_;
   base::scoped_nsobject<NSWindow> devtools_window_;
   BOOL devtools_visible_;
   BOOL devtools_docked_;

+ 13 - 7
brightray/browser/mac/bry_inspectable_web_contents_view.mm

@@ -32,11 +32,17 @@
              name:NSWindowDidBecomeMainNotification
            object:nil];
 
-  auto* contents =
-      inspectableWebContentsView_->inspectable_web_contents()->GetWebContents();
-  auto contentsView = contents->GetNativeView();
-  [contentsView setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
-  [self addSubview:contentsView];
+  if (inspectableWebContentsView_->inspectable_web_contents()->IsGuest()) {
+    fake_view_.reset([[NSView alloc] init]);
+    [fake_view_ setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
+    [self addSubview:fake_view_];
+  } else {
+    auto* contents = inspectableWebContentsView_->inspectable_web_contents()
+                         ->GetWebContents();
+    auto contentsView = contents->GetNativeView();
+    [contentsView setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
+    [self addSubview:contentsView];
+  }
 
   // See https://code.google.com/p/chromium/issues/detail?id=348490.
   [self setWantsLayer:YES];
@@ -75,7 +81,7 @@
   if (visible && devtools_docked_) {
     webContents->SetAllowOtherViews(true);
     devToolsWebContents->SetAllowOtherViews(true);
-  } else {
+  } else if (!inspectable_web_contents->IsGuest()) {
     webContents->SetAllowOtherViews(false);
   }
 
@@ -194,7 +200,7 @@
 - (void)viewDidBecomeFirstResponder:(NSNotification*)notification {
   auto* inspectable_web_contents =
       inspectableWebContentsView_->inspectable_web_contents();
-  if (!inspectable_web_contents)
+  if (!inspectable_web_contents || inspectable_web_contents->IsGuest())
     return;
   auto* webContents = inspectable_web_contents->GetWebContents();
   auto webContentsView = webContents->GetNativeView();

+ 2 - 1
brightray/browser/views/inspectable_web_contents_view_views.cc

@@ -81,7 +81,8 @@ InspectableWebContentsViewViews::InspectableWebContentsViewViews(
       title_(base::ASCIIToUTF16("Developer Tools")) {
   set_owned_by_client();
 
-  if (inspectable_web_contents_->GetWebContents()->GetNativeView()) {
+  if (!inspectable_web_contents_->IsGuest() &&
+      inspectable_web_contents_->GetWebContents()->GetNativeView()) {
     views::WebView* contents_web_view = new views::WebView(nullptr);
     contents_web_view->SetWebContents(
         inspectable_web_contents_->GetWebContents());

+ 32 - 75
lib/browser/guest-view-manager.js

@@ -46,11 +46,6 @@ let nextGuestInstanceId = 0
 const guestInstances = {}
 const embedderElementsMap = {}
 
-// Moves the last element of array to the first one.
-const moveLastToFirst = function (list) {
-  list.unshift(list.pop())
-}
-
 // Generate guestInstanceId.
 const getNextGuestInstanceId = function () {
   return ++nextGuestInstanceId
@@ -73,10 +68,15 @@ const createGuest = function (embedder, params) {
     embedder: embedder
   }
 
-  watchEmbedder(embedder)
+  // Clear the guest from map when it is destroyed.
+  guest.once('destroyed', () => {
+    if (guestInstanceId in guestInstances) {
+      detachGuest(embedder, guestInstanceId)
+    }
+  })
 
   // Init guest web view after attached.
-  guest.on('did-attach', function (event) {
+  guest.once('did-attach', function (event) {
     params = this.attachParams
     delete this.attachParams
 
@@ -88,21 +88,6 @@ const createGuest = function (embedder, params) {
       return
     }
 
-    this.setSize({
-      normal: {
-        width: params.elementWidth,
-        height: params.elementHeight
-      },
-      enableAutoSize: params.autosize,
-      min: {
-        width: params.minwidth,
-        height: params.minheight
-      },
-      max: {
-        width: params.maxwidth,
-        height: params.maxheight
-      }
-    })
     if (params.src) {
       const opts = {}
       if (params.httpreferrer) {
@@ -118,8 +103,7 @@ const createGuest = function (embedder, params) {
   })
 
   const sendToEmbedder = (channel, ...args) => {
-    const embedder = getEmbedder(guestInstanceId)
-    if (embedder != null) {
+    if (!embedder.isDestroyed()) {
       embedder.send(`${channel}-${guest.viewInstanceId}`, ...args)
     }
   }
@@ -139,11 +123,6 @@ const createGuest = function (embedder, params) {
     sendToEmbedder('ELECTRON_GUEST_VIEW_INTERNAL_IPC_MESSAGE', channel, ...args)
   })
 
-  // Autosize.
-  guest.on('size-changed', function (_, ...args) {
-    sendToEmbedder('ELECTRON_GUEST_VIEW_INTERNAL_SIZE_CHANGED', ...args)
-  })
-
   // Notify guest of embedder window visibility when it is ready
   // FIXME Remove once https://github.com/electron/electron/issues/6828 is fixed
   guest.on('dom-ready', function () {
@@ -176,7 +155,7 @@ const createGuest = function (embedder, params) {
 }
 
 // Attach the guest to an element of embedder.
-const attachGuest = function (event, elementInstanceId, guestInstanceId, params) {
+const attachGuest = function (event, embedderFrameId, elementInstanceId, guestInstanceId, params) {
   const embedder = event.sender
   // Destroy the old guest when attaching.
   const key = `${embedder.getId()}-${elementInstanceId}`
@@ -187,7 +166,10 @@ const attachGuest = function (event, elementInstanceId, guestInstanceId, params)
       return
     }
 
-    destroyGuest(embedder, oldGuestInstanceId)
+    const oldGuestInstance = guestInstances[oldGuestInstanceId]
+    if (oldGuestInstance) {
+      oldGuestInstance.guest.destroy()
+    }
   }
 
   const guestInstance = guestInstances[guestInstanceId]
@@ -260,11 +242,10 @@ const attachGuest = function (event, elementInstanceId, guestInstanceId, params)
   embedder.emit('will-attach-webview', event, webPreferences, params)
   if (event.defaultPrevented) {
     if (guest.viewInstanceId == null) guest.viewInstanceId = params.instanceId
-    destroyGuest(embedder, guestInstanceId)
+    guest.destroy()
     return
   }
 
-  webViewManager.addGuest(guestInstanceId, elementInstanceId, embedder, guest, webPreferences)
   guest.attachParams = params
   embedderElementsMap[key] = guestInstanceId
 
@@ -273,21 +254,19 @@ const attachGuest = function (event, elementInstanceId, guestInstanceId, params)
   guestInstance.elementInstanceId = elementInstanceId
 
   watchEmbedder(embedder)
-}
 
-// Destroy an existing guest instance.
-const destroyGuest = function (embedder, guestInstanceId) {
-  if (!(guestInstanceId in guestInstances)) {
-    return
-  }
+  webViewManager.addGuest(guestInstanceId, elementInstanceId, embedder, guest, webPreferences)
+  guest.attachToIframe(embedder, embedderFrameId)
+}
 
+// Remove an guest-embedder relationship.
+const detachGuest = function (embedder, guestInstanceId) {
   const guestInstance = guestInstances[guestInstanceId]
   if (embedder !== guestInstance.embedder) {
     return
   }
 
   webViewManager.removeGuest(embedder, guestInstanceId)
-  guestInstance.guest.destroy()
   delete guestInstances[guestInstanceId]
 
   const key = `${embedder.getId()}-${guestInstance.elementInstanceId}`
@@ -305,7 +284,7 @@ const watchEmbedder = function (embedder) {
 
   // Forward embedder window visiblity change events to guest
   const onVisibilityChange = function (visibilityState) {
-    for (const guestInstanceId of Object.keys(guestInstances)) {
+    for (const guestInstanceId in guestInstances) {
       const guestInstance = guestInstances[guestInstanceId]
       guestInstance.visibilityState = visibilityState
       if (guestInstance.embedder === embedder) {
@@ -315,33 +294,20 @@ const watchEmbedder = function (embedder) {
   }
   embedder.on('-window-visibility-change', onVisibilityChange)
 
-  const destroyEvents = ['will-destroy', 'crashed', 'did-navigate']
-  const destroy = function () {
-    for (const guestInstanceId of Object.keys(guestInstances)) {
-      if (guestInstances[guestInstanceId].embedder === embedder) {
-        destroyGuest(embedder, parseInt(guestInstanceId))
+  embedder.once('will-destroy', () => {
+    // Usually the guestInstances is cleared when guest is destroyed, but it
+    // may happen that the embedder gets manually destroyed earlier than guest,
+    // and the embedder will be invalid in the usual code path.
+    for (const guestInstanceId in guestInstances) {
+      const guestInstance = guestInstances[guestInstanceId]
+      if (guestInstance.embedder === embedder) {
+        detachGuest(embedder, parseInt(guestInstanceId))
       }
     }
-
-    for (const event of destroyEvents) {
-      embedder.removeListener(event, destroy)
-    }
+    // Clear the listeners.
     embedder.removeListener('-window-visibility-change', onVisibilityChange)
-
     watchedEmbedders.delete(embedder)
-  }
-
-  for (const event of destroyEvents) {
-    embedder.once(event, destroy)
-
-    // Users might also listen to the crashed event, so we must ensure the guest
-    // is destroyed before users' listener gets called. It is done by moving our
-    // listener to the first one in queue.
-    const listeners = embedder._events[event]
-    if (Array.isArray(listeners)) {
-      moveLastToFirst(listeners)
-    }
-  }
+  })
 }
 
 ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', function (event, params, requestId) {
@@ -352,17 +318,8 @@ ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST_SYNC', function (event, par
   event.returnValue = createGuest(event.sender, params)
 })
 
-ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', function (event, elementInstanceId, guestInstanceId, params) {
-  attachGuest(event, elementInstanceId, guestInstanceId, params)
-})
-
-ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_DESTROY_GUEST', function (event, guestInstanceId) {
-  destroyGuest(event.sender, guestInstanceId)
-})
-
-ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_SET_SIZE', function (event, guestInstanceId, params) {
-  const guest = getGuest(guestInstanceId)
-  if (guest != null) guest.setSize(params)
+ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', function (event, embedderFrameId, elementInstanceId, guestInstanceId, params) {
+  attachGuest(event, embedderFrameId, elementInstanceId, guestInstanceId, params)
 })
 
 // Returns WebContents from its guest id.

+ 6 - 21
lib/renderer/web-view/guest-view-internal.js

@@ -61,7 +61,6 @@ const dispatchEvent = function (webView, eventName, eventKey, ...args) {
 module.exports = {
   registerEvents: function (webView, viewInstanceId) {
     ipcRenderer.on(`ELECTRON_GUEST_VIEW_INTERNAL_DESTROY_GUEST-${viewInstanceId}`, function () {
-      webFrame.detachGuest(webView.internalInstanceId)
       webView.guestInstanceId = undefined
       webView.reset()
       const domEvent = new Event('destroyed')
@@ -78,22 +77,11 @@ module.exports = {
       domEvent.args = args
       webView.dispatchEvent(domEvent)
     })
-
-    ipcRenderer.on(`ELECTRON_GUEST_VIEW_INTERNAL_SIZE_CHANGED-${viewInstanceId}`, function (event, ...args) {
-      const domEvent = new Event('size-changed')
-      const props = ['oldWidth', 'oldHeight', 'newWidth', 'newHeight']
-      for (let i = 0; i < props.length; i++) {
-        const prop = props[i]
-        domEvent[prop] = args[i]
-      }
-      webView.onSizeChanged(domEvent)
-    })
   },
   deregisterEvents: function (viewInstanceId) {
     ipcRenderer.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_DESTROY_GUEST-${viewInstanceId}`)
     ipcRenderer.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_DISPATCH_EVENT-${viewInstanceId}`)
     ipcRenderer.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_IPC_MESSAGE-${viewInstanceId}`)
-    ipcRenderer.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_SIZE_CHANGED-${viewInstanceId}`)
   },
   createGuest: function (params, callback) {
     requestId++
@@ -103,14 +91,11 @@ module.exports = {
   createGuestSync: function (params) {
     return ipcRenderer.sendSync('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST_SYNC', params)
   },
-  attachGuest: function (elementInstanceId, guestInstanceId, params) {
-    ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', elementInstanceId, guestInstanceId, params)
-    webFrame.attachGuest(elementInstanceId)
-  },
-  destroyGuest: function (guestInstanceId) {
-    ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_DESTROY_GUEST', guestInstanceId)
-  },
-  setSize: function (guestInstanceId, params) {
-    ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_SET_SIZE', guestInstanceId, params)
+  attachGuest: function (elementInstanceId, guestInstanceId, params, contentWindow) {
+    const embedderFrameId = webFrame.getWebFrameId(contentWindow)
+    if (embedderFrameId < 0) {  // this error should not happen.
+      throw new Error('Invalid embedder frame')
+    }
+    ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', embedderFrameId, elementInstanceId, guestInstanceId, params)
   }
 }

+ 0 - 79
lib/renderer/web-view/web-view-attributes.js

@@ -1,7 +1,6 @@
 'use strict'
 
 const WebViewImpl = require('./web-view')
-const guestViewInternal = require('./guest-view-internal')
 const webViewConstants = require('./web-view-constants')
 const {remote} = require('electron')
 
@@ -74,39 +73,6 @@ class BooleanAttribute extends WebViewAttribute {
   }
 }
 
-// Attribute used to define the demension limits of autosizing.
-class AutosizeDimensionAttribute extends WebViewAttribute {
-  getValue () {
-    return parseInt(this.webViewImpl.webviewNode.getAttribute(this.name)) || 0
-  }
-
-  handleMutation () {
-    if (!this.webViewImpl.guestInstanceId) {
-      return
-    }
-    guestViewInternal.setSize(this.webViewImpl.guestInstanceId, {
-      enableAutoSize: this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_AUTOSIZE].getValue(),
-      min: {
-        width: parseInt(this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_MINWIDTH].getValue() || 0),
-        height: parseInt(this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_MINHEIGHT].getValue() || 0)
-      },
-      max: {
-        width: parseInt(this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_MAXWIDTH].getValue() || 0),
-        height: parseInt(this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_MAXHEIGHT].getValue() || 0)
-      }
-    })
-  }
-}
-
-// Attribute that specifies whether the webview should be autosized.
-class AutosizeAttribute extends BooleanAttribute {
-  constructor (webViewImpl) {
-    super(webViewConstants.ATTRIBUTE_AUTOSIZE, webViewImpl)
-  }
-}
-
-AutosizeAttribute.prototype.handleMutation = AutosizeDimensionAttribute.prototype.handleMutation
-
 // Attribute representing the state of the storage partition.
 class PartitionAttribute extends WebViewAttribute {
   constructor (webViewImpl) {
@@ -130,43 +96,6 @@ class PartitionAttribute extends WebViewAttribute {
   }
 }
 
-// An attribute that controls the guest instance this webview is connected to
-class GuestInstanceAttribute extends WebViewAttribute {
-  constructor (webViewImpl) {
-    super(webViewConstants.ATTRIBUTE_GUESTINSTANCE, webViewImpl)
-  }
-
-  // Retrieves and returns the attribute's value.
-  getValue () {
-    if (this.webViewImpl.webviewNode.hasAttribute(this.name)) {
-      return parseInt(this.webViewImpl.webviewNode.getAttribute(this.name))
-    }
-  }
-
-  // Sets the attribute's value.
-  setValue (value) {
-    if (!value) {
-      this.webViewImpl.webviewNode.removeAttribute(this.name)
-    } else if (!isNaN(value)) {
-      this.webViewImpl.webviewNode.setAttribute(this.name, value)
-    }
-  }
-
-  handleMutation (oldValue, newValue) {
-    if (!newValue) {
-      this.webViewImpl.reset()
-      return
-    }
-
-    const intVal = parseInt(newValue)
-    if (!isNaN(newValue) && remote.getGuestWebContents(intVal)) {
-      this.webViewImpl.attachGuestInstance(intVal)
-    } else {
-      this.setValueIgnoreMutation(oldValue)
-    }
-  }
-}
-
 // Attribute that handles the location and navigation of the webview.
 class SrcAttribute extends WebViewAttribute {
   constructor (webViewImpl) {
@@ -314,7 +243,6 @@ class WebPreferencesAttribute extends WebViewAttribute {
 // Sets up all of the webview attributes.
 WebViewImpl.prototype.setupWebViewAttributes = function () {
   this.attributes = {}
-  this.attributes[webViewConstants.ATTRIBUTE_AUTOSIZE] = new AutosizeAttribute(this)
   this.attributes[webViewConstants.ATTRIBUTE_PARTITION] = new PartitionAttribute(this)
   this.attributes[webViewConstants.ATTRIBUTE_SRC] = new SrcAttribute(this)
   this.attributes[webViewConstants.ATTRIBUTE_HTTPREFERRER] = new HttpReferrerAttribute(this)
@@ -326,12 +254,5 @@ WebViewImpl.prototype.setupWebViewAttributes = function () {
   this.attributes[webViewConstants.ATTRIBUTE_PRELOAD] = new PreloadAttribute(this)
   this.attributes[webViewConstants.ATTRIBUTE_BLINKFEATURES] = new BlinkFeaturesAttribute(this)
   this.attributes[webViewConstants.ATTRIBUTE_DISABLEBLINKFEATURES] = new DisableBlinkFeaturesAttribute(this)
-  this.attributes[webViewConstants.ATTRIBUTE_GUESTINSTANCE] = new GuestInstanceAttribute(this)
-  this.attributes[webViewConstants.ATTRIBUTE_DISABLEGUESTRESIZE] = new BooleanAttribute(webViewConstants.ATTRIBUTE_DISABLEGUESTRESIZE, this)
   this.attributes[webViewConstants.ATTRIBUTE_WEBPREFERENCES] = new WebPreferencesAttribute(this)
-
-  const autosizeAttributes = [webViewConstants.ATTRIBUTE_MAXHEIGHT, webViewConstants.ATTRIBUTE_MAXWIDTH, webViewConstants.ATTRIBUTE_MINHEIGHT, webViewConstants.ATTRIBUTE_MINWIDTH]
-  autosizeAttributes.forEach((attribute) => {
-    this.attributes[attribute] = new AutosizeDimensionAttribute(attribute, this)
-  })
 }

+ 0 - 7
lib/renderer/web-view/web-view-constants.js

@@ -1,10 +1,5 @@
 module.exports = {
   // Attributes.
-  ATTRIBUTE_AUTOSIZE: 'autosize',
-  ATTRIBUTE_MAXHEIGHT: 'maxheight',
-  ATTRIBUTE_MAXWIDTH: 'maxwidth',
-  ATTRIBUTE_MINHEIGHT: 'minheight',
-  ATTRIBUTE_MINWIDTH: 'minwidth',
   ATTRIBUTE_NAME: 'name',
   ATTRIBUTE_PARTITION: 'partition',
   ATTRIBUTE_SRC: 'src',
@@ -17,8 +12,6 @@ module.exports = {
   ATTRIBUTE_USERAGENT: 'useragent',
   ATTRIBUTE_BLINKFEATURES: 'blinkfeatures',
   ATTRIBUTE_DISABLEBLINKFEATURES: 'disableblinkfeatures',
-  ATTRIBUTE_GUESTINSTANCE: 'guestinstance',
-  ATTRIBUTE_DISABLEGUESTRESIZE: 'disableguestresize',
   ATTRIBUTE_WEBPREFERENCES: 'webpreferences',
 
   // Internal attribute.

+ 35 - 125
lib/renderer/web-view/web-view.js

@@ -6,8 +6,6 @@ const v8Util = process.atomBinding('v8_util')
 const guestViewInternal = require('./guest-view-internal')
 const webViewConstants = require('./web-view-constants')
 
-const hasProp = {}.hasOwnProperty
-
 // An unique ID that can represent current context.
 const contextId = v8Util.getHiddenValue(global, 'contextId')
 
@@ -23,27 +21,27 @@ class WebViewImpl {
   constructor (webviewNode) {
     this.webviewNode = webviewNode
     v8Util.setHiddenValue(this.webviewNode, 'internal', this)
-    this.attached = false
     this.elementAttached = false
     this.beforeFirstNavigation = true
 
     // on* Event handlers.
     this.on = {}
-    this.browserPluginNode = this.createBrowserPluginNode()
+    this.internalElement = this.createInternalElement()
     const shadowRoot = this.webviewNode.attachShadow({mode: 'open'})
     shadowRoot.innerHTML = '<!DOCTYPE html><style type="text/css">:host { display: flex; }</style>'
     this.setupWebViewAttributes()
     this.setupFocusPropagation()
     this.viewInstanceId = getNextId()
-    shadowRoot.appendChild(this.browserPluginNode)
+    shadowRoot.appendChild(this.internalElement)
   }
 
-  createBrowserPluginNode () {
-    // We create BrowserPlugin as a custom element in order to observe changes
-    // to attributes synchronously.
-    const browserPluginNode = new WebViewImpl.BrowserPlugin()
-    v8Util.setHiddenValue(browserPluginNode, 'internal', this)
-    return browserPluginNode
+  createInternalElement () {
+    const iframeElement = document.createElement('iframe')
+    iframeElement.style.width = '100%'
+    iframeElement.style.height = '100%'
+    iframeElement.style.border = '0'
+    v8Util.setHiddenValue(iframeElement, 'internal', this)
+    return iframeElement
   }
 
   // Resets some state upon reattaching <webview> element to the DOM.
@@ -55,7 +53,6 @@ class WebViewImpl {
     // heard back from createGuest yet. We will not reset the flag in this case so
     // that we don't end up allocating a second guest.
     if (this.guestInstanceId) {
-      guestViewInternal.destroyGuest(this.guestInstanceId)
       this.guestInstanceId = void 0
     }
 
@@ -63,9 +60,12 @@ class WebViewImpl {
     this.beforeFirstNavigation = true
     this.attributes[webViewConstants.ATTRIBUTE_PARTITION].validPartitionId = true
 
-    // Set guestinstance last since this can trigger the attachedCallback to fire
-    // when moving the webview using element.replaceChild
-    this.attributes[webViewConstants.ATTRIBUTE_GUESTINSTANCE].setValueIgnoreMutation(undefined)
+    // Since attachment swaps a local frame for a remote frame, we need our
+    // internal iframe element to be local again before we can reattach.
+    const newFrame = this.createInternalElement()
+    const oldFrame = this.internalElement
+    this.internalElement = newFrame
+    oldFrame.parentNode.replaceChild(newFrame, oldFrame)
   }
 
   // Sets the <webview>.request property.
@@ -87,12 +87,12 @@ class WebViewImpl {
 
     // Focus the BrowserPlugin when the <webview> takes focus.
     this.webviewNode.addEventListener('focus', () => {
-      this.browserPluginNode.focus()
+      this.internalElement.focus()
     })
 
     // Blur the BrowserPlugin when the <webview> loses focus.
     this.webviewNode.addEventListener('blur', () => {
-      this.browserPluginNode.blur()
+      this.internalElement.blur()
     })
   }
 
@@ -110,62 +110,11 @@ class WebViewImpl {
     this.attributes[attributeName].handleMutation(oldValue, newValue)
   }
 
-  handleBrowserPluginAttributeMutation (attributeName, oldValue, newValue) {
-    if (attributeName === webViewConstants.ATTRIBUTE_INTERNALINSTANCEID && !oldValue && !!newValue) {
-      this.browserPluginNode.removeAttribute(webViewConstants.ATTRIBUTE_INTERNALINSTANCEID)
-      this.internalInstanceId = parseInt(newValue)
-
-      // Track when the element resizes using the element resize callback.
-      webFrame.registerElementResizeCallback(this.internalInstanceId, this.onElementResize.bind(this))
-      if (this.guestInstanceId) {
-        guestViewInternal.attachGuest(this.internalInstanceId, this.guestInstanceId, this.buildParams())
-      }
-    }
-  }
-
-  onSizeChanged (webViewEvent) {
-    const {newHeight, newWidth} = webViewEvent
-    const node = this.webviewNode
-    const width = node.offsetWidth
-
-    // Check the current bounds to make sure we do not resize <webview>
-    // outside of current constraints.
-    const maxWidth = this.attributes[webViewConstants.ATTRIBUTE_MAXWIDTH].getValue() | width
-    const maxHeight = this.attributes[webViewConstants.ATTRIBUTE_MAXHEIGHT].getValue() | width
-    let minWidth = this.attributes[webViewConstants.ATTRIBUTE_MINWIDTH].getValue() | width
-    let minHeight = this.attributes[webViewConstants.ATTRIBUTE_MINHEIGHT].getValue() | width
-    minWidth = Math.min(minWidth, maxWidth)
-    minHeight = Math.min(minHeight, maxHeight)
-    if (!this.attributes[webViewConstants.ATTRIBUTE_AUTOSIZE].getValue() || (newWidth >= minWidth && newWidth <= maxWidth && newHeight >= minHeight && newHeight <= maxHeight)) {
-      node.style.width = `${newWidth}px`
-      node.style.height = `${newHeight}px`
-
-      // Only fire the DOM event if the size of the <webview> has actually
-      // changed.
-      this.dispatchEvent(webViewEvent)
-    }
-  }
-
-  onElementResize (newSize) {
-    // Dispatch the 'resize' event.
-    const resizeEvent = new Event('resize', {
-      bubbles: true
-    })
-
-    // Using client size values, because when a webview is transformed `newSize`
-    // is incorrect
-    newSize.width = this.webviewNode.clientWidth
-    newSize.height = this.webviewNode.clientHeight
-
-    resizeEvent.newWidth = newSize.width
-    resizeEvent.newHeight = newSize.height
+  onElementResize () {
+    const resizeEvent = new Event('resize', { bubbles: true })
+    resizeEvent.newWidth = this.webviewNode.clientWidth
+    resizeEvent.newHeight = this.webviewNode.clientHeight
     this.dispatchEvent(resizeEvent)
-    if (this.guestInstanceId &&
-        !this.attributes[webViewConstants.ATTRIBUTE_DISABLEGUESTRESIZE].getValue()) {
-      guestViewInternal.setSize(this.guestInstanceId, {
-        normal: newSize
-      })
-    }
   }
 
   createGuest () {
@@ -175,6 +124,7 @@ class WebViewImpl {
   }
 
   createGuestSync () {
+    this.beforeFirstNavigation = false
     this.attachGuestInstance(guestViewInternal.createGuestSync(this.buildParams()))
   }
 
@@ -225,62 +175,28 @@ class WebViewImpl {
       userAgentOverride: this.userAgentOverride
     }
     for (const attributeName in this.attributes) {
-      if (hasProp.call(this.attributes, attributeName)) {
+      if (this.attributes.hasOwnProperty(attributeName)) {
         params[attributeName] = this.attributes[attributeName].getValue()
       }
     }
-
-    // When the WebView is not participating in layout (display:none)
-    // then getBoundingClientRect() would report a width and height of 0.
-    // However, in the case where the WebView has a fixed size we can
-    // use that value to initially size the guest so as to avoid a relayout of
-    // the on display:block.
-    const css = window.getComputedStyle(this.webviewNode, null)
-    const elementRect = this.webviewNode.getBoundingClientRect()
-    params.elementWidth = parseInt(elementRect.width) || parseInt(css.getPropertyValue('width'))
-    params.elementHeight = parseInt(elementRect.height) || parseInt(css.getPropertyValue('height'))
     return params
   }
 
   attachGuestInstance (guestInstanceId) {
+    if (!this.elementAttached) {
+      // The element could be detached before we got response from browser.
+      return
+    }
+    this.internalInstanceId = getNextId()
     this.guestInstanceId = guestInstanceId
-    this.attributes[webViewConstants.ATTRIBUTE_GUESTINSTANCE].setValueIgnoreMutation(guestInstanceId)
     this.webContents = remote.getGuestWebContents(this.guestInstanceId)
-    if (!this.internalInstanceId) {
-      return true
-    }
-    return guestViewInternal.attachGuest(this.internalInstanceId, this.guestInstanceId, this.buildParams())
-  }
-}
-
-// Registers browser plugin <object> custom element.
-const registerBrowserPluginElement = function () {
-  const proto = Object.create(HTMLObjectElement.prototype)
-  proto.createdCallback = function () {
-    this.setAttribute('type', 'application/browser-plugin')
-    this.setAttribute('id', `browser-plugin-${getNextId()}`)
-
-    // The <object> node fills in the <webview> container.
-    this.style.flex = '1 1 auto'
-  }
-  proto.attributeChangedCallback = function (name, oldValue, newValue) {
-    const internal = v8Util.getHiddenValue(this, 'internal')
-    if (internal) {
-      internal.handleBrowserPluginAttributeMutation(name, oldValue, newValue)
-    }
+    guestViewInternal.attachGuest(this.internalInstanceId, this.guestInstanceId, this.buildParams(), this.internalElement.contentWindow)
+    // ResizeObserver is a browser global not recognized by "standard".
+    /* globals ResizeObserver */
+    // TODO(zcbenz): Should we deprecate the "resize" event? Wait, it is not
+    // even documented.
+    this.resizeObserver = new ResizeObserver(this.onElementResize.bind(this)).observe(this.internalElement)
   }
-  proto.attachedCallback = function () {
-    // Load the plugin immediately.
-    return this.nonExistentAttribute
-  }
-  WebViewImpl.BrowserPlugin = webFrame.registerEmbedderCustomElement('browserplugin', {
-    'extends': 'object',
-    prototype: proto
-  })
-  delete proto.createdCallback
-  delete proto.attachedCallback
-  delete proto.detachedCallback
-  delete proto.attributeChangedCallback
 }
 
 // Registers <webview> custom element.
@@ -313,12 +229,7 @@ const registerWebViewElement = function () {
     if (!internal.elementAttached) {
       guestViewInternal.registerEvents(internal, internal.viewInstanceId)
       internal.elementAttached = true
-      const instance = internal.attributes[webViewConstants.ATTRIBUTE_GUESTINSTANCE].getValue()
-      if (instance) {
-        internal.attachGuestInstance(instance)
-      } else {
-        internal.attributes[webViewConstants.ATTRIBUTE_SRC].parse()
-      }
+      internal.attributes[webViewConstants.ATTRIBUTE_SRC].parse()
     }
   }
 
@@ -450,7 +361,6 @@ const listener = function (event) {
   if (document.readyState === 'loading') {
     return
   }
-  registerBrowserPluginElement()
   registerWebViewElement()
   window.removeEventListener(event.type, listener, useCapture)
 }

+ 17 - 345
spec/webview-spec.js

@@ -5,7 +5,7 @@ const path = require('path')
 const http = require('http')
 const url = require('url')
 const {ipcRenderer, remote} = require('electron')
-const {app, session, getGuestWebContents, ipcMain, BrowserWindow, webContents} = remote
+const {app, session, ipcMain, BrowserWindow} = remote
 const {closeWindow} = require('./window-helpers')
 const {emittedOnce, waitForEvent} = require('./events-helpers')
 
@@ -676,7 +676,8 @@ describe('<webview> tag', function () {
     })
   })
 
-  describe('setDevToolsWebContents() API', () => {
+  // FIXME(zcbenz): Disabled because of moving to OOPIF webview.
+  xdescribe('setDevToolsWebContents() API', () => {
     it('sets webContents of webview as devtools', async () => {
       const webview2 = new WebView()
       loadWebView(webview2)
@@ -1130,12 +1131,15 @@ describe('<webview> tag', function () {
 
     it('updates when the window is shown after the ready-to-show event', async () => {
       const w = await openTheWindow({ show: false })
+      const readyToShowSignal = emittedOnce(w, 'ready-to-show')
+      const pongSignal1 = emittedOnce(ipcMain, 'pong')
       w.loadURL(`file://${fixtures}/pages/webview-visibilitychange.html`)
-
-      await emittedOnce(w, 'ready-to-show')
+      await pongSignal1
+      const pongSignal2 = emittedOnce(ipcMain, 'pong')
+      await readyToShowSignal
       w.show()
 
-      const [, visibilityState, hidden] = await emittedOnce(ipcMain, 'pong')
+      const [, visibilityState, hidden] = await pongSignal2
       assert(!hidden)
       assert.equal(visibilityState, 'visible')
     })
@@ -1236,232 +1240,6 @@ describe('<webview> tag', function () {
     expect(tabId).to.be.not.equal(w.webContents.id)
   })
 
-  // TODO(alexeykuzmin): Some tests rashe a renderer process.
-  // Fix them and enable the tests.
-  xdescribe('guestinstance attribute', () => {
-    it('before loading there is no attribute', () => {
-      loadWebView(webview)  // Don't wait for loading to finish.
-      assert(!webview.hasAttribute('guestinstance'))
-    })
-
-    it('loading a page sets the guest view', async () => {
-      await loadWebView(webview, {
-        src: `file://${fixtures}/api/blank.html`
-      })
-
-      const instance = webview.getAttribute('guestinstance')
-      assert.equal(instance, parseInt(instance))
-
-      const guest = getGuestWebContents(parseInt(instance))
-      assert.equal(guest, webview.getWebContents())
-    })
-
-    it('deleting the attribute destroys the webview', async () => {
-      await loadWebView(webview, {
-        src: `file://${fixtures}/api/blank.html`
-      })
-
-      const instance = parseInt(webview.getAttribute('guestinstance'))
-      const waitForDestroy = waitForEvent(webview, 'destroyed')
-      webview.removeAttribute('guestinstance')
-
-      await waitForDestroy
-      expect(getGuestWebContents(instance)).to.equal(undefined)
-    })
-
-    it('setting the attribute on a new webview moves the contents', (done) => {
-      const loadListener = () => {
-        const webContents = webview.getWebContents()
-        const instance = webview.getAttribute('guestinstance')
-
-        const destroyListener = () => {
-          assert.equal(webContents, webview2.getWebContents())
-
-          // Make sure that events are hooked up to the right webview now
-          webview2.addEventListener('console-message', (e) => {
-            assert.equal(e.message, 'a')
-            document.body.removeChild(webview2)
-            done()
-          })
-
-          webview2.src = `file://${fixtures}/pages/a.html`
-        }
-        webview.addEventListener('destroyed', destroyListener, {once: true})
-
-        const webview2 = new WebView()
-        loadWebView(webview2, {
-          guestinstance: instance
-        })
-      }
-      webview.addEventListener('did-finish-load', loadListener, {once: true})
-      loadWebView(webview, {
-        src: `file://${fixtures}/api/blank.html`
-      })
-    })
-
-    it('setting the attribute to an invalid guestinstance does nothing', async () => {
-      await loadWebView(webview, {
-        src: `file://${fixtures}/api/blank.html`
-      })
-      webview.setAttribute('guestinstance', 55)
-
-      // Make sure that events are still hooked up to the webview
-      const waitForMessage = waitForEvent(webview, 'console-message')
-      webview.src = `file://${fixtures}/pages/a.html`
-      const {message} = await waitForMessage
-      assert.equal(message, 'a')
-    })
-
-    it('setting the attribute on an existing webview moves the contents', (done) => {
-      const load1Listener = () => {
-        const webContents = webview.getWebContents()
-        const instance = webview.getAttribute('guestinstance')
-        let destroyedInstance
-
-        const destroyListener = () => {
-          assert.equal(webContents, webview2.getWebContents())
-          assert.equal(null, getGuestWebContents(parseInt(destroyedInstance)))
-
-          // Make sure that events are hooked up to the right webview now
-          webview2.addEventListener('console-message', (e) => {
-            assert.equal(e.message, 'a')
-            document.body.removeChild(webview2)
-            done()
-          })
-
-          webview2.src = 'file://' + fixtures + '/pages/a.html'
-        }
-        webview.addEventListener('destroyed', destroyListener, {once: true})
-
-        const webview2 = new WebView()
-        const load2Listener = () => {
-          destroyedInstance = webview2.getAttribute('guestinstance')
-          assert.notEqual(instance, destroyedInstance)
-
-          webview2.setAttribute('guestinstance', instance)
-        }
-        webview2.addEventListener('did-finish-load', load2Listener, {once: true})
-        loadWebView(webview2, {
-          src: `file://${fixtures}/api/blank.html`
-        })
-      }
-
-      webview.addEventListener('did-finish-load', load1Listener, {once: true})
-      loadWebView(webview, {
-        src: `file://${fixtures}/api/blank.html`
-      })
-    })
-
-    it('moving a guest back to its original webview should work', (done) => {
-      loadWebView(webview, {
-        src: `file://${fixtures}/api/blank.html`
-      }).then(() => {
-        const webContents = webview.getWebContents()
-        const instance = webview.getAttribute('guestinstance')
-
-        const destroy1Listener = () => {
-          assert.equal(webContents, webview2.getWebContents())
-          assert.notStrictEqual(webContents, webview.getWebContents())
-
-          const destroy2Listener = () => {
-            assert.equal(webContents, webview.getWebContents())
-            assert.notStrictEqual(webContents, webview2.getWebContents())
-
-            // Make sure that events are hooked up to the right webview now
-            webview.addEventListener('console-message', (e) => {
-              document.body.removeChild(webview2)
-              assert.equal(e.message, 'a')
-              done()
-            })
-
-            webview.src = `file://${fixtures}/pages/a.html`
-          }
-          webview2.addEventListener('destroyed', destroy2Listener, {once: true})
-          webview.setAttribute('guestinstance', instance)
-        }
-        webview.addEventListener('destroyed', destroy1Listener, {once: true})
-
-        const webview2 = new WebView()
-        loadWebView(webview2, {guestinstance: instance})
-      })
-    })
-
-    // FIXME(alexeykuzmin): This test only passes if the previous test ^
-    // is run alongside.
-    it('setting the attribute on a webview in a different window moves the contents', (done) => {
-      loadWebView(webview, {
-        src: `file://${fixtures}/api/blank.html`
-      }).then(() => {
-        const instance = webview.getAttribute('guestinstance')
-
-        w = new BrowserWindow({ show: false })
-        w.webContents.once('did-finish-load', () => {
-          ipcMain.once('pong', () => {
-            assert(!webview.hasAttribute('guestinstance'))
-            done()
-          })
-
-          w.webContents.send('guestinstance', instance)
-        })
-        w.loadURL(`file://${fixtures}/pages/webview-move-to-window.html`)
-      })
-    })
-
-    it('does not delete the guestinstance attribute when moving the webview to another parent node', (done) => {
-      webview.addEventListener('dom-ready', function domReadyListener () {
-        webview.addEventListener('did-attach', () => {
-          assert(webview.guestinstance != null)
-          assert(webview.getWebContents() != null)
-          done()
-        })
-
-        document.body.replaceChild(webview, div)
-      })
-      webview.src = `file://${fixtures}/pages/a.html`
-
-      const div = document.createElement('div')
-      div.appendChild(webview)
-      document.body.appendChild(div)
-    })
-
-    it('does not destroy the webContents when hiding/showing the webview (regression)', (done) => {
-      webview.addEventListener('dom-ready', function () {
-        const instance = webview.getAttribute('guestinstance')
-        assert(instance != null)
-
-        // Wait for event directly since attach happens asynchronously over IPC
-        ipcMain.once('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', () => {
-          assert(webview.getWebContents() != null)
-          assert.equal(instance, webview.getAttribute('guestinstance'))
-          done()
-        })
-
-        webview.style.display = 'none'
-        webview.offsetHeight // eslint-disable-line
-        webview.style.display = 'block'
-      }, {once: true})
-      loadWebView(webview, {src: `file://${fixtures}/pages/a.html`})
-    })
-
-    it('does not reload the webContents when hiding/showing the webview (regression)', (done) => {
-      webview.addEventListener('dom-ready', function () {
-        webview.addEventListener('did-start-loading', () => {
-          done(new Error('webview started loading unexpectedly'))
-        })
-
-        // Wait for event directly since attach happens asynchronously over IPC
-        webview.addEventListener('did-attach', () => {
-          done()
-        })
-
-        webview.style.display = 'none'
-        webview.offsetHeight  // eslint-disable-line
-        webview.style.display = 'block'
-      }, {once: true})
-      loadWebView(webview, {src: `file://${fixtures}/pages/a.html`})
-    })
-  })
-
   describe('DOM events', () => {
     let div
 
@@ -1479,140 +1257,34 @@ describe('<webview> tag', function () {
     })
 
     it('emits resize events', async () => {
+      const firstResizeSignal = waitForEvent(webview, 'resize')
+      const domReadySignal = waitForEvent(webview, 'dom-ready')
+
       webview.src = `file://${fixtures}/pages/a.html`
       div.appendChild(webview)
       document.body.appendChild(div)
 
-      const firstResizeEvent = await waitForEvent(webview, 'resize')
+      const firstResizeEvent = await firstResizeSignal
       expect(firstResizeEvent.target).to.equal(webview)
       expect(firstResizeEvent.newWidth).to.equal(100)
       expect(firstResizeEvent.newHeight).to.equal(10)
 
-      await waitForEvent(webview, 'dom-ready')
+      await domReadySignal
+
+      const secondResizeSignal = waitForEvent(webview, 'resize')
 
       const newWidth = 1234
       const newHeight = 789
       div.style.width = `${newWidth}px`
       div.style.height = `${newHeight}px`
 
-      const secondResizeEvent = await waitForEvent(webview, 'resize')
+      const secondResizeEvent = await secondResizeSignal
       expect(secondResizeEvent.target).to.equal(webview)
       expect(secondResizeEvent.newWidth).to.equal(newWidth)
       expect(secondResizeEvent.newHeight).to.equal(newHeight)
     })
   })
 
-  describe('disableguestresize attribute', () => {
-    it('does not have attribute by default', () => {
-      loadWebView(webview)
-      assert(!webview.hasAttribute('disableguestresize'))
-    })
-
-    it('resizes guest when attribute is not present', async () => {
-      const INITIAL_SIZE = 200
-      const w = await openTheWindow(
-          {show: false, width: INITIAL_SIZE, height: INITIAL_SIZE})
-      w.loadURL(`file://${fixtures}/pages/webview-guest-resize.html`)
-      await emittedOnce(ipcMain, 'webview-loaded')
-
-      const elementResize = emittedOnce(ipcMain, 'webview-element-resize')
-          .then(([, width, height]) => {
-            assert.equal(width, CONTENT_SIZE)
-            assert.equal(height, CONTENT_SIZE)
-          })
-
-      const guestResize = emittedOnce(ipcMain, 'webview-guest-resize')
-          .then(([, width, height]) => {
-            assert.equal(width, CONTENT_SIZE)
-            assert.equal(height, CONTENT_SIZE)
-          })
-
-      const CONTENT_SIZE = 300
-      assert(CONTENT_SIZE !== INITIAL_SIZE)
-      w.setContentSize(CONTENT_SIZE, CONTENT_SIZE)
-
-      return Promise.all([elementResize, guestResize])
-    })
-
-    // TODO(alexeykuzmin): [Ch66] Enable the test.
-    xit('does not resize guest when attribute is present', async () => {
-      const INITIAL_SIZE = 200
-      const w = await openTheWindow(
-          {show: false, width: INITIAL_SIZE, height: INITIAL_SIZE})
-      w.loadURL(`file://${fixtures}/pages/webview-no-guest-resize.html`)
-      await emittedOnce(ipcMain, 'webview-loaded')
-
-      const noGuestResizePromise = Promise.race([
-        emittedOnce(ipcMain, 'webview-guest-resize'),
-        new Promise(resolve => setTimeout(() => resolve(), 500))
-      ]).then((eventData = null) => {
-        if (eventData !== null) {
-          // Means we got the 'webview-guest-resize' event before the time out.
-          return Promise.reject(new Error('Unexpected guest resize message'))
-        }
-      })
-
-      const CONTENT_SIZE = 300
-      assert(CONTENT_SIZE !== INITIAL_SIZE)
-      w.setContentSize(CONTENT_SIZE, CONTENT_SIZE)
-
-      return noGuestResizePromise
-    })
-
-    // TODO(alexeykuzmin): [Ch66] Enable the test.
-    xit('dispatches element resize event even when attribute is present', async () => {
-      const INITIAL_SIZE = 200
-      const w = await openTheWindow(
-          {show: false, width: INITIAL_SIZE, height: INITIAL_SIZE})
-      w.loadURL(`file://${fixtures}/pages/webview-no-guest-resize.html`)
-      await emittedOnce(ipcMain, 'webview-loaded')
-
-      const elementResizePromise = emittedOnce(ipcMain, 'webview-element-resize')
-          .then(([, width, height]) => {
-            expect(width).to.equal(CONTENT_SIZE)
-            expect(height).to.equal(CONTENT_SIZE)
-          })
-
-      const CONTENT_SIZE = 300
-      assert(CONTENT_SIZE !== INITIAL_SIZE)
-      w.setContentSize(CONTENT_SIZE, CONTENT_SIZE)
-
-      return elementResizePromise
-    })
-
-    // TODO(alexeykuzmin): [Ch66] Enable the test.
-    xit('can be manually resized with setSize even when attribute is present', async () => {
-      const INITIAL_SIZE = 200
-      const w = await openTheWindow(
-          {show: false, width: INITIAL_SIZE, height: INITIAL_SIZE})
-      w.loadURL(`file://${fixtures}/pages/webview-no-guest-resize.html`)
-      await emittedOnce(ipcMain, 'webview-loaded')
-
-      const GUEST_WIDTH = 10
-      const GUEST_HEIGHT = 20
-
-      const guestResizePromise = emittedOnce(ipcMain, 'webview-guest-resize')
-          .then(([, width, height]) => {
-            expect(width).to.be.equal(GUEST_WIDTH)
-            expect(height).to.be.equal(GUEST_HEIGHT)
-          })
-
-      const wc = webContents.getAllWebContents().find((wc) => {
-        return wc.hostWebContents &&
-            wc.hostWebContents.id === w.webContents.id
-      })
-      assert(wc)
-      wc.setSize({
-        normal: {
-          width: GUEST_WIDTH,
-          height: GUEST_HEIGHT
-        }
-      })
-
-      return guestResizePromise
-    })
-  })
-
   describe('zoom behavior', () => {
     const zoomScheme = remote.getGlobal('zoomScheme')
     const webviewSession = session.fromPartition('webview-temp')