New issue
Advanced search Search tips

Issue 873672 link

Starred by 1 user
Status: Fixed
Owner:
wjmaclean@chromium.org
Closed: Aug 15
Cc:
lfg@chromium.org
wjmaclean@chromium.org
mcnee@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

DCHECK in AppWindow::CheckMediaAccessPermission for flash loaded inside <webview>

Project Member Reported by ekaramad@chromium.org, Aug 13 
Chrome Version: 70.0.3521.0
OS: Desktop

What steps will reproduce the problem?
(1) Build Chromium for ToT and make sure DCHECK is enabled.
(2) Run chrome with --pappi-flash-version=VERSION and --ppapi-flash-path=PATH (the values can be obtained from chrome://flash).
(3) Install and run chrome multi-tab browser:
https://chrome.google.com/webstore/detail/multi-tabbed-browser/nfcmophndjlljioblddmepjbcfnocnak
Alternatively, any page with <webview> which also makes use of permissions API works.
(4) Navigate the browser app to: https://www.permadi.com/tutorial/flash9FullScreen/index.html
(5) Grant permission to load plugin when the popup appears.
(6) DCHECK fires.

I believe the assumption of DCHECK is incorrect as the <webview> inside is a separate WebContents than that of the AppWindow itself. Note that <webview> uses its embedder contents' delegate to handle permissions:
https://cs.chromium.org/chromium/src/extensions/browser/guest_view/web_view/web_view_permission_helper.cc?rcl=b73143abfac967ceddc0fc631212d72e48ceb881&l=242
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 15 

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

commit c2752c91a571d223804bba508aabc6b153101fd2
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Wed Aug 15 13:33:18 2018

Revise DCHECK to consider app-guest WeContents difference.

This CL revises a DCHECK that has been firing (erroneously) when Flash
content is loaded inside a WebView.

Bug:  873672 
Change-Id: I75c12c67b47a5b337054a262991d23247bbbb4ea
Reviewed-on: https://chromium-review.googlesource.com/1173359
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583232}
[modify] https://crrev.com/c2752c91a571d223804bba508aabc6b153101fd2/extensions/browser/app_window/app_window.cc

Comment 2 by wjmaclean@chromium.org, Aug 15

Owner: wjmaclean@chromium.org
Status: Fixed (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 16 

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

commit 3f9fba2310272a2ab50b90a61c7985599eaad6e0
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Thu Aug 16 15:20:36 2018

Use GetOutermostWebContents() when checking media permissions.

Instead of using GetOuterWebContents, which only return the immediate
parent of this WebContents, use GetOutermostWebContents to check against
the root of the web-contents tree, which in this case should be the
app-view's web-contents.

Bug:  873672 ,  823021 
Change-Id: I421a64be495c575d22501328ec732794abc045f4
Reviewed-on: https://chromium-review.googlesource.com/1176342
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583656}
[modify] https://crrev.com/3f9fba2310272a2ab50b90a61c7985599eaad6e0/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/3f9fba2310272a2ab50b90a61c7985599eaad6e0/content/public/browser/web_contents.h
[modify] https://crrev.com/3f9fba2310272a2ab50b90a61c7985599eaad6e0/extensions/browser/app_window/app_window.cc
►

Sign in to add a comment