New issue
Advanced search Search tips
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
link

Issue 659750: Convert MimeHandlerViewGuest to load via OOPIF instead of BrowserPlugin

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

Issue description

This is a tracking bug for the work to make PDFs display inside an oopif-based GuestView.
 

Comment 1 by ekaramad@chromium.org, Oct 26 2016

Cc: mknowles@chromium.org
 Issue 642826  has been merged into this issue.

Comment 2 by ekaramad@chromium.org, Nov 8 2016

 Issue 659772  has been merged into this issue.

Comment 3 by ekaramad@chromium.org, Jun 6 2017

Cc: thestig@chromium.org dsinclair@chromium.org fsam...@chromium.org
 Issue 600214  has been merged into this issue.

Comment 4 by mknowles@chromium.org, Jun 19 2017

Cc: -ekaramad@chromium.org aval...@chromium.org
Owner: ekaramad@chromium.org

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

Blocking: 732393

Comment 6 by ekaramad@chromium.org, Oct 19 2017

Blockedon: 776510

Comment 7 by thestig@chromium.org, Dec 21 2017

Components: Internals>Plugins>PDF

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

Cc: -mcnee@google.com mcnee@chromium.org

Comment 9 by ekaramad@chromium.org, Jan 24 2018

Blockedon: 781580

Comment 10 by ekaramad@chromium.org, Jan 25 2018

Blocking: 510568

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

Blocking: 464269

Comment 12 by sahel@chromium.org, May 4 2018

Blocking: 832547

Comment 13 by bugdroid1@chromium.org, Jul 27 2018

Project Member
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

Comment 14 by bugdroid1@chromium.org, Aug 2

Project Member
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

Comment 15 by bugdroid1@chromium.org, Aug 3

Project Member
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

Comment 16 by bugdroid1@chromium.org, Aug 8

Project Member
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

Comment 17 by bugdroid1@chromium.org, Aug 10

Project Member
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

Comment 18 by bugdroid1@chromium.org, Aug 13

Project Member
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

Comment 19 by bugdroid1@chromium.org, Aug 22

Project Member
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

Comment 20 by bugdroid1@chromium.org, Sep 6

Project Member
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

Comment 21 by bugdroid1@chromium.org, Sep 7

Project Member
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

Comment 22 by bugdroid1@chromium.org, Sep 9

Project Member
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

Comment 23 by bugdroid1@chromium.org, Sep 12

Project Member
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

Comment 24 by bugdroid1@chromium.org, Sep 19

Project Member
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

Comment 25 by bugdroid1@chromium.org, Oct 5

Project Member
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

Comment 26 by bugdroid1@chromium.org, Oct 16

Project Member
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

Comment 27 by ekaramad@chromium.org, Oct 18

Blocking: 896679

Comment 28 by bugdroid1@chromium.org, Oct 23

Project Member
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

Comment 29 by ekaramad@chromium.org, Dec 3

Blockedon: 911161

Comment 30 by bugdroid1@chromium.org, Dec 4

Project Member
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

Comment 31 by bugdroid1@chromium.org, Dec 5

Project Member
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

Comment 32 by bugdroid1@chromium.org, Dec 14

Project Member
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

Comment 33 by bugdroid1@chromium.org, Jan 9

Project Member
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

Comment 34 by bugdroid1@chromium.org, Jan 17

Project Member
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

Comment 35 by bugdroid, Jan 31

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0634e8dd340be0d34b5b28a63fdbe3177d8b8a0c

commit 0634e8dd340be0d34b5b28a63fdbe3177d8b8a0c
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Thu Jan 31 22:59:59 2019

[ MimeHandlerView ] Fix an error in test

The |query_index| was not defined and the test was not actually
navigating the test page to the desired URL.

TBR=wjmaclean@chromium.org

Bug: 659750
Change-Id: I8f9658ac80adaa20f3cdceb0c19f799b59f96987
Reviewed-on: https://chromium-review.googlesource.com/c/1448776
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628125}
[modify] https://crrev.com/0634e8dd340be0d34b5b28a63fdbe3177d8b8a0c/chrome/test/data/extensions/api_test/mime_handler_view/test_page.html

Comment 36 by bugdroid, Feb 1

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

