New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 659750 link

Starred by 10 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Feature


Sign in to add a comment

Convert MimeHandlerViewGuest to load via OOPIF instead of BrowserPlugin

Project Member Reported by wjmaclean@chromium.org, Oct 26 2016

Issue description

This is a tracking bug for the work to make PDFs display inside an oopif-based GuestView.
 
Cc: mknowles@chromium.org
 Issue 642826  has been merged into this issue.
 Issue 659772  has been merged into this issue.
Cc: thestig@chromium.org dsinclair@chromium.org fsam...@chromium.org
 Issue 600214  has been merged into this issue.
Cc: -ekaramad@chromium.org aval...@chromium.org
Owner: ekaramad@chromium.org

Comment 5 by lfg@chromium.org, Aug 22 2017

Blocking: 732393
Blockedon: 776510
Components: Internals>Plugins>PDF

Comment 8 by mcnee@chromium.org, Jan 2 2018

Cc: -mcnee@google.com mcnee@chromium.org
Blockedon: 781580
Blocking: 510568

Comment 11 by lfg@chromium.org, Mar 15 2018

Blocking: 464269
Blocking: 832547
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 27

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

commit 5053971dd2543d576a84eb883b8edf92fbf57517
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Fri Jul 27 07:07:46 2018

Introduce API for external handling of plugins

When the contents of a plugin element (<embed> and <object>) are to be
handeld externally inside an extension (most notably PDF) we currently
use browser plugin. BrowserPlugin is used to render GuestView contents
in another process.

However, BrowserPlugin-based guest views have been deprecated and all
guest views except for MimeHandlerViewGuest are not implemented on top
of cross-process frames.

This CL introduces the first steps in fully replacing BrowserPlugin with
corss-process frames.

Different mechanisms for this project have already been discussed in the
design doc:
https://docs.google.com/document/d/10g7Y9cprYKkch9JZ0TBUWaEnHBJT1nzhskQIt1nHbWM/edit#heading=h.ue5a8s290yhk

Bug: 659750, 330264
Change-Id: If273fbbab3e9f4a4591c61b19d54e4cca73c3464
Reviewed-on: https://chromium-review.googlesource.com/1101161
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578554}
[modify] https://crrev.com/5053971dd2543d576a84eb883b8edf92fbf57517/third_party/blink/public/web/web_local_frame_client.h
[modify] https://crrev.com/5053971dd2543d576a84eb883b8edf92fbf57517/third_party/blink/renderer/core/exported/local_frame_client_impl.cc
[modify] https://crrev.com/5053971dd2543d576a84eb883b8edf92fbf57517/third_party/blink/renderer/core/exported/local_frame_client_impl.h
[modify] https://crrev.com/5053971dd2543d576a84eb883b8edf92fbf57517/third_party/blink/renderer/core/frame/local_frame_client.h
[modify] https://crrev.com/5053971dd2543d576a84eb883b8edf92fbf57517/third_party/blink/renderer/core/html/html_plugin_element.cc
[modify] https://crrev.com/5053971dd2543d576a84eb883b8edf92fbf57517/third_party/blink/renderer/core/html/html_plugin_element.h

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 2

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

commit e411392c3633e4dbbf5fb059f79ca10bec976f9b
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Thu Aug 02 17:48:37 2018

Content feature for MimeHandlerView in OOPF

This CL introduces a bare content feature to allow landing experimental
work and future CLs on MimeHandlerView using cross process frames. The
feature will be disabled by default.

MimeHandlerViewContainer is currently the last remaining GuestView
which still relies on BrowserPlugin. Eventually and starting with this
CL, the new implementation of MimeHandlerView will be based on
cross-process frames that no longer requires a BrowserPlugin.

Bug: 659750, 330264
Change-Id: I52a8eba4e9f2446ae0d899cd3a639f418bc01d12
Reviewed-on: https://chromium-review.googlesource.com/1073923
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580240}
[modify] https://crrev.com/e411392c3633e4dbbf5fb059f79ca10bec976f9b/content/public/common/content_features.cc
[modify] https://crrev.com/e411392c3633e4dbbf5fb059f79ca10bec976f9b/content/public/common/content_features.h

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 3

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

commit 378384acf5e76872e88c144e4ab8d154ce533f84
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Fri Aug 03 21:26:46 2018

Make a BrowserPluginSpecific test a MimeHandlerViewTest

