Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1996863002:
media: Use bundled Widevine CDM in encrypted media browser tests (Closed)
|
Created:
4 years, 7 months ago by xhwang Modified:
4 years, 7 months ago CC:
chromium-reviews, markusheintz_, msramek+watch_chromium.org, raymes+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionmedia: Use bundled Widevine CDM in encrypted media browser tests
- Provide option to enable component update in browser tests
- Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM.
- Remove code that manually registers Widevine CDM.
- Enable more tests that wasn't possible when CDM is manually registered.
BUG=613442, 582622, 613581
Committed: https://crrev.com/3be44be552846983cf371256718ce8ec81c565b4
Cr-Commit-Position: refs/heads/master@{#395922}
Patch Set 1 #
Total comments: 1
Patch Set 2 : #
Total comments: 1
Patch Set 3 : #
Total comments: 10
Patch Set 4 : add if-def #
Total comments: 3
Patch Set 5 : new approach #
Total comments: 2
Patch Set 6 : comments addressed #
Messages
Total messages: 65 (24 generated)
xhwang@chromium.org changed reviewers: + waffles@chromium.org
This is not really used in this CL, but will be necessary for my next CL. I can either land this separately, or as part of my next CL. PTAL
https://chromiumcodereview.appspot.com/1996863002/diff/1/chrome/browser/conte... File chrome/browser/content_settings/content_settings_browsertest.cc (right): https://chromiumcodereview.appspot.com/1996863002/diff/1/chrome/browser/conte... chrome/browser/content_settings/content_settings_browsertest.cc:324: PepperContentSettingsSpecialCasesTest() { enable_component_update(); } This just shows how this new method could be used. For now it doesn't have any effect to the test (I think).
The CQ bit was checked by xhwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996863002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996863002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Provide option to enable component update in browser tests BUG=613442 ========== to ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. BUG=613442 ==========
xhwang@chromium.org changed reviewers: + ddorwin@chromium.org
Description was changed from ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. BUG=613442 ========== to ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. - Enable more tests that wasn't possible when CDM is manually registered. BUG=613442 ==========
This will be the CL to update tests after Widevine CDM is bundled. PTAL https://chromiumcodereview.appspot.com/1996863002/diff/20001/chrome/test/base... File chrome/test/base/in_process_browser_test.cc (right): https://chromiumcodereview.appspot.com/1996863002/diff/20001/chrome/test/base... chrome/test/base/in_process_browser_test.cc:142: , this is done by git cl format
https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/c... File chrome/browser/content_settings/content_settings_browsertest.cc (right): https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/c... chrome/browser/content_settings/content_settings_browsertest.cc:324: PepperContentSettingsSpecialCasesTest() { enable_component_update(); } Ditto with next comment, especially since this will not be necessary once the related CDM product code is no longer necessary. https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... File chrome/browser/media/encrypted_media_browsertest.cc (right): https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... chrome/browser/media/encrypted_media_browsertest.cc:101: EncryptedMediaTestBase() { enable_component_update(); } What are the test performance impacts? Should we put this inside ENABLE_PEPPER_CDMS? https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... File chrome/browser/media/encrypted_media_supported_types_browsertest.cc (right): https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... chrome/browser/media/encrypted_media_supported_types_browsertest.cc:650: #if !defined(WIDEVINE_CDM_AVAILABLE) ditto with below. https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... chrome/browser/media/encrypted_media_supported_types_browsertest.cc:675: #if !defined(WIDEVINE_CDM_AVAILABLE) This test could be run if defined(WIDEVINE_CDM_IS_COMPONENT) and we disabled the component update. Actually, in that case, we don't even need to use the wrong path.
Description was changed from ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. - Enable more tests that wasn't possible when CDM is manually registered. BUG=613442 ========== to ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. - Enable more tests that wasn't possible when CDM is manually registered. BUG=613442,582622,613581 ==========
lgtm https://codereview.chromium.org/1996863002/diff/40001/chrome/test/base/test_l... File chrome/test/base/test_launcher_utils.h (right): https://codereview.chromium.org/1996863002/diff/40001/chrome/test/base/test_l... chrome/test/base/test_launcher_utils.h:23: bool enable_component_update = false); Not sure how people will feel about this. I'm OK with it.
https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/c... File chrome/browser/content_settings/content_settings_browsertest.cc (right): https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/c... chrome/browser/content_settings/content_settings_browsertest.cc:324: PepperContentSettingsSpecialCasesTest() { enable_component_update(); } On 2016/05/20 16:56:15, ddorwin wrote: > Ditto with next comment, especially since this will not be necessary once the > related CDM product code is no longer necessary. Done. https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... File chrome/browser/media/encrypted_media_browsertest.cc (right): https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... chrome/browser/media/encrypted_media_browsertest.cc:101: EncryptedMediaTestBase() { enable_component_update(); } On 2016/05/20 16:56:15, ddorwin wrote: > What are the test performance impacts? Should we put this inside > ENABLE_PEPPER_CDMS? I suppose we'll load existing components, then before we really try to do update (in ~6 mins), the test should already finished. Putting them in ENABLE_PEPPER_CDMS makes sense. Done. But I think long term we shouldn't disable loading existing components at all for better test coverage. See issue 613581. https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... File chrome/browser/media/encrypted_media_supported_types_browsertest.cc (right): https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... chrome/browser/media/encrypted_media_supported_types_browsertest.cc:650: #if !defined(WIDEVINE_CDM_AVAILABLE) On 2016/05/20 16:56:15, ddorwin wrote: > ditto with below. see below. https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... chrome/browser/media/encrypted_media_supported_types_browsertest.cc:675: #if !defined(WIDEVINE_CDM_AVAILABLE) On 2016/05/20 16:56:15, ddorwin wrote: > This test could be run if defined(WIDEVINE_CDM_IS_COMPONENT) and we disabled the > component update. Actually, in that case, we don't even need to use the wrong > path. I don't think it's super interesting to test the case where component update is disabled. It's only for dev and we might actually change what it does. If we don't use the wrong path, CDM will not be registered. I think this is already covered above. This test is really for WidevineCDMRegistered_but_...
lgtm https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... File chrome/browser/media/encrypted_media_supported_types_browsertest.cc (right): https://chromiumcodereview.appspot.com/1996863002/diff/40001/chrome/browser/m... chrome/browser/media/encrypted_media_supported_types_browsertest.cc:675: #if !defined(WIDEVINE_CDM_AVAILABLE) On 2016/05/20 21:23:28, xhwang wrote: > On 2016/05/20 16:56:15, ddorwin wrote: > > This test could be run if defined(WIDEVINE_CDM_IS_COMPONENT) and we disabled > the > > component update. Actually, in that case, we don't even need to use the wrong > > path. > > I don't think it's super interesting to test the case where component update is > disabled. It's only for dev and we might actually change what it does. > > If we don't use the wrong path, CDM will not be registered. I think this is > already covered above. This test is really for WidevineCDMRegistered_but_... I was thinking it's interesting for the case where the component is not present, but that should no longer be an issue. :)
xhwang@chromium.org changed reviewers: + phajdan.jr@chromium.org
phajdan.jr@chromium.org: Please OWNERS review chrome/test/* changes.
xhwang@chromium.org changed reviewers: + thestig@chromium.org
thestig: Please OWNERS review chrome/browser changes.
lgtm
The CQ bit was checked by xhwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996863002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996863002/60001
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 xhwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996863002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996863002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. - Enable more tests that wasn't possible when CDM is manually registered. BUG=613442,582622,613581 ========== to ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. - Enable more tests that wasn't possible when CDM is manually registered. TBR=phajdan.jr@chromium.org BUG=613442,582622,613581 ==========
TBRing phajdan.jr@chromium.org for minor changes in chrome/test/*
The CQ bit was checked by xhwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996863002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996863002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I'm attending a workshop that makes me respond slower. I prefer this change not to be TBR-ed. Feel free to ping me for re-review. https://codereview.chromium.org/1996863002/diff/60001/chrome/test/base/in_pro... File chrome/test/base/in_process_browser_test.h (right): https://codereview.chromium.org/1996863002/diff/60001/chrome/test/base/in_pro... chrome/test/base/in_process_browser_test.h:230: void PrepareTestCommandLine(base::CommandLine* command_line); How about we make this method virtual and override it in tests? I'd prefer not to have one-off "hacks" for things like this (even though I see some similar ones have apparently made it to this code). You can then remove flags in a way similar to src/chrome/browser/process_singleton_browsertest.cc : // Add the normal test-mode switches, except for the ones we're adding // ourselves. base::CommandLine standard_switches(base::CommandLine::NO_PROGRAM); test_launcher_utils::PrepareBrowserCommandLineForTests(&standard_switches); const base::CommandLine::SwitchMap& switch_map = standard_switches.GetSwitches(); for (base::CommandLine::SwitchMap::const_iterator i = switch_map.begin(); i != switch_map.end(); ++i) { const std::string& switch_name = i->first; if (switch_name == switches::kUserDataDir || switch_name == switches::kForceFirstRun || switch_name == switches::kNoFirstRun) continue; command_line.AppendSwitchNative(switch_name, i->second); } WDYT?
Description was changed from ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. - Enable more tests that wasn't possible when CDM is manually registered. TBR=phajdan.jr@chromium.org BUG=613442,582622,613581 ========== to ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. - Enable more tests that wasn't possible when CDM is manually registered. BUG=613442,582622,613581 ==========
https://codereview.chromium.org/1996863002/diff/60001/chrome/test/base/in_pro... File chrome/test/base/in_process_browser_test.h (right): https://codereview.chromium.org/1996863002/diff/60001/chrome/test/base/in_pro... chrome/test/base/in_process_browser_test.h:230: void PrepareTestCommandLine(base::CommandLine* command_line); On 2016/05/24 20:45:40, Paweł Hajdan Jr. wrote: > How about we make this method virtual and override it in tests? > > I'd prefer not to have one-off "hacks" for things like this (even though I see > some similar ones have apparently made it to this code). > > You can then remove flags in a way similar to > src/chrome/browser/process_singleton_browsertest.cc : > > // Add the normal test-mode switches, except for the ones we're adding > // ourselves. > base::CommandLine standard_switches(base::CommandLine::NO_PROGRAM); > test_launcher_utils::PrepareBrowserCommandLineForTests(&standard_switches); > const base::CommandLine::SwitchMap& switch_map = > standard_switches.GetSwitches(); > for (base::CommandLine::SwitchMap::const_iterator i = switch_map.begin(); > i != switch_map.end(); ++i) { > const std::string& switch_name = i->first; > if (switch_name == switches::kUserDataDir || > switch_name == switches::kForceFirstRun || > switch_name == switches::kNoFirstRun) > continue; > > command_line.AppendSwitchNative(switch_name, i->second); > } > > WDYT? Thanks for the comment. Let me give it a try.
On 2016/05/24 21:16:30, xhwang wrote: > https://codereview.chromium.org/1996863002/diff/60001/chrome/test/base/in_pro... > File chrome/test/base/in_process_browser_test.h (right): > > https://codereview.chromium.org/1996863002/diff/60001/chrome/test/base/in_pro... > chrome/test/base/in_process_browser_test.h:230: void > PrepareTestCommandLine(base::CommandLine* command_line); > On 2016/05/24 20:45:40, Paweł Hajdan Jr. wrote: > > How about we make this method virtual and override it in tests? > > > > I'd prefer not to have one-off "hacks" for things like this (even though I see > > some similar ones have apparently made it to this code). > > > > You can then remove flags in a way similar to > > src/chrome/browser/process_singleton_browsertest.cc : > > > > // Add the normal test-mode switches, except for the ones we're adding > > // ourselves. > > base::CommandLine standard_switches(base::CommandLine::NO_PROGRAM); > > > test_launcher_utils::PrepareBrowserCommandLineForTests(&standard_switches); > > const base::CommandLine::SwitchMap& switch_map = > > standard_switches.GetSwitches(); > > for (base::CommandLine::SwitchMap::const_iterator i = switch_map.begin(); > > i != switch_map.end(); ++i) { > > const std::string& switch_name = i->first; > > if (switch_name == switches::kUserDataDir || > > switch_name == switches::kForceFirstRun || > > switch_name == switches::kNoFirstRun) > > continue; > > > > command_line.AppendSwitchNative(switch_name, i->second); > > } > > > > WDYT? > > Thanks for the comment. Let me give it a try. BTW, I wish there's just a RemoveSwitch() method on CommandLine, which will make this much easier.
The CQ bit was checked by xhwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996863002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996863002/80001
Paweł Hajdan Jr.: PTAL again! https://chromiumcodereview.appspot.com/1996863002/diff/60001/chrome/test/base... File chrome/test/base/in_process_browser_test.h (right): https://chromiumcodereview.appspot.com/1996863002/diff/60001/chrome/test/base... chrome/test/base/in_process_browser_test.h:230: void PrepareTestCommandLine(base::CommandLine* command_line); On 2016/05/24 21:16:30, xhwang wrote: > On 2016/05/24 20:45:40, Paweł Hajdan Jr. wrote: > > How about we make this method virtual and override it in tests? > > > > I'd prefer not to have one-off "hacks" for things like this (even though I see > > some similar ones have apparently made it to this code). > > > > You can then remove flags in a way similar to > > src/chrome/browser/process_singleton_browsertest.cc : > > > > // Add the normal test-mode switches, except for the ones we're adding > > // ourselves. > > base::CommandLine standard_switches(base::CommandLine::NO_PROGRAM); > > > test_launcher_utils::PrepareBrowserCommandLineForTests(&standard_switches); > > const base::CommandLine::SwitchMap& switch_map = > > standard_switches.GetSwitches(); > > for (base::CommandLine::SwitchMap::const_iterator i = switch_map.begin(); > > i != switch_map.end(); ++i) { > > const std::string& switch_name = i->first; > > if (switch_name == switches::kUserDataDir || > > switch_name == switches::kForceFirstRun || > > switch_name == switches::kNoFirstRun) > > continue; > > > > command_line.AppendSwitchNative(switch_name, i->second); > > } > > > > WDYT? > > Thanks for the comment. Let me give it a try. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Dry run: None
On 2016/05/25 08:03:24, commit-bot: I haz the power wrote: > Dry run: None Actually I thought about this more and I feel we should be able to further simplify this. Subclasses of InProcessBrowserTest should be able to just call RemoveSwitchFromDrfaultCommand(switch) to remove a switch from the default command line. I'll give this idea a try now.
LGTM w/comment https://codereview.chromium.org/1996863002/diff/80001/chrome/test/base/test_l... File chrome/test/base/test_launcher_utils.cc (right): https://codereview.chromium.org/1996863002/diff/80001/chrome/test/base/test_l... chrome/test/base/test_launcher_utils.cc:75: void RemoveCommandLineSwitch(const base::CommandLine& in_command_line, The base place for that would IMO be right in src/base CommandLine implementation. I'm fine with a TODO though, so you can land this CL earlier.
comments addressed
https://codereview.chromium.org/1996863002/diff/80001/chrome/test/base/test_l... File chrome/test/base/test_launcher_utils.cc (right): https://codereview.chromium.org/1996863002/diff/80001/chrome/test/base/test_l... chrome/test/base/test_launcher_utils.cc:75: void RemoveCommandLineSwitch(const base::CommandLine& in_command_line, On 2016/05/25 16:15:11, Paweł Hajdan Jr. wrote: > The base place for that would IMO be right in src/base CommandLine > implementation. > > I'm fine with a TODO though, so you can land this CL earlier. Agreed that if we have a CommandLine::RemoveSwitch() then everything will be super easy here. Added a TODO. With that I am not gonna try the other idea I just mentioned, in favor of CommandLine::RemoveSwitch().
The CQ bit was checked by xhwang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from waffles@chromium.org, thestig@chromium.org, ddorwin@chromium.org, phajdan.jr@chromium.org Link to the patchset: https://codereview.chromium.org/1996863002/#ps100001 (title: "comments addressed")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996863002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996863002/100001
Message was sent while issue was closed.
Description was changed from ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. - Enable more tests that wasn't possible when CDM is manually registered. BUG=613442,582622,613581 ========== to ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. - Enable more tests that wasn't possible when CDM is manually registered. BUG=613442,582622,613581 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. - Enable more tests that wasn't possible when CDM is manually registered. BUG=613442,582622,613581 ========== to ========== media: Use bundled Widevine CDM in encrypted media browser tests - Provide option to enable component update in browser tests - Enable component update in encrypted media related browser tests to pick up the bundled Widevine CDM. - Remove code that manually registers Widevine CDM. - Enable more tests that wasn't possible when CDM is manually registered. BUG=613442,582622,613581 Committed: https://crrev.com/3be44be552846983cf371256718ce8ec81c565b4 Cr-Commit-Position: refs/heads/master@{#395922} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/3be44be552846983cf371256718ce8ec81c565b4 Cr-Commit-Position: refs/heads/master@{#395922}
Message was sent while issue was closed.
thakis@chromium.org changed reviewers: + thakis@chromium.org
Message was sent while issue was closed.
The clang/win bots doing official builds have been failing browser_tests (widevine-related stuff) after this went in. I'm guessing it broke browser_tests on Windows in non-clang builds as well. https://bugs.chromium.org/p/chromium/issues/detail?id=614745 has some more details (but we had a bunch of different redness as well, so that bug isn't solely about the widevine problems)
Message was sent while issue was closed.
On 2016/05/26 14:56:31, Nico wrote: > The clang/win bots doing official builds have been failing browser_tests > (widevine-related stuff) after this went in. I'm guessing it broke browser_tests > on Windows in non-clang builds as well. > > https://bugs.chromium.org/p/chromium/issues/detail?id=614745 has some more > details (but we had a bunch of different redness as well, so that bug isn't > solely about the widevine problems) looking
Message was sent while issue was closed.
On 2016/05/26 15:56:00, xhwang wrote: > On 2016/05/26 14:56:31, Nico wrote: > > The clang/win bots doing official builds have been failing browser_tests > > (widevine-related stuff) after this went in. I'm guessing it broke > browser_tests > > on Windows in non-clang builds as well. > > > > https://bugs.chromium.org/p/chromium/issues/detail?id=614745 has some more > > details (but we had a bunch of different redness as well, so that bug isn't > > solely about the widevine problems) > > looking any luck?
Message was sent while issue was closed.
On 2016/05/26 17:56:10, Nico wrote: > On 2016/05/26 15:56:00, xhwang wrote: > > On 2016/05/26 14:56:31, Nico wrote: > > > The clang/win bots doing official builds have been failing browser_tests > > > (widevine-related stuff) after this went in. I'm guessing it broke > > browser_tests > > > on Windows in non-clang builds as well. > > > > > > https://bugs.chromium.org/p/chromium/issues/detail?id=614745 has some more > > > details (but we had a bunch of different redness as well, so that bug isn't > > > solely about the widevine problems) > > > > looking > > any luck? This change affects Mac and Windows in the same way. I tested Mac and it's working fine with an official build. I am building Windows right now. Can you point me to the Windows non-clang official bots to see whether the tests are failing there as well?
Message was sent while issue was closed.
I don't know of public bots that build official and run tests (hence the "guessing" in "I'm guessing it broke browser_tests on Windows in non-clang builds as well -- in almost all cases when tests fail on just that official clang/win bot and not on the other clang/win bots it's because the test is just broken in official). If it is broken in official though, you'll have a bug assigned to you by the good people doing official builds for canary releases tomorrow morning. I can try and do a local official build with msvc to check as well.
Message was sent while issue was closed.
On 2016/05/26 18:53:03, Nico wrote: > I don't know of public bots that build official and run tests (hence the > "guessing" in "I'm guessing it broke browser_tests on Windows in non-clang > builds as well -- in almost all cases when tests fail on just that official > clang/win bot and not on the other clang/win bots it's because the test is just > broken in official). If it is broken in official though, you'll have a bug > assigned to you by the good people doing official builds for canary releases > tomorrow morning. > > I can try and do a local official build with msvc to check as well. Thanks for the info. No wonder I can't find the bot that does official builds and run tests. I am building an official build now, but it's Windows, so it'll take some time.
Message was sent while issue was closed.
On 2016/05/26 18:58:25, xhwang wrote: > On 2016/05/26 18:53:03, Nico wrote: > > I don't know of public bots that build official and run tests (hence the > > "guessing" in "I'm guessing it broke browser_tests on Windows in non-clang > > builds as well -- in almost all cases when tests fail on just that official > > clang/win bot and not on the other clang/win bots it's because the test is > just > > broken in official). If it is broken in official though, you'll have a bug > > assigned to you by the good people doing official builds for canary releases > > tomorrow morning. > > > > I can try and do a local official build with msvc to check as well. > > Thanks for the info. No wonder I can't find the bot that does official builds > and run tests. > > I am building an official build now, but it's Windows, so it'll take some time. Couldn't repro on ToT Windows official build. Is there anything special about this clang bot? Is it ia32 or x64?
Message was sent while issue was closed.
happens on both the 32-bit and the 64-bit official bot. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