commit f47f58d524e15bb0c84ed940ac37736fd54b0ca0
Author: Keishi Hattori <keishi@chromium.org>
Date: Fri Feb 01 03:36:57 2019

Revert "[ MimeHandlerView ] Fix an error in test"

This reverts commit 0634e8dd340be0d34b5b28a63fdbe3177d8b8a0c.

Reason for revert: MimeHandlerViewCrossProcessTest.NavigationRaceFromCrossProcessRenderer is failing on Mac bots
crbug.com/927564

Original change's description:
> [ MimeHandlerView ] Fix an error in test
> 
> The |query_index| was not defined and the test was not actually
> navigating the test page to the desired URL.
> 
> TBR=wjmaclean@chromium.org
> 
> Bug: 659750
> Change-Id: I8f9658ac80adaa20f3cdceb0c19f799b59f96987
> Reviewed-on: https://chromium-review.googlesource.com/c/1448776
> Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
> Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#628125}

TBR=ekaramad@chromium.org,wjmaclean@chromium.org

Change-Id: Iae21364b81a5a031c96ba6342b5ffa9186e21076
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 659750
Reviewed-on: https://chromium-review.googlesource.com/c/1448087
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628227}
[modify] https://crrev.com/f47f58d524e15bb0c84ed940ac37736fd54b0ca0/chrome/test/data/extensions/api_test/mime_handler_view/test_page.html

Comment 37 by bugdroid, Feb 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/537884654b9e22db33937522fac156f03c87c955

commit 537884654b9e22db33937522fac156f03c87c955
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Fri Feb 08 16:31:49 2019

[ MimeHandlerView ] Clean-up unused declarations

The attaching logic and FrameNavigationHelper are now moved to another class
(MimeHandlerViewAttachHelper). This CL removes the left over code.

TBR=wjmaclean@chromium.org

Bug: 659750
Change-Id: Ib67d21ab8104e957bf41df0d20ddf99a04dac3d3
Reviewed-on: https://chromium-review.googlesource.com/c/1460596
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630344}
[modify] https://crrev.com/537884654b9e22db33937522fac156f03c87c955/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
[modify] https://crrev.com/537884654b9e22db33937522fac156f03c87c955/extensions/browser/guest_view/extensions_guest_view_message_filter.h

Comment 38 by bugdroid, Feb 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94967ad6a9d8ec3befb3ffd27362c11ed99b639f

commit 94967ad6a9d8ec3befb3ffd27362c11ed99b639f
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Thu Feb 14 16:24:15 2019

[ MimeHandlerView ] Runtime Feature for blink

Frame-based MimeHandlerView is currently behind a flag which is defined
in content features. This CL adds a corresponding runtime feature flag
which is drived by the content feature. This is needed in upcoming CLs
for blink changes needed to support frame-based MHV.

The proof of concept CL that benefits from this flag is: issue 1287198

Bug: 659750
Change-Id: Ia127f0cd01b7123ff4a592a7334d90dd417f0d6a
Reviewed-on: https://chromium-review.googlesource.com/c/1459336
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Rick Byers <rbyers@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632234}
[modify] https://crrev.com/94967ad6a9d8ec3befb3ffd27362c11ed99b639f/content/child/runtime_features.cc
[modify] https://crrev.com/94967ad6a9d8ec3befb3ffd27362c11ed99b639f/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/94967ad6a9d8ec3befb3ffd27362c11ed99b639f/third_party/blink/public/platform/web_runtime_features.h
[modify] https://crrev.com/94967ad6a9d8ec3befb3ffd27362c11ed99b639f/third_party/blink/renderer/core/exported/local_frame_client_impl.cc
[modify] https://crrev.com/94967ad6a9d8ec3befb3ffd27362c11ed99b639f/third_party/blink/renderer/platform/exported/web_runtime_features.cc
[modify] https://crrev.com/94967ad6a9d8ec3befb3ffd27362c11ed99b639f/third_party/blink/renderer/platform/runtime_enabled_features.json5

Comment 39 by bugdroid, Feb 19 (3 days ago)

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/44f598b008e49b284bc7d47b906d2c523f3f3150

commit 44f598b008e49b284bc7d47b906d2c523f3f3150
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Tue Feb 19 18:42:00 2019

