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

Issue 819761 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 26
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-04-30
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 705114
issue 890401



Sign in to add a comment

Convert QuickOffice extension to use mimeHandlerPrivate instead of streamsPrivate

Project Member Reported by jam@chromium.org, Mar 7 2018

Issue description

https://codereview.chromium.org/797183005 converted the PDF plugin to use a new API to fix a bug.

streamsPrivate doesn't work with network service, so it seems better better to convert QuickOffice and then we have one fewer private extensions API.

 

Comment 2 by jam@chromium.org, Mar 7 2018

Owner: sdoerner@google.com
Status: Assigned (was: Untriaged)
Assigning to Sebastian as a vendor will look at this. Thank you!
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 9 2018

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

commit 9dc4d3a8c0718eb9f32aea3b5e799c80ac9da234
Author: John Abd-El-Malek <jam@chromium.org>
Date: Fri Mar 09 22:44:37 2018

Whitelist QuickOffice extension to use the mimeHandler private API.

Bug:  819761 
Change-Id: I7d34886e5d12b1a77287463a6d15daf054192858
Reviewed-on: https://chromium-review.googlesource.com/956965
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542262}
[modify] https://crrev.com/9dc4d3a8c0718eb9f32aea3b5e799c80ac9da234/extensions/common/api/_manifest_features.json

Comment 4 by jam@chromium.org, Apr 5 2018

Owner: sa...@chromium.org
Assigning to Sam since he's looking into it. Thanks!

Comment 5 by jam@chromium.org, Apr 9 2018

Labels: Merge-Request-66 OS-Chrome OS-Linux OS-Mac OS-Windows
Requesting merge approval for the small commit in comment 3 to allow M66 to whitelist QuickOffice for the private API.
Project Member

Comment 6 by sheriffbot@chromium.org, Apr 9 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: We are only 7 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-66 Merge-Approved-66
Approving merge for M66. Branch:3359
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 9 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c1065f42a5b8f6c90a7979b9f7899e539dff42f

commit 5c1065f42a5b8f6c90a7979b9f7899e539dff42f
Author: John Abd-El-Malek <jam@chromium.org>
Date: Mon Apr 09 23:46:33 2018

Whitelist QuickOffice extension to use the mimeHandler private API.

Bug:  819761 
Change-Id: I7d34886e5d12b1a77287463a6d15daf054192858
Reviewed-on: https://chromium-review.googlesource.com/956965
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#542262}(cherry picked from commit 9dc4d3a8c0718eb9f32aea3b5e799c80ac9da234)
Reviewed-on: https://chromium-review.googlesource.com/1003613
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#635}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/5c1065f42a5b8f6c90a7979b9f7899e539dff42f/extensions/common/api/_manifest_features.json

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 12 2018

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

commit 4aa62fd0b891413b123835e824c9ed79924d0bde
Author: Sam McNally <sammc@chromium.org>
Date: Thu Apr 12 22:12:11 2018

Support fullscreen for mime handler view guests.

QuickOffice needs to be able to enable fullscreen to support its present
functionality. Implement the fullscreen-related WebContentsDelegate
methods in MimeHandlerViewGuest by forwarding to the embedding
WebContentsDelegate. Also override EmbedderFullscreenToggled to forward
fullscreen exit reporting to the guest.

Bug:  819761 
Tbr: lazyboy@chromium.org
Change-Id: Ia334104375cf8d92a0b45b92cb1b6e80a1e9c407
Reviewed-on: https://chromium-review.googlesource.com/1004394
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550382}
[modify] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/chrome/test/data/extensions/api_test/mime_handler_view/index.js
[modify] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/chrome/test/data/extensions/api_test/mime_handler_view/manifest.json
[add] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/chrome/test/data/extensions/api_test/mime_handler_view/testFullscreen.csv
[add] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/chrome/test/data/extensions/api_test/mime_handler_view/testFullscreen.csv.mock-http-headers
[modify] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/extensions/BUILD.gn
[modify] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
[modify] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h
[add] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/extensions/browser/guest_view/mime_handler_view/mime_handler_view_interactive_uitest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 12 2018

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

commit b0d6b132cd481d35557db5009aa4a1878a29fe1c
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Apr 12 23:14:35 2018

