New issue
Advanced search Search tips

Issue 882795 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Sep 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Picture-in-Picture bindings are not working in Chrome Extension

Project Member Reported by fbeaufort@chromium.org, Sep 11

Issue description

Chrome Version       : 69.0.3497.81
OS Version: OS X 10.13.6

What steps will reproduce the problem?
1. Enable chrome://flags/#enable-surfaces-for-videos flag
2. Install https://chrome.google.com/webstore/detail/picture-in-picture/hkgfoiooedgoejojocmhlaklaeopbecg Picture-in-Picture chrome extension
3. Go to youtube and watch video
4. Click extension browser action icon to trigger Picture-in-Picture
5. Click extension browser action icon to exit Picture-in-Picture

What is the expected result?
Picture-in-Picture window is closed

What happens instead of that?
Nothing. DevTools shows this error:

VM1275:6 Uncaught (in promise) TypeError: document.exitPictureInPicture is not a function
    at <anonymous>:6:26
    at <anonymous>:14:9

The interesting part is that document.exitPictureInPicture is defined in Top Frame, but not in Picture-in-Picture chrome extension. See screenshots attached.
 
Screen Shot 2018-09-11 at 11.01.29 AM.png
211 KB View Download
Screen Shot 2018-09-11 at 11.02.26 AM.png
192 KB View Download
Components: Blink>Bindings
Owner: peria@chromium.org
Status: Assigned (was: Unconfirmed)
Labels: -Pri-3 -OS-Mac Pri-2
Not tested yet, but I guess we have to update the installer of V8Document as we do it for V8Window.
https://cs.chromium.org/chromium/src/third_party/blink/renderer/bindings/core/v8/v8_context_snapshot.cc?dr=CSs&g=0&l=254-258

SnapshotInterface& snapshot_document = g_snapshot_interfaces[4];
DCHECK(V8Document::wrapperTypeInfo.Equals(snapshot_document.wrapper_type_info));
// Update the install function for V8Window to work for partial interfaces.
snapshot_document.install_function =
      V8Document::install_runtime_enabled_features_on_template_function_;


After introducing Picture-in-Picture, we newly have V8DocumentPartial class,
but we did not use its InstallRuntimeEnabledFeaturesOnTemplate(),
and this issue hits the case.
Cc: mlamouri@chromium.org
Labels: OS-Linux OS-Mac OS-Windows
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 12

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

commit d14e46fd2d04affa773b2048a2a93ae8d6116a11
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Wed Sep 12 03:21:00 2018

bindings: Update V8Document::InstallRuntimeOnTemplate

After introducing Picture-in-Picture, we newly have
V8DocumentPartial in modules component.
Hence we have to update V8Document template when we
ensure templates at launching V8 isolates to enable
process-wide conditioal features (e.g. runtime enabled)

This CL works for it.

Bug:  882795 ,  617892 
Change-Id: Ia10d2157685a77a743dbeb500b040271514abefc
Reviewed-on: https://chromium-review.googlesource.com/1220868
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590585}
[modify] https://crrev.com/d14e46fd2d04affa773b2048a2a93ae8d6116a11/third_party/blink/renderer/bindings/core/v8/v8_context_snapshot.cc

Cc: fbeaufort@chromium.org
Status: Fixed (was: Assigned)
I believe CL in #5 fixed this issue.
Could you check this?
Status: Verified (was: Fixed)
Verified in 71.0.3551.0. Thank you!

Please, can you ask for merge in M70 branch?
Labels: Merge-Request-70
Yes.
We'd like to request a merge of CL in #6 to M70.
Labels: -Merge-Request-70 Merge-Approved-70
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 13

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/640d93372162aad4fe4860187c1e693660ef1fb7

commit 640d93372162aad4fe4860187c1e693660ef1fb7
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Thu Sep 13 02:27:10 2018

bindings: Update V8Document::InstallRuntimeOnTemplate

After introducing Picture-in-Picture, we newly have
V8DocumentPartial in modules component.
Hence we have to update V8Document template when we
ensure templates at launching V8 isolates to enable
process-wide conditioal features (e.g. runtime enabled)

This CL works for it.

Bug:  882795 ,  617892 
Change-Id: Ia10d2157685a77a743dbeb500b040271514abefc
Reviewed-on: https://chromium-review.googlesource.com/1220868
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#590585}(cherry picked from commit d14e46fd2d04affa773b2048a2a93ae8d6116a11)
Reviewed-on: https://chromium-review.googlesource.com/1223266
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#357}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/640d93372162aad4fe4860187c1e693660ef1fb7/third_party/blink/renderer/bindings/core/v8/v8_context_snapshot.cc

Labels: TE-Verified-M70 TE-Verified-70.0.3538.22
Able to reproduce the issue on Mac 10.13.3 using chrome reported version #69.0.3497.81

Verified the fix on Mac 10.13.3, Win-10 and Ubuntu 14.04 using Chrome version #70.0.3538.22 as per the comment #0.
Attaching screen cast for reference.
Observed that Picture-in-Picture window is closed.
Hence, the fix is working as expected. 
Adding the verified labels.

Thanks...!!
882795.mp4
2.8 MB View Download

Sign in to add a comment