[ MimeHandlerView ] Navigation to PDF Resource

This CL implements the browser side hooks/logic required to support
navigation of the main frame or a subframe to a PDF resource. With the
non-frame based version the response for the navigation is intercepted,
replaced with a GUID and sent to blink where blink generates a
PluginDocument.

With frame-based MHV, a PluginDocument does not make sense (it expects
to have a PluginView inside). Instead, this CL changes the behavior such
that the response from the browser includes a templated HTML page.

The follow-up CL to this will ensure that in response to mime-types such
as PDF, blink creates a HTMLDocument instead of a PluginDocument, and
further adds the required MimeHandlerViewContainer logic to support
navigations to PDF resource.

Design Document:
https://docs.google.com/document/d/1_gJv4_fewyfjI7lcUgFX14iQxDudtEMrsjjWKpkI5BI/edit

TBR=lazyboy@chromium.org

Bug: 659750
Change-Id: Idf6bcf20450c2b50d360b8fa4033c5b9494981fd
Reviewed-on: https://chromium-review.googlesource.com/c/1460426
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Ɓukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633322}
[modify] https://crrev.com/44f598b008e49b284bc7d47b906d2c523f3f3150/chrome/app/chrome_content_renderer_overlay_manifest.cc
[modify] https://crrev.com/44f598b008e49b284bc7d47b906d2c523f3f3150/chrome/browser/plugins/plugin_response_interceptor_url_loader_throttle.cc
[modify] https://crrev.com/44f598b008e49b284bc7d47b906d2c523f3f3150/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/44f598b008e49b284bc7d47b906d2c523f3f3150/extensions/browser/guest_view/mime_handler_view/mime_handler_view_attach_helper.cc
[modify] https://crrev.com/44f598b008e49b284bc7d47b906d2c523f3f3150/extensions/browser/guest_view/mime_handler_view/mime_handler_view_attach_helper.h
[modify] https://crrev.com/44f598b008e49b284bc7d47b906d2c523f3f3150/extensions/common/mojo/guest_view.mojom
[modify] https://crrev.com/44f598b008e49b284bc7d47b906d2c523f3f3150/extensions/renderer/BUILD.gn
[add] https://crrev.com/44f598b008e49b284bc7d47b906d2c523f3f3150/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_manager.cc
[add] https://crrev.com/44f598b008e49b284bc7d47b906d2c523f3f3150/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_manager.h

Comment 40 by bugdroid, Yesterday (27 hours ago)

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9539fccf96cc4695d50fabd5e1774eabd7805d86

commit 9539fccf96cc4695d50fabd5e1774eabd7805d86
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Fri Feb 22 03:04:08 2019

[ MimeHandlerView ] Do not create PluginDocument

This CL is the second part in the implementation of navigation to PDF
resources with frame-based MimeHandlerView. This change implements the
logic for creating a MimeHandlerViewFrameContainer in response to
browser's request. To this end, when handling the resource request
response for a PDF mime-type, DOMImplementation avoids creating a
PluginDocument and implements an HTMLDocument instead. The document is
then populated with the templated HTML page injected by the plugin
response interceptor.

Design document:
https://docs.google.com/document/d/1_gJv4_fewyfjI7lcUgFX14iQxDudtEMrsjjWKpkI5BI/edit

Bug: 659750
Change-Id: I9a4490d99391d746333f76979e98682fa6742be7
Reviewed-on: https://chromium-review.googlesource.com/c/1477921
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634499}
[modify] https://crrev.com/9539fccf96cc4695d50fabd5e1774eabd7805d86/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.h
[modify] https://crrev.com/9539fccf96cc4695d50fabd5e1774eabd7805d86/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_manager.cc
[modify] https://crrev.com/9539fccf96cc4695d50fabd5e1774eabd7805d86/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_manager.h
[modify] https://crrev.com/9539fccf96cc4695d50fabd5e1774eabd7805d86/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.cc
[modify] https://crrev.com/9539fccf96cc4695d50fabd5e1774eabd7805d86/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.h
[modify] https://crrev.com/9539fccf96cc4695d50fabd5e1774eabd7805d86/third_party/blink/renderer/core/dom/dom_implementation.cc

Sign in to add a comment