All GuestViews except for MimeHandlerViewGuest are now implemented based on
cross-process frames. There is however a hadnful of browser tests which are not
MimeHandlerViewTest but still use BrowserPlugin.

This CL will
  1- Change the MimeHandlerViewTest from parameteric to normal test.
  Essentially, the UseCrossProcessFramesForGuest flag is unrelated to
  the MimeHandlerViewGuest. There is a separate flag for that matter
  which is "MimeHandlerViewInCrossProcessFrame" and will be added
  eventually when the a first version of MimeHandlerViewGuest based on
  cross-process frames is implemented on ToT.

  2- Move WebViewBrowserPluginSpecificTest.AcceptTouchEvents to
  MimeHandlerViewTest. This involves overhauling the test test is also
  modified to load MimeHandlerViewGuest instead of WebViewGuest.

The short-term plan is to change all such (BrowserPluginSpecific) tests
to become MimeHandlerViewBrowserPluginSpecificTest. The longer term goal
is to remove all such tests when MimeHandlerViewInCrossProcessFrames
is shipped.

Bug: 659750, 533069, 330264
Change-Id: Icad9ddd1f2d1e6851b20f6c0d923dd2192b7dcec
Reviewed-on: https://chromium-review.googlesource.com/1161745
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580655}
[modify] https://crrev.com/378384acf5e76872e88c144e4ab8d154ce533f84/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/378384acf5e76872e88c144e4ab8d154ce533f84/chrome/test/data/extensions/api_test/mime_handler_view/index.js
[modify] https://crrev.com/378384acf5e76872e88c144e4ab8d154ce533f84/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Aug 8

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

commit ed87958c709c96e0a151ab83e782594d3f8dc6c3
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Wed Aug 08 02:02:29 2018

Handling plugins externally for MimeHandlerView

This CL routes the call IsPluginHandledExternally() to the extensions
layer.

The work here will soon be followed up by followup CLs which implement
the logic MimeHandlerViewGuest based on cross-process frames.

PoC CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1121680

Design Doc:
https://docs.google.com/document/d/10g7Y9cprYKkch9JZ0TBUWaEnHBJT1nzhskQIt1nHbWM/edit#heading=h.ue5a8s290yhk

Bug: 659750
Change-Id: If8546f80842c726fa95aba702f35e6d594fc09e8
Reviewed-on: https://chromium-review.googlesource.com/1161115
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581440}
[modify] https://crrev.com/ed87958c709c96e0a151ab83e782594d3f8dc6c3/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/ed87958c709c96e0a151ab83e782594d3f8dc6c3/chrome/renderer/chrome_content_renderer_client.h
[modify] https://crrev.com/ed87958c709c96e0a151ab83e782594d3f8dc6c3/chrome/renderer/extensions/chrome_extensions_renderer_client.cc
[modify] https://crrev.com/ed87958c709c96e0a151ab83e782594d3f8dc6c3/chrome/renderer/extensions/chrome_extensions_renderer_client.h
[modify] https://crrev.com/ed87958c709c96e0a151ab83e782594d3f8dc6c3/content/public/common/BUILD.gn
[add] https://crrev.com/ed87958c709c96e0a151ab83e782594d3f8dc6c3/content/public/common/mime_handler_view_mode.cc
[add] https://crrev.com/ed87958c709c96e0a151ab83e782594d3f8dc6c3/content/public/common/mime_handler_view_mode.h
[modify] https://crrev.com/ed87958c709c96e0a151ab83e782594d3f8dc6c3/content/public/renderer/content_renderer_client.cc
[modify] https://crrev.com/ed87958c709c96e0a151ab83e782594d3f8dc6c3/content/public/renderer/content_renderer_client.h
[modify] https://crrev.com/ed87958c709c96e0a151ab83e782594d3f8dc6c3/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/ed87958c709c96e0a151ab83e782594d3f8dc6c3/content/renderer/render_frame_impl.h

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 10

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

commit 8eb68052e037952a13565c5545ca3fdc9dbac2b9
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Fri Aug 10 21:39:23 2018

Move BrowserPluginSpecific tests out of web_view_browsertest.cc

The WebViewBrowserPluginSpecificTest class includes tests which use a
BrowserPluign-based GuestView. Since all GuestViews except MimeHandlerViewGuest
are now based on cross-process frames, these tests should not be implemented as
WebViewGuest tests anymore.

This CL introduces a new chrome browser test target for such
BrowserPluginSpecific tests where the tests instantiate a MimeHandlerViewGuest
instead of a <webview>.

