New issue
Advanced search Search tips

Issue 821187 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Allow creation of inner WebContents without BrowserPluginGuestDelegate

Project Member Reported by nick@chromium.org, Mar 12 2018

Issue description

Currently, GuestMode::IsCrossProcessFrameGuest returns false if there's no browser_plugin_guest so calling AttachToOuterWebContentsFrame will hit a CHECK unless there is a browser_plugin_guest. Thus, it is not possible currently to create an inner WebContents without a browser_plugin_guest, even though the OOPIF architecture things should just work even without the guest delegate.

We can consider this bug fixed when CreateAndAttachInnerContents() in test_utils.cc no longer needs to set inner_params.guest_delegate. Perhaps that creation parameter should be replaced with a boolean.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d73635b06b597ee7ef97b416e734025a48d37fc3

commit d73635b06b597ee7ef97b416e734025a48d37fc3
Author: Nick Carter <nick@chromium.org>
Date: Tue Mar 13 18:31:41 2018

Add mechanism to test inner WebContents from a content_browsertest.

Write a test of mouse_lock_widget_ cleanup. Fix WebContentsImpl so that it passes.

Bug: 820593,  821187 

Change-Id: I99cd6b8c728f7e4501205574db8d17b206f2856e
Reviewed-on: https://chromium-review.googlesource.com/957479
Commit-Queue: Nick Carter <nick@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542865}
[modify] https://crrev.com/d73635b06b597ee7ef97b416e734025a48d37fc3/content/browser/pointer_lock_browsertest.cc
[modify] https://crrev.com/d73635b06b597ee7ef97b416e734025a48d37fc3/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/d73635b06b597ee7ef97b416e734025a48d37fc3/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/d73635b06b597ee7ef97b416e734025a48d37fc3/content/public/test/test_utils.cc
[modify] https://crrev.com/d73635b06b597ee7ef97b416e734025a48d37fc3/content/public/test/test_utils.h

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 23 2018

Labels: merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/934e2e2a1b1d1448b04e8d5df2f3d910d0867be3

commit 934e2e2a1b1d1448b04e8d5df2f3d910d0867be3
Author: Nick Carter <nick@chromium.org>
Date: Fri Mar 23 17:04:40 2018

Add mechanism to test inner WebContents from a content_browsertest.

Write a test of mouse_lock_widget_ cleanup. Fix WebContentsImpl so that it passes.

Bug: 820593,  821187 

Change-Id: I99cd6b8c728f7e4501205574db8d17b206f2856e
Reviewed-on: https://chromium-review.googlesource.com/957479
Commit-Queue: Nick Carter <nick@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#542865}(cherry picked from commit d73635b06b597ee7ef97b416e734025a48d37fc3)
Reviewed-on: https://chromium-review.googlesource.com/978482
Cr-Commit-Position: refs/branch-heads/3359@{#400}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/934e2e2a1b1d1448b04e8d5df2f3d910d0867be3/content/browser/pointer_lock_browsertest.cc
[modify] https://crrev.com/934e2e2a1b1d1448b04e8d5df2f3d910d0867be3/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/934e2e2a1b1d1448b04e8d5df2f3d910d0867be3/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/934e2e2a1b1d1448b04e8d5df2f3d910d0867be3/content/public/test/test_utils.cc
[modify] https://crrev.com/934e2e2a1b1d1448b04e8d5df2f3d910d0867be3/content/public/test/test_utils.h

> even though the OOPIF architecture things should just work even without the guest delegate.

Unfortunately, BrowserPluginGuestDelegate is currently used the communication bridge from content/ into the general guest-view architecture [both OOPIF and legacy paths]. 

e.g. WebContentsImpl::CreateNewWindow calls GetBrowserPluginGuest()->CreateNewGuestWindow(create_params));  

which is used to create a new WebContents instance that is associated with a "guest view". As such, we will not be able to fully get rid of BrowserPluginGuestDelegate. We could probably rename/split out behavior into a smaller subclass.


It was always expected that BrowserPluginGuestDelegate would need to be refactored in this way. Since BrowserPlugin will be going away when ekaramad@ gets PDF (MimeHandlerGuest) moved to OOPIF, we had always planned to do it at that time, but if it needs to happen sooner that's fine too.
Project Member

Comment 5 by bugdroid1@chromium.org, May 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/105de1e81e85b5bffae5612495b1c3e57819606d

commit 105de1e81e85b5bffae5612495b1c3e57819606d
Author: Lucas Furukawa Gadani <lfg@chromium.org>
Date: Thu May 31 19:38:39 2018

Allow inner WebContents without a BroserPluginGuestDelegate.

With the implementation of out-of-process iframes based GuestViews,
it should no longer be necessary to have a BrowserPluginGuestDelegate
when attaching an inner WebContents to an outer WebContents.

Bug:  821187 

Change-Id: Idf01cd486b6fb55b5df35a779984bdb19fd46e0d
Reviewed-on: https://chromium-review.googlesource.com/1077197
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Lucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563352}
[modify] https://crrev.com/105de1e81e85b5bffae5612495b1c3e57819606d/chrome/browser/apps/platform_app_navigation_redirector.cc
[modify] https://crrev.com/105de1e81e85b5bffae5612495b1c3e57819606d/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/105de1e81e85b5bffae5612495b1c3e57819606d/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/105de1e81e85b5bffae5612495b1c3e57819606d/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/105de1e81e85b5bffae5612495b1c3e57819606d/content/public/browser/web_contents.h
[modify] https://crrev.com/105de1e81e85b5bffae5612495b1c3e57819606d/content/public/test/test_utils.cc

Comment 6 by lfg@chromium.org, Jun 15 2018

Status: Fixed (was: Assigned)
There may still be features that won't work when creating an inner WebContents without a BrowserPluginGuestDelegate, but the main purpose of this bug, which is to be able to create an inner content without the delegate for testing purposes should be working. Marking as fixed.

Sign in to add a comment