PNaCl just-in-time download is broken |
|||||||
Issue descriptionThe PNaCl JIT fetch is broken in the case where no component exists on disk (still works if an incompatible component exists on disk): A call to CheckVersionCompatibility during the registration flow was removed by https://chromium-review.googlesource.com/c/chromium/src/+/627237 .
,
Nov 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c59b0960eb1c82e9a3c18c97dc94bd2d464ca634 commit c59b0960eb1c82e9a3c18c97dc94bd2d464ca634 Author: Joshua Pawlicki <waffles@google.com> Date: Fri Nov 03 00:09:07 2017 By default, claim PNaCl needs a JIT update: CheckVersionCompatibility is not called in the registration flow unless there is a version of PNaCl already on disk. Bug: 781011 Change-Id: I9be05564bd9a40df6dba7a8256cea286bf5aec24 Reviewed-on: https://chromium-review.googlesource.com/752049 Reviewed-by: Sorin Jianu <sorin@chromium.org> Commit-Queue: Joshua Pawlicki <waffles@chromium.org> Cr-Commit-Position: refs/heads/master@{#513627} [modify] https://crrev.com/c59b0960eb1c82e9a3c18c97dc94bd2d464ca634/chrome/browser/component_updater/pnacl_component_installer.cc
,
Nov 3 2017
Premptively merge-requesting to 63 (it's probably unrealistic to get into 62 given how late we are into the cycle).
,
Nov 3 2017
Verified the fix in this morning's canary. There's an M62 respin happening on Monday, maybe we want to catch that, too? +abdulsyed Release owners: The merge is low-risk (but also has limited impact: only new installs of Chrome are affected, and only for the first 6 minutes of the browser uptime). Do you prefer we take it or not? (One-line change that only touches the pnacl JIT install.)
,
Nov 3 2017
What are the implications if we don't take this in M62? What is the full impact? My recommendation is to wait until M63.
,
Nov 3 2017
Users will not be able to use websites that use PNaCl within the first 6 minutes of browser use after they install Chrome. I don't know how to quantify the number of sites that do this. This is certainly not a P0. (Presumably the number of broken sites is not too large because the bug went this long before being discovered.) Example site: https://earth.google.com/web/
,
Nov 3 2017
Since impact is lower, recommendation is to target this for M63.
,
Nov 3 2017
Approving merge to M63 branch 3239 based on comment #4 and #7. Please merge ASAP. Thank you.
,
Nov 3 2017
Merged @ https://chromium-review.googlesource.com/c/chromium/src/+/753804
,
Nov 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/90709468f3e0be750182fe0d5bb94c9a7eea4cd8 commit 90709468f3e0be750182fe0d5bb94c9a7eea4cd8 Author: Joshua Pawlicki <waffles@google.com> Date: Fri Nov 03 17:23:45 2017 By default, claim PNaCl needs a JIT update: CheckVersionCompatibility is not called in the registration flow unless there is a version of PNaCl already on disk. Bug: 781011 Change-Id: I9be05564bd9a40df6dba7a8256cea286bf5aec24 Reviewed-on: https://chromium-review.googlesource.com/752049 Reviewed-by: Sorin Jianu <sorin@chromium.org> Commit-Queue: Joshua Pawlicki <waffles@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#513627}(cherry picked from commit c59b0960eb1c82e9a3c18c97dc94bd2d464ca634) Reviewed-on: https://chromium-review.googlesource.com/753804 Reviewed-by: Joshua Pawlicki <waffles@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#366} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/90709468f3e0be750182fe0d5bb94c9a7eea4cd8/chrome/browser/component_updater/pnacl_component_installer.cc
,
Dec 5 2017
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by waff...@chromium.org
, Nov 2 2017