Ideally, the tests should have lived with the rest of MimeHandlerViewTests in
extension layer; however, due to chrome layer dependencies it is not possible.

Bug: 659750, 533069, 330264, 870604
Change-Id: Ica252a60e94bad47cabbff8b9275b8f9fb05d7ae
Reviewed-on: https://chromium-review.googlesource.com/1168734
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582345}
[modify] https://crrev.com/8eb68052e037952a13565c5545ca3fdc9dbac2b9/chrome/browser/apps/guest_view/web_view_browsertest.cc
[add] https://crrev.com/8eb68052e037952a13565c5545ca3fdc9dbac2b9/chrome/browser/guest_view/mime_handler_view/chrome_mime_handler_view_browsertest.cc
[modify] https://crrev.com/8eb68052e037952a13565c5545ca3fdc9dbac2b9/chrome/test/BUILD.gn
[modify] https://crrev.com/8eb68052e037952a13565c5545ca3fdc9dbac2b9/chrome/test/data/extensions/api_test/mime_handler_view/index.js

Project Member

Comment 18 by bugdroid1@chromium.org, Aug 13

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

commit 040df725447fda691ec72eeea88916b442f5a867
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Mon Aug 13 04:49:59 2018

Refactor MimeHandlerViewContainer to use with cross-process version

MimeHandlerViewContainer implements the renderer side of logic for
embedding the MimeHandlerView extension. The logic is currently based
on a GuestViewContainer which internally uses a WebPlugin
(BrowserPlugin).

MimeHandlerView will eventually get implemented on top of cross-process
frames architecture; requiring its owner container class on the
embedder side.

This CL will extract the common code between the current BrowserPlugin
version and the forthcoming frame-based version into a base class so
that the similar code between the two containers is not duplicated.

The common code includes a) logic in creating MimeHandlerViewGuest,
b) support for network service and c) support for post message.

Bug: 659750
Change-Id: Iabe44b1438fc22aa052b1ebf6d8696bdca00d0c9
Reviewed-on: https://chromium-review.googlesource.com/1170564
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582504}
[modify] https://crrev.com/040df725447fda691ec72eeea88916b442f5a867/chrome/renderer/printing/chrome_print_render_frame_helper_delegate.cc
[modify] https://crrev.com/040df725447fda691ec72eeea88916b442f5a867/extensions/renderer/BUILD.gn
[modify] https://crrev.com/040df725447fda691ec72eeea88916b442f5a867/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc
[modify] https://crrev.com/040df725447fda691ec72eeea88916b442f5a867/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.h
[add] https://crrev.com/040df725447fda691ec72eeea88916b442f5a867/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.cc
[add] https://crrev.com/040df725447fda691ec72eeea88916b442f5a867/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.h

Project Member

Comment 19 by bugdroid1@chromium.org, Aug 22

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

commit 95ba2baa685ff32148a414ed72d4baec6803d0c7
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Wed Aug 22 14:20:42 2018

Fix a methods dec. in MimeHandlerViewContainerBase

The method CreateMimeHandlerViewGuestIfNecessary() has an implementation
in mime_handler_view_container_base.cc. It is marked as abstract by
mistake.

TBR=wjmaclean@chromium.org

Bug: 659750
Change-Id: If4751752f05ff120f47c7d65a5a6c3189aa08ac3
Reviewed-on: https://chromium-review.googlesource.com/1184119
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585007}
[modify] https://crrev.com/95ba2baa685ff32148a414ed72d4baec6803d0c7/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.h

Project Member

Comment 20 by bugdroid1@chromium.org, Sep 6

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

commit 92df23c1ce15615ba52698264485e8785103492c
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Thu Sep 06 20:48:56 2018

Implement MimeHandlerViewFrameController

This CL implements the renderer side of MimeHandlerView based on
cross-process frames (behind a flag).

The new subclass of MimeHandlerViewContainerBase will use a content
frame inside a plugin element for loading the MimeHandlerViewGuest's
extension.

The current implementation is not complete and the missing features
such as postMessage support will be added in the follow-up CLs.

This CL also introduces a new class of parametric browser tests
(MimeHandlerViewCrossProcessTest) which will be the destination of
the current MimeHandlerViewTests as more features are integrated into
the frame-based version. The new test class runs the test on both
BrowserPlugin-based and cross-process-frame-based versions.

Document: https://docs.google.com/document/d/10g7Y9cprYKkch9JZ0TBUWaEnHBJT1nzhskQIt1nHbWM/edit#heading=h.qw81a2bk3v6w