Whitelist more QuickOffice extensions to use the mimeHandler private API.

This matches the permissions used by the old mime_types API that the new API replaces.

Bug:  819761 
Change-Id: I7b9cb7150762d54a375bf1fa69cd10d4b715dfb0
Reviewed-on: https://chromium-review.googlesource.com/1007636
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550406}
[modify] https://crrev.com/b0d6b132cd481d35557db5009aa4a1878a29fe1c/extensions/common/api/_manifest_features.json

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4aa62fd0b891413b123835e824c9ed79924d0bde

commit 4aa62fd0b891413b123835e824c9ed79924d0bde
Author: Sam McNally <sammc@chromium.org>
Date: Thu Apr 12 22:12:11 2018

Support fullscreen for mime handler view guests.

QuickOffice needs to be able to enable fullscreen to support its present
functionality. Implement the fullscreen-related WebContentsDelegate
methods in MimeHandlerViewGuest by forwarding to the embedding
WebContentsDelegate. Also override EmbedderFullscreenToggled to forward
fullscreen exit reporting to the guest.

Bug:  819761 
Tbr: lazyboy@chromium.org
Change-Id: Ia334104375cf8d92a0b45b92cb1b6e80a1e9c407
Reviewed-on: https://chromium-review.googlesource.com/1004394
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550382}
[modify] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/chrome/test/data/extensions/api_test/mime_handler_view/index.js
[modify] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/chrome/test/data/extensions/api_test/mime_handler_view/manifest.json
[add] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/chrome/test/data/extensions/api_test/mime_handler_view/testFullscreen.csv
[add] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/chrome/test/data/extensions/api_test/mime_handler_view/testFullscreen.csv.mock-http-headers
[modify] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/extensions/BUILD.gn
[modify] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
[modify] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h
[add] https://crrev.com/4aa62fd0b891413b123835e824c9ed79924d0bde/extensions/browser/guest_view/mime_handler_view/mime_handler_view_interactive_uitest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 17 2018

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

commit b0d6b132cd481d35557db5009aa4a1878a29fe1c
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Apr 12 23:14:35 2018

Whitelist more QuickOffice extensions to use the mimeHandler private API.

This matches the permissions used by the old mime_types API that the new API replaces.

Bug:  819761 
Change-Id: I7b9cb7150762d54a375bf1fa69cd10d4b715dfb0
Reviewed-on: https://chromium-review.googlesource.com/1007636
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550406}
[modify] https://crrev.com/b0d6b132cd481d35557db5009aa4a1878a29fe1c/extensions/common/api/_manifest_features.json

Project Member

Comment 13 by bugdroid1@chromium.org, Apr 24 2018

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

commit 87f6f2c5ffa313d65d244f0768abaf9acae0c8f0
Author: Sam McNally <sammc@chromium.org>
Date: Tue Apr 24 05:34:18 2018

Stop flashing different background colors loading PDF and QO plugins.

PluginDocument uses a hard-coded background color; for a brief window
between the plugin document loading and the mime handler loading (and
replacing the background with its own content), that background color
is visible, producing a background color flash during plugin loading.

To avoid this, for plugins with fixed background colors (PDF and QO
[QuickOffice]), use their background color for the plugin document.

Bug:  819761 
Change-Id: I38943d900073f7fb150476d9bf159a5e547804d1
Reviewed-on: https://chromium-review.googlesource.com/1023770
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553005}
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/chrome/browser/extensions/plugin_manager.cc
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/chrome/browser/pdf/pdf_extension_test.cc
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/content/public/common/webplugininfo.cc
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/content/public/common/webplugininfo.h
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/content/public/common/webplugininfo.typemap
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/content/public/common/webplugininfo_param_traits.h
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/content/test/test_blink_web_unit_test_support.cc
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/extensions/common/manifest_handlers/mime_types_handler.cc
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/extensions/common/manifest_handlers/mime_types_handler.h
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/third_party/blink/public/platform/web_plugin_list_builder.h
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/third_party/blink/renderer/core/dom/dom_implementation.cc
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/third_party/blink/renderer/core/html/plugin_document.cc
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/third_party/blink/renderer/core/html/plugin_document.h
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/third_party/blink/renderer/platform/plugins/DEPS
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/third_party/blink/renderer/platform/plugins/plugin_data.cc
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/third_party/blink/renderer/platform/plugins/plugin_data.h
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/third_party/blink/renderer/platform/plugins/plugin_list_builder.cc
[modify] https://crrev.com/87f6f2c5ffa313d65d244f0768abaf9acae0c8f0/third_party/blink/renderer/platform/plugins/plugin_list_builder.h

