Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2295563002:
[HBD] Fix Widevine breakage introduced by prior patch (Closed)
Description[HBD] Fix Widevine breakage introduced by prior patch
Breakage introduced by prior patch here:
https://codereview.chromium.org/2208463002
We had split off GetPluginContentSetting into a utils file, but
forgot to add the widevine header file include.
This means that WIDEVINE_CDM_AVAILABLE is never defined, and the
Widevine-treated-as-JavaScript for content settings clause is
never activated, breaking Widevine on Corp machines.
BUG=641706
Committed: https://crrev.com/ff96ea580802bcc6910c930d636a3d6edb82de9a
Cr-Commit-Position: refs/heads/master@{#415339}
Patch Set 1 #
Total comments: 2
Patch Set 2 : fix #Messages
Total messages: 26 (16 generated)
tommycli@chromium.org changed reviewers: + jrummell@chromium.org
jrummel: Not sure, but maybe this fixes the widevine?
This fixes the issue reported, so lgtm w/nit to get it to compile on official builds. https://codereview.chromium.org/2295563002/diff/1/chrome/browser/plugins/plug... File chrome/browser/plugins/plugin_filter_utils.cc (right): https://codereview.chromium.org/2295563002/diff/1/chrome/browser/plugins/plug... chrome/browser/plugins/plugin_filter_utils.cc:36: DCHECK_EQ(WebPluginInfo::PLUGIN_TYPE_PEPPER_OUT_OF_PROCESS, plugin.type); This needs to be content::WebPluginInfo, with reformatting.
Description was changed from ========== [HBD] Fix Widevine breakage introduced by prior patch BUG=641706 ========== to ========== [HBD] Fix Widevine breakage introduced by prior patch BUG=641706 ==========
tommycli@chromium.org changed reviewers: + bauerb@chromium.org
tommycli@chromium.org changed reviewers: + thestig@chromium.org
thestig: PTAL (bauerb OOO) https://codereview.chromium.org/2295563002/diff/1/chrome/browser/plugins/plug... File chrome/browser/plugins/plugin_filter_utils.cc (right): https://codereview.chromium.org/2295563002/diff/1/chrome/browser/plugins/plug... chrome/browser/plugins/plugin_filter_utils.cc:36: DCHECK_EQ(WebPluginInfo::PLUGIN_TYPE_PEPPER_OUT_OF_PROCESS, plugin.type); On 2016/08/30 00:23:11, jrummell wrote: > This needs to be content::WebPluginInfo, with reformatting. Done.
The CQ bit was checked by tommycli@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
What "prior patch" ? Neither the CL nor the bug makes it obvious.
Description was changed from ========== [HBD] Fix Widevine breakage introduced by prior patch BUG=641706 ========== to ========== [HBD] Fix Widevine breakage introduced by prior patch Breakage introduced by prior patch here: https://chromium.googlesource.com/chromium/src/+/c56e1750274cbd7efb5cba947421... BUG=641706 ==========
On 2016/08/30 17:15:44, Lei Zhang wrote: > What "prior patch" ? Neither the CL nor the bug makes it obvious. Hi, thanks. I added the breaking prior patch to the CL desc. and the bug.
lgtm if you can provide a wee bit more context.
Description was changed from ========== [HBD] Fix Widevine breakage introduced by prior patch Breakage introduced by prior patch here: https://chromium.googlesource.com/chromium/src/+/c56e1750274cbd7efb5cba947421... BUG=641706 ========== to ========== [HBD] Fix Widevine breakage introduced by prior patch Breakage introduced by prior patch here: https://chromium.googlesource.com/chromium/src/+/c56e1750274cbd7efb5cba947421... We had split off this function into a utils file, but forgot to add the widevine header file include. This means that WIDEVINE_CDM_AVAILABLE is never defined, and the Widevine-treated-as-JavaScript for content settings clause is never activated, breaking Widevine on Corp machines. BUG=641706 ==========
Description was changed from ========== [HBD] Fix Widevine breakage introduced by prior patch Breakage introduced by prior patch here: https://chromium.googlesource.com/chromium/src/+/c56e1750274cbd7efb5cba947421... We had split off this function into a utils file, but forgot to add the widevine header file include. This means that WIDEVINE_CDM_AVAILABLE is never defined, and the Widevine-treated-as-JavaScript for content settings clause is never activated, breaking Widevine on Corp machines. BUG=641706 ========== to ========== [HBD] Fix Widevine breakage introduced by prior patch Breakage introduced by prior patch here: https://chromium.googlesource.com/chromium/src/+/c56e1750274cbd7efb5cba947421... We had split off GetPluginContentSetting into a utils file, but forgot to add the widevine header file include. This means that WIDEVINE_CDM_AVAILABLE is never defined, and the Widevine-treated-as-JavaScript for content settings clause is never activated, breaking Widevine on Corp machines. BUG=641706 ==========
On 2016/08/30 17:17:50, Lei Zhang wrote: > lgtm if you can provide a wee bit more context. I added a bit more context into the CL desc.
Description was changed from ========== [HBD] Fix Widevine breakage introduced by prior patch Breakage introduced by prior patch here: https://chromium.googlesource.com/chromium/src/+/c56e1750274cbd7efb5cba947421... We had split off GetPluginContentSetting into a utils file, but forgot to add the widevine header file include. This means that WIDEVINE_CDM_AVAILABLE is never defined, and the Widevine-treated-as-JavaScript for content settings clause is never activated, breaking Widevine on Corp machines. BUG=641706 ========== to ========== [HBD] Fix Widevine breakage introduced by prior patch Breakage introduced by prior patch here: https://codereview.chromium.org/2208463002 We had split off GetPluginContentSetting into a utils file, but forgot to add the widevine header file include. This means that WIDEVINE_CDM_AVAILABLE is never defined, and the Widevine-treated-as-JavaScript for content settings clause is never activated, breaking Widevine on Corp machines. BUG=641706 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tommycli@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jrummell@chromium.org Link to the patchset: https://codereview.chromium.org/2295563002/#ps20001 (title: "fix")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [HBD] Fix Widevine breakage introduced by prior patch Breakage introduced by prior patch here: https://codereview.chromium.org/2208463002 We had split off GetPluginContentSetting into a utils file, but forgot to add the widevine header file include. This means that WIDEVINE_CDM_AVAILABLE is never defined, and the Widevine-treated-as-JavaScript for content settings clause is never activated, breaking Widevine on Corp machines. BUG=641706 ========== to ========== [HBD] Fix Widevine breakage introduced by prior patch Breakage introduced by prior patch here: https://codereview.chromium.org/2208463002 We had split off GetPluginContentSetting into a utils file, but forgot to add the widevine header file include. This means that WIDEVINE_CDM_AVAILABLE is never defined, and the Widevine-treated-as-JavaScript for content settings clause is never activated, breaking Widevine on Corp machines. BUG=641706 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [HBD] Fix Widevine breakage introduced by prior patch Breakage introduced by prior patch here: https://codereview.chromium.org/2208463002 We had split off GetPluginContentSetting into a utils file, but forgot to add the widevine header file include. This means that WIDEVINE_CDM_AVAILABLE is never defined, and the Widevine-treated-as-JavaScript for content settings clause is never activated, breaking Widevine on Corp machines. BUG=641706 ========== to ========== [HBD] Fix Widevine breakage introduced by prior patch Breakage introduced by prior patch here: https://codereview.chromium.org/2208463002 We had split off GetPluginContentSetting into a utils file, but forgot to add the widevine header file include. This means that WIDEVINE_CDM_AVAILABLE is never defined, and the Widevine-treated-as-JavaScript for content settings clause is never activated, breaking Widevine on Corp machines. BUG=641706 Committed: https://crrev.com/ff96ea580802bcc6910c930d636a3d6edb82de9a Cr-Commit-Position: refs/heads/master@{#415339} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/ff96ea580802bcc6910c930d636a3d6edb82de9a Cr-Commit-Position: refs/heads/master@{#415339} |