Bug: 659750
TBR=thestig@chromium.org

Change-Id: Ia69aeed47f4fa1c7f5b81cdde71a8d8b5ff59165
Reviewed-on: https://chromium-review.googlesource.com/1187231
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589284}
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/chrome/renderer/chrome_content_renderer_client.h
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/chrome/renderer/extensions/chrome_extensions_renderer_client.cc
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/chrome/renderer/extensions/chrome_extensions_renderer_client.h
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/components/guest_view/renderer/guest_view_container_dispatcher.h
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/content/public/browser/guest_host.h
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/content/public/renderer/content_renderer_client.cc
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/content/public/renderer/content_renderer_client.h
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/browser/guest_view/extensions_guest_view_message_filter.h
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/browser/guest_view/mime_handler_view/mime_handler_view_constants.cc
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/browser/guest_view/mime_handler_view/mime_handler_view_constants.h
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/common/mojo/guest_view.mojom
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/renderer/BUILD.gn
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/renderer/guest_view/extensions_guest_view_container_dispatcher.cc
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/renderer/guest_view/extensions_guest_view_container_dispatcher.h
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.h
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.cc
[modify] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.h
[add] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.cc
[add] https://crrev.com/92df23c1ce15615ba52698264485e8785103492c/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.h

Project Member

Comment 21 by bugdroid1@chromium.org, Sep 7

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

commit 97a781683ef159a365aa395bc91a541c0364beab
Author: Markus Heintz <markusheintz@chromium.org>
Date: Fri Sep 07 12:26:17 2018

Revert "Implement MimeHandlerViewFrameController"

This reverts commit 92df23c1ce15615ba52698264485e8785103492c.

Reason for revert: 
Failure size on mac_rel

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/mac-rel/546

speculative revert due to:
 HINT: To get this list, run tools/mac/show_mod_init_func.py
# HINT: diff against the log from the last run to see what changed
/b/s/w/ir/cache/builder/src/out/Release/Chromium Framework.unstripped
0x0000000001fe9410 @ _GLOBAL__sub_I_mime_handler_view_frame_container.cc (in Chromium Framework.unstripped) (mime_handler_view_frame_container.cc:0)


Original change's description:
> Implement MimeHandlerViewFrameController
> 
> This CL implements the renderer side of MimeHandlerView based on
> cross-process frames (behind a flag).
> 
> The new subclass of MimeHandlerViewContainerBase will use a content
> frame inside a plugin element for loading the MimeHandlerViewGuest's
> extension.
> 
> The current implementation is not complete and the missing features
> such as postMessage support will be added in the follow-up CLs.
> 
> This CL also introduces a new class of parametric browser tests
> (MimeHandlerViewCrossProcessTest) which will be the destination of
> the current MimeHandlerViewTests as more features are integrated into
> the frame-based version. The new test class runs the test on both
> BrowserPlugin-based and cross-process-frame-based versions.
> 
> Document: https://docs.google.com/document/d/10g7Y9cprYKkch9JZ0TBUWaEnHBJT1nzhskQIt1nHbWM/edit#heading=h.qw81a2bk3v6w
> 
> Bug: 659750
> TBR=thestig@chromium.org
> 
> Change-Id: Ia69aeed47f4fa1c7f5b81cdde71a8d8b5ff59165
> Reviewed-on: https://chromium-review.googlesource.com/1187231
> Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
> Reviewed-by: James MacLean <wjmaclean@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589284}

TBR=lazyboy@chromium.org,thestig@chromium.org,sammc@chromium.org,alexmos@chromium.org,ekaramad@chromium.org,wjmaclean@chromium.org