Project Member

Comment 14 by bugdroid1@chromium.org, Apr 26 2018

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

commit 75861848f769d8780dda8caa3b4e9956c2e1f867
Author: Sam McNally <sammc@chromium.org>
Date: Thu Apr 26 03:29:21 2018

Set the view type for mime handler guests.

Mime handler guests currently do not set their view type. Thus, their
foreground pages do not count as foreground pages for the purpose of
deciding whether to keep their background pages alive, resulting in the
background page being shut down while the mime handler remains open.

Set the view type to VIEW_TYPE_EXTENSION_GUEST when creating mime
handler guests to ensure mime handlers keep their background pages alive
while open.

Bug:  819761 
Change-Id: Ibc64c93bbb33022375cf91b45785f543a53ca71c
Reviewed-on: https://chromium-review.googlesource.com/1023518
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553893}
[add] https://crrev.com/75861848f769d8780dda8caa3b4e9956c2e1f867/chrome/test/data/extensions/api_test/mime_handler_view/background.js
[modify] https://crrev.com/75861848f769d8780dda8caa3b4e9956c2e1f867/chrome/test/data/extensions/api_test/mime_handler_view/index.js
[modify] https://crrev.com/75861848f769d8780dda8caa3b4e9956c2e1f867/chrome/test/data/extensions/api_test/mime_handler_view/manifest.json
[add] https://crrev.com/75861848f769d8780dda8caa3b4e9956c2e1f867/chrome/test/data/extensions/api_test/mime_handler_view/testBackgroundPage.csv
[add] https://crrev.com/75861848f769d8780dda8caa3b4e9956c2e1f867/chrome/test/data/extensions/api_test/mime_handler_view/testBackgroundPage.csv.mock-http-headers
[modify] https://crrev.com/75861848f769d8780dda8caa3b4e9956c2e1f867/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc
[modify] https://crrev.com/75861848f769d8780dda8caa3b4e9956c2e1f867/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc

Comment 15 by sa...@chromium.org, Apr 26 2018

Labels: Merge-Request-67
Requesting merge to 67 for the two CLs above.

I've verified the first in canary. It removes a dark grey flash when loading full page PDFs, as well as when loading docs, spreadsheets and slideshows in QuickOffice when running as a mime handler.

The second hasn't made it into a canary yet. It avoids a breakage of QuickOffice when attempting to upgrade files to a newer file format when running as a mime handler.
NextAction: 2018-04-27
Could you pls point exactly which CLs you're requesting a merge for? Also pls update once changes are baked/verified in canary. Thank you.
Project Member

Comment 17 by sheriffbot@chromium.org, Apr 27 2018

Labels: -Merge-Request-67 Merge-Review-67
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 18 by sa...@chromium.org, Apr 27 2018

r553005 and r553893.

r553005 has been verified in canary; r553893 is still waiting for the next canary build.
Thank you sammc@.

Pls update the bug with canary result tomorrow for r553893. 

Comment 20 by sa...@chromium.org, Apr 27 2018

Verified r553893 in canary 68.0.3410.0. I'm not seeing any related crashes, but it hasn't had much soak time so far.
The NextAction date has arrived: 2018-04-27
Labels: -Merge-Review-67 Merge-Approved-67
NextAction: 2018-04-30
Thank you sammc@. 
Approving merge for r553005 and r553893 to M67 branch 3396, pls merge on Mondat morning if canary data continue to look good. 
Project Member

Comment 23 by bugdroid1@chromium.org, Apr 30 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c1e113b600fb04180124b41575aea530f923700

commit 8c1e113b600fb04180124b41575aea530f923700
Author: Sam McNally <sammc@chromium.org>
Date: Mon Apr 30 01:08:59 2018

Set the view type for mime handler guests.