Change-Id: Ice96f16404a1026693a72425535929bd883264fe
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 659750
Reviewed-on: https://chromium-review.googlesource.com/1213148
Reviewed-by: Markus Heintz <markusheintz@chromium.org>
Commit-Queue: Markus Heintz <markusheintz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589497}
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/chrome/renderer/chrome_content_renderer_client.h
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/chrome/renderer/extensions/chrome_extensions_renderer_client.cc
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/chrome/renderer/extensions/chrome_extensions_renderer_client.h
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/components/guest_view/renderer/guest_view_container_dispatcher.h
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/content/public/browser/guest_host.h
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/content/public/renderer/content_renderer_client.cc
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/content/public/renderer/content_renderer_client.h
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/browser/guest_view/extensions_guest_view_message_filter.h
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/browser/guest_view/mime_handler_view/mime_handler_view_constants.cc
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/browser/guest_view/mime_handler_view/mime_handler_view_constants.h
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/common/mojo/guest_view.mojom
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/renderer/BUILD.gn
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/renderer/guest_view/extensions_guest_view_container_dispatcher.cc
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/renderer/guest_view/extensions_guest_view_container_dispatcher.h
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.h
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.cc
[modify] https://crrev.com/97a781683ef159a365aa395bc91a541c0364beab/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.h
[delete] https://crrev.com/7431ee0cbf641a735c6e6cd91aafdc19f4cfd668/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.cc
[delete] https://crrev.com/7431ee0cbf641a735c6e6cd91aafdc19f4cfd668/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.h

Project Member

Comment 22 by bugdroid1@chromium.org, Sep 9

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

commit dce6d45e03ac15b11191948e51f4157f03d18047
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Sun Sep 09 18:49:19 2018

Reland "Implement MimeHandlerViewFrameController"

The initial attempt was causeing build failures in "mac-rel" bot.

The cause seems to be the use of static initializers. This CL
moves them to a method.

This is a reland of 92df23c1ce15615ba52698264485e8785103492c

Original change's description:
> Implement MimeHandlerViewFrameController
>
> This CL implements the renderer side of MimeHandlerView based on
> cross-process frames (behind a flag).
>
> The new subclass of MimeHandlerViewContainerBase will use a content
> frame inside a plugin element for loading the MimeHandlerViewGuest's
> extension.
>
> The current implementation is not complete and the missing features
> such as postMessage support will be added in the follow-up CLs.
>
> This CL also introduces a new class of parametric browser tests
> (MimeHandlerViewCrossProcessTest) which will be the destination of
> the current MimeHandlerViewTests as more features are integrated into
> the frame-based version. The new test class runs the test on both
> BrowserPlugin-based and cross-process-frame-based versions.
>
> Document: https://docs.google.com/document/d/10g7Y9cprYKkch9JZ0TBUWaEnHBJT1nzhskQIt1nHbWM/edit#heading=h.qw81a2bk3v6w
>
> Bug: 659750
> TBR=thestig@chromium.org
>
> Change-Id: Ia69aeed47f4fa1c7f5b81cdde71a8d8b5ff59165
> Reviewed-on: https://chromium-review.googlesource.com/1187231
> Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
> Reviewed-by: James MacLean <wjmaclean@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589284}

TBR=alexmos@chromium.org,lazyboy@chromium.org,sammc@chromium.org,wjmaclean@chromium.org

Bug: 659750,  881696 
Change-Id: I94c779664981738222c8d79ca0b10ab06174a4a7
Reviewed-on: https://chromium-review.googlesource.com/1213369
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589807}
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/chrome/renderer/chrome_content_renderer_client.h
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/chrome/renderer/extensions/chrome_extensions_renderer_client.cc
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/chrome/renderer/extensions/chrome_extensions_renderer_client.h
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/components/guest_view/renderer/guest_view_container_dispatcher.h
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/content/public/browser/guest_host.h
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/content/public/renderer/content_renderer_client.cc
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/content/public/renderer/content_renderer_client.h
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/browser/guest_view/extensions_guest_view_message_filter.h
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/browser/guest_view/mime_handler_view/mime_handler_view_constants.cc
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/browser/guest_view/mime_handler_view/mime_handler_view_constants.h
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/common/mojo/guest_view.mojom
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/renderer/BUILD.gn
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/renderer/guest_view/extensions_guest_view_container_dispatcher.cc
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/renderer/guest_view/extensions_guest_view_container_dispatcher.h
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.h
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.cc
[modify] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.h
[add] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.cc
[add] https://crrev.com/dce6d45e03ac15b11191948e51f4157f03d18047/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.h

Project Member

Comment 23 by bugdroid1@chromium.org, Sep 12

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

commit 631eee44d19571625528a44d0d4de7748df6a1b9
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Wed Sep 12 14:21:54 2018

BadMessage for invalid plugin frame ID

(Follow-up work to CL:1213369).

Currently API in guest_view.mojom send two routing IDs to the browser:
the embedder frame routing ID (the frame which adds an <embed>/<object>)
and the plugin frame ID (the actual frame inside <embed>/<object>). This
is to support both a frame-based MimeHandlerView (which only needs the
plugin frame ID), and the BrowserPlugin-based version which uses the
embedder frame ID.

This CL adds a security check on the browser side to verify the reported
routing IDs make sense, i.e., when they point to actual RFH we always
observer the child and parent relationship. This of course is not the
case in BP-based case where we expect the child routing ID to be invalid
all the time.

Bug: 659750
Change-Id: I92f1643303899054cdee36897e7a973a5a3b3d85
Reviewed-on: https://chromium-review.googlesource.com/1217704
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590668}
[modify] https://crrev.com/631eee44d19571625528a44d0d4de7748df6a1b9/extensions/browser/bad_message.h
[modify] https://crrev.com/631eee44d19571625528a44d0d4de7748df6a1b9/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
[modify] https://crrev.com/631eee44d19571625528a44d0d4de7748df6a1b9/tools/metrics/histograms/enums.xml

Project Member

Comment 24 by bugdroid1@chromium.org, Sep 19

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

commit 361cc6fa6e2bc663c5492cad0dfea3b2c88330bd
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Wed Sep 19 20:40:21 2018

Fix (frame-based) MimeHandlerViewGuest Attaching

The CL 1187231 implemented MimeHandlerViewFrameContainer. This is a
follow-up to that CL to fix some major issues in attaching a MHVG on
the browser side.

The main problems and updates:
  1- The test added in 1187231 was inadvertently removed in Patch 4.
     This caused the incorrect code to land without flagging. This CL
     adds the test again (test runs with
     MimeHandlerViewInCrossProcessFrame).
  2- MimeHandlerViewGuest::WillBeginAttach was using
     |embedder_web_contents()| which is null at this point.
  3- The newly introduced code path for attaching Guests on the browser
     side would end up in code paths which expected RWHVGuest. This was
     happening in BPG::OnWillAttachComplete leading to an invalid cast
     from RWHVCF to RWHVG. This turned out to cause crashes on some
     bots.
  4- It is possible with small changes to unify the attaching code path
     for other GuestViews and MHVG. This CL does just that.

This CL also adds a sanity DCHECK in WCViewGuest to make sure we do not
instantiate an incorrect type of GuestView.

Bug: 659750
Change-Id: Id9c668aa5a98a8f669f56596828cef40198ec57f
Reviewed-on: https://chromium-review.googlesource.com/1225307
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592537}
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/components/guest_view/browser/guest_view_message_filter.cc
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/components/guest_view/browser/guest_view_message_filter.h
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/content/browser/web_contents/web_contents_view_guest.cc
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/content/public/browser/guest_host.h
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/extensions/browser/guest_view/mime_handler_view/mime_handler_view_constants.cc
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/extensions/browser/guest_view/mime_handler_view/mime_handler_view_constants.h
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h
[modify] https://crrev.com/361cc6fa6e2bc663c5492cad0dfea3b2c88330bd/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 5

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

commit db49e76b1348eb7ab7e88bcbbd30b80ff8d01a5f
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Fri Oct 05 20:45:35 2018

Create MimeHandlerViewFrameContainer only once

Currently embedding a PDF creates a MimeHandlerViewFrameContainer which
then loads an extension that attaches another internal plugin element
to load the PDF plugin. This CL makes sure we do not create a second
MimeHandlerViewFrameContainer for that.

Bug: 659750
Change-Id: Iff71eee6d59b824572628f1ba68651d6be16138c
Reviewed-on: https://chromium-review.googlesource.com/c/1265502
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597284}
[modify] https://crrev.com/db49e76b1348eb7ab7e88bcbbd30b80ff8d01a5f/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc
[modify] https://crrev.com/db49e76b1348eb7ab7e88bcbbd30b80ff8d01a5f/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Oct 16

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

commit 18f81dcf84ee6f58e68bc12c7a5da39a1a16a140
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Tue Oct 16 15:12:53 2018

Support for Proxies in Frame-based MimeHandlerView

The API for attaching inner and outer WebContentses only supports a
RenderFrameHost which is same-process and origin with its parent. This
is due to the fact that all GuestViews start off with a blank <iframe>
which is later swapped out with a proxy during attaching process.

For MimeHandlerViewGuest it is not guaranteed that the starting frame
in an <embed> or <object> is in 'about:blank'. This CL circumvents the
problem by first navigating a plugin frame (on the browser side) to
'about:blank' and then proceeding with the attaching process.

 Bug: 659750