Mime handler guests currently do not set their view type. Thus, their
foreground pages do not count as foreground pages for the purpose of
deciding whether to keep their background pages alive, resulting in the
background page being shut down while the mime handler remains open.

Set the view type to VIEW_TYPE_EXTENSION_GUEST when creating mime
handler guests to ensure mime handlers keep their background pages alive
while open.

Bug:  819761 
Change-Id: Ibc64c93bbb33022375cf91b45785f543a53ca71c
Reviewed-on: https://chromium-review.googlesource.com/1023518
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#553893}(cherry picked from commit 75861848f769d8780dda8caa3b4e9956c2e1f867)
Reviewed-on: https://chromium-review.googlesource.com/1033594
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#370}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[add] https://crrev.com/8c1e113b600fb04180124b41575aea530f923700/chrome/test/data/extensions/api_test/mime_handler_view/background.js
[modify] https://crrev.com/8c1e113b600fb04180124b41575aea530f923700/chrome/test/data/extensions/api_test/mime_handler_view/index.js
[modify] https://crrev.com/8c1e113b600fb04180124b41575aea530f923700/chrome/test/data/extensions/api_test/mime_handler_view/manifest.json
[add] https://crrev.com/8c1e113b600fb04180124b41575aea530f923700/chrome/test/data/extensions/api_test/mime_handler_view/testBackgroundPage.csv
[add] https://crrev.com/8c1e113b600fb04180124b41575aea530f923700/chrome/test/data/extensions/api_test/mime_handler_view/testBackgroundPage.csv.mock-http-headers
[modify] https://crrev.com/8c1e113b600fb04180124b41575aea530f923700/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc
[modify] https://crrev.com/8c1e113b600fb04180124b41575aea530f923700/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Apr 30 2018

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

commit e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09
Author: Sam McNally <sammc@chromium.org>
Date: Mon Apr 30 01:15:29 2018

Stop flashing different background colors loading PDF and QO plugins.

PluginDocument uses a hard-coded background color; for a brief window
between the plugin document loading and the mime handler loading (and
replacing the background with its own content), that background color
is visible, producing a background color flash during plugin loading.

To avoid this, for plugins with fixed background colors (PDF and QO
[QuickOffice]), use their background color for the plugin document.

TBR=sammc@chromium.org

(cherry picked from commit 87f6f2c5ffa313d65d244f0768abaf9acae0c8f0)

Bug:  819761 
Change-Id: I38943d900073f7fb150476d9bf159a5e547804d1
Reviewed-on: https://chromium-review.googlesource.com/1023770
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#553005}
Reviewed-on: https://chromium-review.googlesource.com/1034417
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#371}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/chrome/browser/extensions/plugin_manager.cc
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/chrome/browser/pdf/pdf_extension_test.cc
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/content/public/common/webplugininfo.cc
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/content/public/common/webplugininfo.h
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/content/public/common/webplugininfo.typemap
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/content/public/common/webplugininfo_param_traits.h
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/content/test/test_blink_web_unit_test_support.cc
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/extensions/common/manifest_handlers/mime_types_handler.cc
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/extensions/common/manifest_handlers/mime_types_handler.h
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/third_party/blink/public/platform/web_plugin_list_builder.h
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/third_party/blink/renderer/core/dom/dom_implementation.cc
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/third_party/blink/renderer/core/html/plugin_document.cc
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/third_party/blink/renderer/core/html/plugin_document.h
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/third_party/blink/renderer/platform/plugins/plugin_data.cc
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/third_party/blink/renderer/platform/plugins/plugin_data.h
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/third_party/blink/renderer/platform/plugins/plugin_list_builder.cc
[modify] https://crrev.com/e57f5ad834cc004e0b8ff12ce3efe6286a1dbb09/third_party/blink/renderer/platform/plugins/plugin_list_builder.h

The NextAction date has arrived: 2018-04-30

Comment 26 by jam@chromium.org, Apr 30 2018

Thanks a ton Sam for all these fixes

Comment 27 by jam@chromium.org, May 8 2018

Note for those following: the changes have all landed and merged when necessary to 67. The QuickOffice team will push a new version that uses these APIs in the third week of June.