Change-Id: I57d41a38e957b22c4ba3c29cb11122115373946f
Reviewed-on: https://chromium-review.googlesource.com/c/1240635
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599986}
[modify] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/chrome/browser/chrome_content_browser_client.cc
[add] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/chrome/test/data/extensions/api_test/mime_handler_view/test_object_with_frame.html
[add] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/chrome/test/data/extensions/api_test/mime_handler_view/test_page.html
[modify] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
[modify] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/extensions/browser/guest_view/extensions_guest_view_message_filter.h
[modify] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc
[modify] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
[modify] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h
[modify] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/extensions/common/guest_view/extensions_guest_view_messages.h
[modify] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.cc
[modify] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.h
[modify] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.cc
[modify] https://crrev.com/18f81dcf84ee6f58e68bc12c7a5da39a1a16a140/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.h

Blocking: 896679
Project Member

Comment 28 by bugdroid1@chromium.org, Oct 23

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

commit 99b7675d49cd772d473338f2efc385e6144bcb09
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Tue Oct 23 17:50:16 2018

Do not reset |element_instance_id_| for GuestViews

When a guest view is being destroyed the |element_instance_id_| is reset
to kInstanceIDNone. This serves as a signal that the guest is being
destroyed (or not attached yet). However, |is_being_destroyed_| and
|attach_in_progress_| already service those purposes and resetting the
|element_instance_id_| seems unnecessary.

For frame-based MimeHandlerViewGuest, |element_instance_id()| is used to
uniquely idenitfy the MimeHandlerViewContainerBase in the embedder
process. For clean-up purposes, its value is needed around the time
MimeHandlerViewGuest is being destroyed (i.e., due to detaching the
plugin element on the embedder side).

This CL will remove the code that resets the value of
|element_instance_id_| during destruction, and makes attach rely on the
signal |is_being_destroyed_| as well.

Bug: 659750
Change-Id: I929d1dc1a255bcb70597baefaa663ed7c00cdfa1
Reviewed-on: https://chromium-review.googlesource.com/c/1294202
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602012}
[modify] https://crrev.com/99b7675d49cd772d473338f2efc385e6144bcb09/components/guest_view/browser/guest_view_base.cc
[modify] https://crrev.com/99b7675d49cd772d473338f2efc385e6144bcb09/components/guest_view/browser/guest_view_base.h

Blockedon: 911161
Project Member

Comment 30 by bugdroid1@chromium.org, Dec 4

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

commit dd9a414abfcc248c94dca646d85741cf7c7d956e
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Tue Dec 04 20:38:20 2018

WebContents should attach after navigations commit

To accommodate attaching MimeHandlerViewGuest though a proxy, the
FrameTreeNode is first navigated to about:blank. The current logic in
FrameNavigationHelper waits for the first commit to about:blank and
then continues the attach process. This current behavior has some
problems which this CL tries to resolve.

1- The helper class listens to WebContentsObserver::DidFinishNavigation
to find out when the navigation in the plugin frame commits; then
immediately attaches to the outer WebContents. This is incorrect as
attaching will synchronously destroy the RenderFrameHost which causes
issues with other observers of the navigations in that frame
(NavigationHandle::GetRenderFrameHost() becomes invalid.

2- There could be many queued up navigations to about:blank and some
of them will commit after attach which causes the same problem as 1).

To address issue 1), this CL will post task the attach to give all the
observers the chance to observe the committed navigation. To address
 issue 2 ) new APIs are introduced to content layer which help to reset
a frame's state (navigation requests and loading state) prior to
attaching.

The CL will also enable a test which was disabled due to crashes caused
by the mentioned problems above.

Bug: 897971, 659750
Change-Id: Ie26a7ca644dcbdb1b18e5a659119ea2f46719c0a
Reviewed-on: https://chromium-review.googlesource.com/c/1298354
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613683}
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/chrome/test/data/extensions/api_test/mime_handler_view/test_page.html
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/public/browser/render_frame_host.h
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/content/public/browser/web_contents.h
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/extensions/browser/guest_view/extensions_guest_view_message_filter.h
[modify] https://crrev.com/dd9a414abfcc248c94dca646d85741cf7c7d956e/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc

Project Member

Comment 31 by bugdroid1@chromium.org, Dec 5

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

commit f461bf8f5e6149abfae57d1abff10ed48b2863ad
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Wed Dec 05 22:25:09 2018

Remove attaching logic from GuestViewMessageFilter

This CL will remove part of the logic to the GuestViewBase itself and
exposes a public method rather than marking GuestViewMessageFilter a
friend class.