Comment 28 by dxie@chromium.org, May 15 2018

Labels: -Pri-2 Proj-Servicification-Canary Pri-1
Owner: jam@chromium.org

Comment 29 by mek@chromium.org, Jun 27 2018

We're in the fourth week of June now, did things go as planned/what are the next steps?
Project Member

Comment 30 by bugdroid1@chromium.org, Jun 27 2018

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

commit 69a3377d60d1418a44a599b87210881d1f5fc59b
Author: Sam McNally <sammc@chromium.org>
Date: Wed Jun 27 16:54:12 2018

Make mime handlers inherit the session storage of their embedders.

Mime handlers create a nested WebContents to contain the mime handler
extension. Currently, these run with a separate session storage
namespace, resulting in session storage being cleared if a tab recreates
a mime handler, such as due to a reload. Change mime handler WebContents
creation to inherit the session storage namespaces of their embedders so
the session storage behaves similarly to iframes within a single tab.

Bug:  819761 
Change-Id: I3b4f653836190ed86572eb12cdbeef3fbc157683
Reviewed-on: https://chromium-review.googlesource.com/1116633
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570814}
[modify] https://crrev.com/69a3377d60d1418a44a599b87210881d1f5fc59b/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/69a3377d60d1418a44a599b87210881d1f5fc59b/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc

Comment 31 by sa...@chromium.org, Jun 28 2018

Labels: Merge-Request-68
Requesting merge to 68 for r570814. Manually verified with Canary 69.0.3475.0.
Project Member

Comment 32 by sheriffbot@chromium.org, Jun 28 2018

Labels: -Merge-Request-68 Merge-Review-68
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Since we only have 2 more beta cycles remaining, can you please comment on why this is critical for M68? Can we wait until 69? How safe is this merge?

Comment 34 by sa...@chromium.org, Jun 29 2018

Network service is blocked on migration of Quickoffice from streams to mime handler, which is blocked on their tests passing, which needs this fix. QuickOffice tests run against Chrome stable and beta channels. jam@ probably has more context on the urgency.

The change itself is small and should be safe.
Labels: -Merge-Review-68 Merge-Approved-68
Approving merge to M68. Branch:3440
Project Member

Comment 36 by bugdroid1@chromium.org, Jul 4

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a64375000e1ef261e59856d1d9615a303aeff81

commit 8a64375000e1ef261e59856d1d9615a303aeff81
Author: Sam McNally <sammc@chromium.org>
Date: Wed Jul 04 01:30:48 2018

Make mime handlers inherit the session storage of their embedders.

Mime handlers create a nested WebContents to contain the mime handler
extension. Currently, these run with a separate session storage
namespace, resulting in session storage being cleared if a tab recreates
a mime handler, such as due to a reload. Change mime handler WebContents
creation to inherit the session storage namespaces of their embedders so
the session storage behaves similarly to iframes within a single tab.

TBR=sammc@chromium.org

(cherry picked from commit 69a3377d60d1418a44a599b87210881d1f5fc59b)

Bug:  819761 
Change-Id: I3b4f653836190ed86572eb12cdbeef3fbc157683
Reviewed-on: https://chromium-review.googlesource.com/1116633
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#570814}
Reviewed-on: https://chromium-review.googlesource.com/1124736
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#597}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/8a64375000e1ef261e59856d1d9615a303aeff81/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/8a64375000e1ef261e59856d1d9615a303aeff81/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc

Project Member

Comment 37 by bugdroid1@chromium.org, Jul 9

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

commit 745a1200cc9eb479480882fd93da93d22b8105dd
Author: Sam McNally <sammc@chromium.org>
Date: Mon Jul 09 02:22:41 2018

Fix mime handler target="_blank" navigations.

Navigations triggered by regular clicks on an anchor element with
target="_blank" follow a different codepath to both window.open() and
setting location.href - one not handled by MimeHandlerViewGuest.

Override WebContentsDelegate::ShouldCreateWebContents() in
MimeHandlerViewGuest, forwarding to the embedder's
WebContentsDelegate::OpenURLFromTab(), so this case is handled by
creating a new tab outside the mime handler.