A motivation for the change the refactoring need for frame-based
MimeHandlerViewGuest to be separated form
ExtensionsGuestViewMessageFilter.

Bug: 896679, 659750
Change-Id: I16a21117c06dcfd00426dcec6fc4927d3e287342
Reviewed-on: https://chromium-review.googlesource.com/c/1361794
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614132}
[modify] https://crrev.com/f461bf8f5e6149abfae57d1abff10ed48b2863ad/components/guest_view/browser/guest_view_base.cc
[modify] https://crrev.com/f461bf8f5e6149abfae57d1abff10ed48b2863ad/components/guest_view/browser/guest_view_base.h
[modify] https://crrev.com/f461bf8f5e6149abfae57d1abff10ed48b2863ad/components/guest_view/browser/guest_view_message_filter.cc
[modify] https://crrev.com/f461bf8f5e6149abfae57d1abff10ed48b2863ad/components/guest_view/browser/guest_view_message_filter.h
[modify] https://crrev.com/f461bf8f5e6149abfae57d1abff10ed48b2863ad/extensions/browser/guest_view/extensions_guest_view_message_filter.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Dec 14

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

commit 2a462553b178d64333df31f21b1eb4993f101b7a
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Fri Dec 14 21:41:06 2018

GuestView: Replace CHECK with bad_message

If a MimeHandlerViewGuest and a guest WebContents are created without a proper
GuestViewManager in place (for the owner WebContents BrowserContext), we are currently
CHECK-ing the browser. It might be wise to simply destroy the GuestView and crash the
embedder process on a bad message instead.

Bug: 659750
Change-Id: Ie46ede5889ec9ce087fcb4d3fb9d98adb6300314
Reviewed-on: https://chromium-review.googlesource.com/c/1378810
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616835}
[modify] https://crrev.com/2a462553b178d64333df31f21b1eb4993f101b7a/extensions/browser/guest_view/extensions_guest_view_message_filter.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Jan 9

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

commit 5bb223676defeba9c44a5ce42460c86e24561e73
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Wed Jan 09 15:53:27 2019

[GuestView] - Introduce MimeHandlerViewAttachHelper

This CL is for the most part a mechanical change which extracts almost
all the frame-based MimeHandlerView code out of
ExtensionsGuestViewMessageFilter. This change both removes the current
clutter form EGVMF as well as fixesa race introduced when the
frame-based logic was added to EGVMF. The reason for the race was that
EGVMF is destroyed on IO thread but all the access to it (for
frame-based MHV) are from UI.

TBR=avi@chromium.org,lazyboy@chromium.org

Bug: 659750, 896679, 911161, 918861
Change-Id: I6474b870e4d56daa68be03637bb633665d9f9dda
Reviewed-on: https://chromium-review.googlesource.com/c/1401451
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621155}
[modify] https://crrev.com/5bb223676defeba9c44a5ce42460c86e24561e73/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/5bb223676defeba9c44a5ce42460c86e24561e73/extensions/browser/BUILD.gn
[modify] https://crrev.com/5bb223676defeba9c44a5ce42460c86e24561e73/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
[modify] https://crrev.com/5bb223676defeba9c44a5ce42460c86e24561e73/extensions/browser/guest_view/extensions_guest_view_message_filter.h
[add] https://crrev.com/5bb223676defeba9c44a5ce42460c86e24561e73/extensions/browser/guest_view/mime_handler_view/mime_handler_view_attach_helper.cc
[add] https://crrev.com/5bb223676defeba9c44a5ce42460c86e24561e73/extensions/browser/guest_view/mime_handler_view/mime_handler_view_attach_helper.h

Project Member

Comment 34 by bugdroid1@chromium.org, Jan 17 (6 days ago)

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

commit fd340ff75203d06f8676745a54bb310e01e541d1
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Thu Jan 17 16:16:14 2019

[one-liner] - Remove unused variable

This CL removes an unused variable which was mistakenly added in with
the refactoring CL 1401451.

TBR=wjmaclean@chromium.org

Bug: 659750
Change-Id: Ic1045e6fe23f8a6465aa371c6c31c383e282b5cd
Reviewed-on: https://chromium-review.googlesource.com/c/1407651
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623705}
[modify] https://crrev.com/fd340ff75203d06f8676745a54bb310e01e541d1/extensions/browser/guest_view/mime_handler_view/mime_handler_view_attach_helper.h

Sign in to add a comment