Bug:  819761 
Change-Id: If24299b8569fcd2c867ba6f3f32e345248e4c957
Reviewed-on: https://chromium-review.googlesource.com/1125647
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573202}
[modify] https://crrev.com/745a1200cc9eb479480882fd93da93d22b8105dd/chrome/test/data/extensions/api_test/mime_handler_view/index.js
[add] https://crrev.com/745a1200cc9eb479480882fd93da93d22b8105dd/chrome/test/data/extensions/api_test/mime_handler_view/testTargetBlankAnchor.csv
[add] https://crrev.com/745a1200cc9eb479480882fd93da93d22b8105dd/chrome/test/data/extensions/api_test/mime_handler_view/testTargetBlankAnchor.csv.mock-http-headers
[modify] https://crrev.com/745a1200cc9eb479480882fd93da93d22b8105dd/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc
[modify] https://crrev.com/745a1200cc9eb479480882fd93da93d22b8105dd/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
[modify] https://crrev.com/745a1200cc9eb479480882fd93da93d22b8105dd/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h

Project Member

Comment 38 by bugdroid1@chromium.org, Jul 19

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

commit ad57375ebe0d558fbe2add61303960eeb813931f
Author: Sam McNally <sammc@chromium.org>
Date: Thu Jul 19 00:41:08 2018

Add support for beforeunload for mime handler guests.

QuickOffice uses beforeunload events. When running inside a mime
handler, these beforeunload event handlers are ignored. Add a
beforeunload event handler to PluginDocument when a plugin requires it.

This event handler is controlled over a mojo interface, exposed to the
mime handler as chrome.mimeHandlerPrivate.setShowBeforeUnloadDialog().

Bug:  819761 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ieb61cf98ce2bf719fc128499f8d0e57d8fe7ad33
Reviewed-on: https://chromium-review.googlesource.com/1139946
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576283}
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/chrome/test/data/extensions/api_test/mime_handler_view/index.js
[add] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/chrome/test/data/extensions/api_test/mime_handler_view/testBeforeUnloadNoDialog.csv
[add] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/chrome/test/data/extensions/api_test/mime_handler_view/testBeforeUnloadNoDialog.csv.mock-http-headers
[add] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/chrome/test/data/extensions/api_test/mime_handler_view/testBeforeUnloadShowDialog.csv
[add] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/chrome/test/data/extensions/api_test/mime_handler_view/testBeforeUnloadShowDialog.csv.mock-http-headers
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/BUILD.gn
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/browser/DEPS
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/browser/guest_view/extensions_guest_view_message_filter.h
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/common/BUILD.gn
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/common/api/OWNERS
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/common/api/mime_handler.mojom
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/common/api/mime_handler_private.idl
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/common/guest_view/extensions_guest_view_messages.h
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/common/mojo/guest_view.mojom
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.h
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/extensions/renderer/resources/mime_handler_private_custom_bindings.js
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/third_party/blink/public/web/web_plugin_document.h
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/third_party/blink/renderer/core/exported/web_plugin_document.cc
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/third_party/blink/renderer/core/html/plugin_document.cc
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/third_party/blink/renderer/core/html/plugin_document.h
[modify] https://crrev.com/ad57375ebe0d558fbe2add61303960eeb813931f/third_party/closure_compiler/externs/mime_handler_private.js

Project Member

Comment 39 by bugdroid1@chromium.org, Jul 19

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

commit 7b3e2d9493ce36a2f1e627b4e51bc44e22aee499
Author: Sam McNally <sammc@chromium.org>
Date: Thu Jul 19 23:48:51 2018

Inherit the user-gesture state of the mime handler for opening new tabs.

The popup blocker denies OpenURLFromTab() calls without a user gesture.
The calls made from MimeHandlerViewGuest in response to
ShouldCreateWebContents() calls (resulting from clicks on anchors with
target="_blank") defaulted to not having a user gesture, causing them to
always be blocked.

Assume user gesture for mime handlers to avoid
this problem. Mime handlers can only be a whitelisted set of extensions,
so this difference in enforcement is unlikely to result in many
undesired popups.

Bug:  819761 ,863356
Tbr: ekaramad@chromium.org
Change-Id: Ia0c36a6e7c7b3dfd74cd9e0661cc69866bb962d5
Reviewed-on: https://chromium-review.googlesource.com/1135878
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@google.com>
Cr-Commit-Position: refs/heads/master@{#576699}
[modify] https://crrev.com/7b3e2d9493ce36a2f1e627b4e51bc44e22aee499/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc
[modify] https://crrev.com/7b3e2d9493ce36a2f1e627b4e51bc44e22aee499/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc

Project Member

Comment 40 by bugdroid1@chromium.org, Aug 14

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

commit e6b8c52b00a861471a42b82ec3e038329a2e2a07
Author: Sam McNally <sammc@chromium.org>
Date: Tue Aug 14 17:49:45 2018

Re-enable navigate_on_drag_drop for mime handler guests.

BrowserPluginGuest disables navigate_on_drag_drop for all browser plugin
guests. For mime handlers navigating on default drag drops is desirable.
After the mime handler is attached, update the web preferences to
re-enable navigate_on_drag_drop so unhandled drag drops onto mime
handlers result in navigations similar to other web content.

Bug:  819761 
Change-Id: I5b0165be1d8c52538b189194fc9cbca9c86d96a7
Reviewed-on: https://chromium-review.googlesource.com/1173944
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582966}
[modify] https://crrev.com/e6b8c52b00a861471a42b82ec3e038329a2e2a07/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc

Labels: Merge-Request-69
Requesting merge to 69 for r582966. I've manually verified in 70.0.3523.0 Canary.
Before we approve merge to M69, please answer followings:

* Is this M69 regression?  (this is old bug reported back on March 7th, so doesn't seem to be M69 regressio)
* Is it critical to merge to M69?
* Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M69?
* Any other important details to justify the merge.

Please note M69 going to Stable soon, so merge bar is very high. Thank you.

(I'll answer since it's nighttime in Sydney)

This isn't a regression, but is necessary for QuickOffice team to push their new app that stops using the old streams extensions API (they're the only user) and uses the same extensions API as the PDF plugin. This is necessary because the network service, which is about to enter canary, only works with the new API. Until the QuickOffice team publishes their extension (which can't work with both APIs at the same time), users in the finch trial for network service won't be able to use the QuickOffice extension.

This is a very small and low risk change.
Labels: -Merge-Request-69 Merge-Approved-69
Approving merge for r582966 to M69 branch 3497 based on comment #43. 
Project Member

Comment 45 by bugdroid1@chromium.org, Aug 15

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/78dc1aa3060fd889aa73c34fc3ed1872ceb6ab82

commit 78dc1aa3060fd889aa73c34fc3ed1872ceb6ab82
Author: Sam McNally <sammc@chromium.org>
Date: Wed Aug 15 17:23:13 2018

Re-enable navigate_on_drag_drop for mime handler guests.

BrowserPluginGuest disables navigate_on_drag_drop for all browser plugin
guests. For mime handlers navigating on default drag drops is desirable.
After the mime handler is attached, update the web preferences to
re-enable navigate_on_drag_drop so unhandled drag drops onto mime
handlers result in navigations similar to other web content.

Bug:  819761 
Change-Id: I5b0165be1d8c52538b189194fc9cbca9c86d96a7
Reviewed-on: https://chromium-review.googlesource.com/1173944
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#582966}(cherry picked from commit e6b8c52b00a861471a42b82ec3e038329a2e2a07)
Reviewed-on: https://chromium-review.googlesource.com/1174776
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#643}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/78dc1aa3060fd889aa73c34fc3ed1872ceb6ab82/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc

Labels: -Proj-Servicification-Canary Proj-Servicification
Thanks to all of Sam's work, the QuickOffice extension now works with no regressions with the new API. I'll leave this open until the extension is pushed to users (which depends on 69 going to stable). We can also use this to track deleting the old API.

In the meantime, removing the canary blocking label for network service. This is mostly for enterprise, who don't generally use canary, and so it won't block our rollout.
Woo! Isn't 705114 already tracking removing the old API btw?
@mek: yes you're right, I forgot. So I'll just leave this open until the QuickOffice extension update is pushed.
Labels: Hotlist-KnownIssue
Status: Fixed (was: Assigned)
New extension is pushed, and I verified it works with network service enabled.
Blocking: 890401

Sign in to add a comment