Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
10784009:
screenshot disabling policy tests (Closed)
|
Created:
8 years, 5 months ago by qfel Modified:
8 years, 3 months ago Reviewers:
rkc, bartfab (slow), sky, Mattias Nissler (ping if slow), Nirnimesh, Mihai Parparita -not on Chrome, Matt Perry, Paweł Hajdan Jr., Joao da Silva, darin (slow to review), Ben Goodger (Google), xot CC:
chromium-reviews, mihaip-chromium-reviews_chromium.org, sadrul, ben+watch_chromium.org, Aaron Boodman, anantha, dyu1, dennis_jeffrey, Joao da Silva, bartfab (slow) Base URL:
http://git.chromium.org/chromium/src.git@disable_screenshots Visibility:
Public. |
DescriptionTests for disabling screenshots policy (see https://chromiumcodereview.appspot.com/10692110/):
* Test screenshot keyboard shortcut
* Test feedback form screenshot element
* Test CaptureVisibleTab extension function
BUG=chromium-os:24747
TEST=browser_tests, policy.py
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=154714
Patch Set 1 : Tests #
Total comments: 15
Patch Set 2 : Addressed comments (1) #
Total comments: 1
Patch Set 3 : accelerator_table.h, renamed ApplyAshAccelerator, addressed comments (2) #
Total comments: 1
Patch Set 4 : Addressed comments (3) #Patch Set 5 : Rebased, removed #pragma once #
Total comments: 1
Patch Set 6 : Rebased, removed conditional compilation in enum definition #Patch Set 7 : Fixed Android and Mac builds #Patch Set 8 : Updated accelerator_action.h #
Total comments: 8
Patch Set 9 : Addressed comments (4) #Patch Set 10 : Rebased, incorporated new pyauto RunCommand changes #Patch Set 11 : Removed pyauto tests #Patch Set 12 : Fixed gypi file #Patch Set 13 : Browser tests #Patch Set 14 : Added missing CrOS guards #
Total comments: 22
Patch Set 15 : Addressed comments #Patch Set 16 : Rebased #Messages
Total messages: 44 (0 generated)
Nirnimesh, I think you could handle this review much better than me :) https://chromiumcodereview.appspot.com/10784009/diff/2001/chrome/test/functio... File chrome/test/functional/policy.py (right): https://chromiumcodereview.appspot.com/10784009/diff/2001/chrome/test/functio... chrome/test/functional/policy.py:715: timeout=5, expect_retval=True)) nirnimesh: I'm unhappy with these timeouts, but essentially we're waiting for nothing to happen here. Any suggestions on how to improve this?
Please fix the CL title. https://chromiumcodereview.appspot.com/10784009/diff/2001/chrome/test/data/ex... File chrome/test/data/extensions/api_test/tabs/capture_visible_tab/test_disabled.js (right): https://chromiumcodereview.appspot.com/10784009/diff/2001/chrome/test/data/ex... chrome/test/data/extensions/api_test/tabs/capture_visible_tab/test_disabled.js:1: var pass = chrome.test.callbackPass; License header? https://chromiumcodereview.appspot.com/10784009/diff/2001/chrome/test/functio... File chrome/test/functional/policy.py (right): https://chromiumcodereview.appspot.com/10784009/diff/2001/chrome/test/functio... chrome/test/functional/policy.py:691: def testDisableScreenshotFile(self): one-line docstring please https://chromiumcodereview.appspot.com/10784009/diff/2001/chrome/test/functio... chrome/test/functional/policy.py:695: TAKE_SCREENSHOT = 34 # See AcceleratorAction enum in accelerator_table.h This seems bad. Can you try to swig out the accelerator_table.h file? See how chrome/app/chrome_command_ids.h is swigged out in chrome/test/pyautolib/pyautolib.i https://chromiumcodereview.appspot.com/10784009/diff/2001/chrome/test/functio... chrome/test/functional/policy.py:698: return len(os.listdir(download_dir)) It's bad to have a function use vars defined after the function. It's a one-liner anyway. please choose one of: 1. get rid of the function 2. define the function after the var (or define a lamda) https://chromiumcodereview.appspot.com/10784009/diff/2001/chrome/test/functio... chrome/test/functional/policy.py:715: timeout=5, expect_retval=True)) On 2012/07/16 10:51:55, Mattias Nissler wrote: > nirnimesh: I'm unhappy with these timeouts, but essentially we're waiting for > nothing to happen here. Any suggestions on how to improve this? Unfortunately, I don't have a good solution for this problem. We avoid using assertFalse with WaitUntil, for this very reason. Instead, is it possible to get a notification back from the browser when the accelerator has been handled completely? https://chromiumcodereview.appspot.com/10784009/diff/2001/chrome/test/functio... chrome/test/functional/policy.py:717: def testDisableScreenshotFeedback(self): docstring please https://chromiumcodereview.appspot.com/10784009/diff/2001/chrome/test/functio... chrome/test/functional/policy.py:720: self.WaitUntilNavigationCompletes(tab_index=1) you might want to wait for tab count to become 2 before this
http://codereview.chromium.org/10784009/diff/2001/chrome/test/data/extensions... File chrome/test/data/extensions/api_test/tabs/capture_visible_tab/test_disabled.js (right): http://codereview.chromium.org/10784009/diff/2001/chrome/test/data/extensions... chrome/test/data/extensions/api_test/tabs/capture_visible_tab/test_disabled.js:1: var pass = chrome.test.callbackPass; On 2012/07/16 20:01:50, Nirnimesh wrote: > License header? Done. http://codereview.chromium.org/10784009/diff/2001/chrome/test/functional/poli... File chrome/test/functional/policy.py (right): http://codereview.chromium.org/10784009/diff/2001/chrome/test/functional/poli... chrome/test/functional/policy.py:691: def testDisableScreenshotFile(self): On 2012/07/16 20:01:50, Nirnimesh wrote: > one-line docstring please Done. http://codereview.chromium.org/10784009/diff/2001/chrome/test/functional/poli... chrome/test/functional/policy.py:695: TAKE_SCREENSHOT = 34 # See AcceleratorAction enum in accelerator_table.h On 2012/07/16 20:01:50, Nirnimesh wrote: > This seems bad. Can you try to swig out the accelerator_table.h file? > See how chrome/app/chrome_command_ids.h is swigged out in > chrome/test/pyautolib/pyautolib.i Done. http://codereview.chromium.org/10784009/diff/2001/chrome/test/functional/poli... chrome/test/functional/policy.py:698: return len(os.listdir(download_dir)) On 2012/07/16 20:01:50, Nirnimesh wrote: > It's bad to have a function use vars defined after the function. It's a > one-liner anyway. please choose one of: > 1. get rid of the function > 2. define the function after the var (or define a lamda) Done. http://codereview.chromium.org/10784009/diff/2001/chrome/test/functional/poli... chrome/test/functional/policy.py:715: timeout=5, expect_retval=True)) > Instead, is it possible to get a notification back from the browser when the > accelerator has been handled completely? The accelerator is always considered handled. Its handler returns when saving of screenshot has been posted to the file thread (or grabbing the screenshot failed). Perhaps we could sync with file thread and see if the file appears afterwards, if you do not fear depending on implementation details (eg. just posting a blocking job to the file thread). http://codereview.chromium.org/10784009/diff/2001/chrome/test/functional/poli... chrome/test/functional/policy.py:717: def testDisableScreenshotFeedback(self): On 2012/07/16 20:01:50, Nirnimesh wrote: > docstring please Done. http://codereview.chromium.org/10784009/diff/2001/chrome/test/functional/poli... chrome/test/functional/policy.py:720: self.WaitUntilNavigationCompletes(tab_index=1) On 2012/07/16 20:01:50, Nirnimesh wrote: > you might want to wait for tab count to become 2 before this Done.
LGTM http://codereview.chromium.org/10784009/diff/9001/chrome/test/pyautolib/pyaut... File chrome/test/pyautolib/pyauto.py (right): http://codereview.chromium.org/10784009/diff/9001/chrome/test/pyautolib/pyaut... chrome/test/pyautolib/pyauto.py:5434: def ApplyAshAccelerator(self, action): Add a docstring please. Also a Returns: section. Also, mention the path to accelerator_table.h for reference.
Sorry, it turns out I failed to run chrome-os only test correctly, and didn't catch SWIG errors. Now it should be OK. Factored out AcceleratorAction from accelerator_table.h for use with SWIG. Renamed ApplyAshAccelerator to RunAshCommand, to be more consistent with AppleAccelerator/RunCommand - RunCommand is a synchronous version of ApplyAccelerator.
LGTM http://codereview.chromium.org/10784009/diff/6010/ash/accelerators/accelerato... File ash/accelerators/accelerator_table.h (right): http://codereview.chromium.org/10784009/diff/6010/ash/accelerators/accelerato... ash/accelerators/accelerator_table.h:12: // Used to inclulde AcceleratorAction definition which originally was here end with .
Mihai: please review chrome/test/data/extensions/api_test/tabs/capture_visible_tab/* and chrome/browser/extensions/extension_tabs_apitest.cc
Since Matt reviewed the CL that adds the functionality, he seems like a more appropriate reviewer for this CL.
Matt, since you picked up the job to review parent CL, please also see chrome/test/data/extensions/api_test/tabs/capture_visible_tab/* and chrome/browser/extensions/extension_tabs_apitest.cc (added you to reviewers)
On 2012/07/17 23:05:34, Nirnimesh wrote: > LGTM > > http://codereview.chromium.org/10784009/diff/6010/ash/accelerators/accelerato... > File ash/accelerators/accelerator_table.h (right): > > http://codereview.chromium.org/10784009/diff/6010/ash/accelerators/accelerato... > ash/accelerators/accelerator_table.h:12: // Used to inclulde AcceleratorAction > definition which originally was here > end with . Oh, it fails on release builds. Problem is, accelerator_action.h I factored out of accelerator_table.h contains conditional definitions in enum. Normally, PRINT_LAYER_HIERARCHY and PRINT_WINDOW_HIERARCHY are defined, but in case of release build (NDEBUG) they are not. Currently SWIG does not use the same preprocessor defines as C++ compiler, so it generates pyautolib_wrap.cc file using those constants - and then C++ compiler fails with NDEBUG defined (because in that case they are not found). See http://build.chromium.org/p/tryserver.chromium/builders/win_rel/builds/45763/.... Maybe we could just get rid of those ifdefs (but it may break binary compatibility if they are used across library boundaries), but IMHO we should use SWIG with the same set of definitions as C++ compiler (not sure how to reference them in .gypi file though).
lgtm http://codereview.chromium.org/10784009/diff/27002/chrome/test/data/extension... File chrome/test/data/extensions/api_test/tabs/capture_visible_tab/test_disabled.js (right): http://codereview.chromium.org/10784009/diff/27002/chrome/test/data/extension... chrome/test/data/extensions/api_test/tabs/capture_visible_tab/test_disabled.js:28: 'Taking screenshots has been disabled')); nit: make this a constant at the top of the file and reference that
Sky, can you review ash/* files?
http://codereview.chromium.org/10784009/diff/36019/ash/accelerators/accelerat... File ash/accelerators/accelerator_action.h (right): http://codereview.chromium.org/10784009/diff/36019/ash/accelerators/accelerat... ash/accelerators/accelerator_action.h:5: #ifndef ASH_ACCELERATORS_ACTION_TABLE_H_ Should match file name. http://codereview.chromium.org/10784009/diff/36019/ash/accelerators/accelerat... File ash/accelerators/accelerator_table.h (right): http://codereview.chromium.org/10784009/diff/36019/ash/accelerators/accelerat... ash/accelerators/accelerator_table.h:11: // Used to inclulde AcceleratorAction definition which originally was here. There is no reason to specially comment this include. The header needs AcceleratorAction so it has to import it. Move import with rest of imports above.
Some tentative comments. https://chromiumcodereview.appspot.com/10784009/diff/36019/chrome/test/functi... File chrome/test/functional/policy.py (right): https://chromiumcodereview.appspot.com/10784009/diff/36019/chrome/test/functi... chrome/test/functional/policy.py:705: def FilesCount(): Can you make FilesCount() only count files that begin with "Screenshot*"? https://chromiumcodereview.appspot.com/10784009/diff/36019/chrome/test/functi... chrome/test/functional/policy.py:710: self.assertTrue(self.RunAshCommand(pyauto.TAKE_SCREENSHOT)) Shouldn't you first make sure that DisableScreenshots is false or "Not Set" before trying this?
http://codereview.chromium.org/10784009/diff/36019/ash/accelerators/accelerat... File ash/accelerators/accelerator_action.h (right): http://codereview.chromium.org/10784009/diff/36019/ash/accelerators/accelerat... ash/accelerators/accelerator_action.h:5: #ifndef ASH_ACCELERATORS_ACTION_TABLE_H_ On 2012/08/02 20:14:39, sky wrote: > Should match file name. Done. http://codereview.chromium.org/10784009/diff/36019/ash/accelerators/accelerat... File ash/accelerators/accelerator_table.h (right): http://codereview.chromium.org/10784009/diff/36019/ash/accelerators/accelerat... ash/accelerators/accelerator_table.h:11: // Used to inclulde AcceleratorAction definition which originally was here. On 2012/08/02 20:14:39, sky wrote: > There is no reason to specially comment this include. The header needs > AcceleratorAction so it has to import it. Move import with rest of imports > above. Done. http://codereview.chromium.org/10784009/diff/36019/chrome/test/functional/pol... File chrome/test/functional/policy.py (right): http://codereview.chromium.org/10784009/diff/36019/chrome/test/functional/pol... chrome/test/functional/policy.py:705: def FilesCount(): On 2012/08/03 15:40:36, xot wrote: > Can you make FilesCount() only count files that begin with "Screenshot*"? Done. http://codereview.chromium.org/10784009/diff/36019/chrome/test/functional/pol... chrome/test/functional/policy.py:710: self.assertTrue(self.RunAshCommand(pyauto.TAKE_SCREENSHOT)) On 2012/08/03 15:40:36, xot wrote: > Shouldn't you first make sure that DisableScreenshots is false or "Not Set" > before trying this? I assumed it's not set = false by default, but I suppose it's a good idea to ensure it so the test is less vulnerable to environment configuration changes.
I just realized I need a reviewer for chrome/browser/automation/*, so adding Paweł.
On 2012/08/09 18:38:05, qfel wrote: > I just realized I need a reviewer for chrome/browser/automation/*, so adding > Paweł. Due to Paweł's absence, bouncing to Darin
So, wallpaper is implemented almost entirely in ash, and should be tested there. The whole purpose of ash was to get ChromeOS code out of Chrome. We should not be adding more code to Chrome even just to test things that were moved out of Chrome and into Ash.
e.g. see ash_unittests.exe
Ben, if the RunAshCommand() automation call gets added, it can be used for other (non-browser) autotest tests on chromeos. WDYT?
And when I started that CL I heard the goal is to have pyauto test for every policy.. Anyway, this is taking so long. How about I strip the pyauto part away and commit extension tests?
On 2012/08/16 08:47:44, qfel wrote: > And when I started that CL I heard the goal is to have pyauto test for every > policy.. Anyway, this is taking so long. How about I strip the pyauto part away > and commit extension tests? Well, that was before Darin's update in directions regarding use of PyAuto :) Committing just the extension tests for now sounds good to me.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qfel@google.com/10784009/48001
Try job failure for 10784009-48001 (retry) on win for step "compile" (clobber build). It's a second try, previously, step "compile" failed. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qfel@google.com/10784009/48001
Try job failure for 10784009-48001 (retry) on linux_rel for step "interactive_ui_tests". It's a second try, previously, step "interactive_ui_tests" failed. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qfel@google.com/10784009/48001
Try job failure for 10784009-48001 (retry) on win for step "compile" (clobber build). It's a second try, previously, step "compile" failed. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
LGTM
Converted old committed pyauto tests to browser tests.
On 2012/08/30 12:19:03, qfel wrote: > Converted old committed pyauto tests to browser tests. Sorry, meant to say uncommitted.
https://chromiumcodereview.appspot.com/10784009/diff/71001/chrome/browser/pol... File chrome/browser/policy/policy_browsertest.cc (right): https://chromiumcodereview.appspot.com/10784009/diff/71001/chrome/browser/pol... chrome/browser/policy/policy_browsertest.cc:169: class LoadObserver : public content::NotificationObserver { #include "content/public/browser/notification_observer.h" is missing for this. https://chromiumcodereview.appspot.com/10784009/diff/71001/chrome/browser/pol... chrome/browser/policy/policy_browsertest.cc:186: CHECK_EQ(type, content::NOTIFICATION_LOAD_STOP); Why CHECK_EQ, not ASSERT_EQ? https://chromiumcodereview.appspot.com/10784009/diff/71001/chrome/browser/pol... chrome/browser/policy/policy_browsertest.cc:200: DownloadPrefs* download_prefs = DownloadPrefs::FromBrowserContext( #include "chrome/browser/download/download_prefs.h" is missing for this. https://chromiumcodereview.appspot.com/10784009/diff/71001/chrome/browser/pol... chrome/browser/policy/policy_browsertest.cc:213: // callback to wait for message loop to process previous messages. This is what base::DoNothing is for. https://chromiumcodereview.appspot.com/10784009/diff/71001/chrome/browser/pol... chrome/browser/policy/policy_browsertest.cc:238: POLICY_SCOPE_USER, base::Value::CreateBooleanValue(!enabled)); #include "base/values.h" is missing for this. https://chromiumcodereview.appspot.com/10784009/diff/71001/chrome/browser/pol... chrome/browser/policy/policy_browsertest.cc:254: std::wstring(), L"" would work just as well. https://chromiumcodereview.appspot.com/10784009/diff/71001/chrome/browser/pol... chrome/browser/policy/policy_browsertest.cc:255: L"function btest_initCompleted(url) {" Indentation throughout this JS code should be two spaces, not one space.
Nice tests! Please see the comment about the WindowedNotificationObserver. http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... File chrome/browser/policy/policy_browsertest.cc (right): http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:169: class LoadObserver : public content::NotificationObserver { This is essentially a WindowedNotificationObserver (from content/public/test/test_utils.cc), can't it be used instead? http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:271: L" btest_initCompleted(img.src);", else domAutomationController.send(false); http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:273: EXPECT_TRUE(result == enabled); EXPECT_EQ http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:284: ash::TAKE_SCREENSHOT, ui::Accelerator()); indent
http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... File chrome/browser/policy/policy_browsertest.cc (right): http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:169: class LoadObserver : public content::NotificationObserver { On 2012/09/03 09:01:04, Joao da Silva wrote: > This is essentially a WindowedNotificationObserver (from > content/public/test/test_utils.cc), can't it be used instead? Seems I just missed its existence. http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:186: CHECK_EQ(type, content::NOTIFICATION_LOAD_STOP); On 2012/08/31 15:50:13, bartfab wrote: > Why CHECK_EQ, not ASSERT_EQ? I wonder myself.. Seems I just happened to see similar code with check when writing this. Should be assert though. http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:200: DownloadPrefs* download_prefs = DownloadPrefs::FromBrowserContext( On 2012/08/31 15:50:13, bartfab wrote: > #include "chrome/browser/download/download_prefs.h" is missing for this. It's in the OS_CHROMEOS guarded include section. Moved to unguarded section (now that I think of it, I recall those kind of included should not be guarded). http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:213: // callback to wait for message loop to process previous messages. On 2012/08/31 15:50:13, bartfab wrote: > This is what base::DoNothing is for. Done. http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:238: POLICY_SCOPE_USER, base::Value::CreateBooleanValue(!enabled)); On 2012/08/31 15:50:13, bartfab wrote: > #include "base/values.h" is missing for this. Done. http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:254: std::wstring(), On 2012/08/31 15:50:13, bartfab wrote: > L"" would work just as well. The code where I found ExecuteJavaScriptAndExtractBool used explicit constructor call, so I decided to do the same. http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:255: L"function btest_initCompleted(url) {" On 2012/08/31 15:50:13, bartfab wrote: > Indentation throughout this JS code should be two spaces, not one space. Done. http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:271: L" btest_initCompleted(img.src);", On 2012/09/03 09:01:04, Joao da Silva wrote: > else > domAutomationController.send(false); No. If !img, then setupCurrentScreenshot is yet to be called, so do nothing. http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:273: EXPECT_TRUE(result == enabled); On 2012/09/03 09:01:04, Joao da Silva wrote: > EXPECT_EQ Done. http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:284: ash::TAKE_SCREENSHOT, ui::Accelerator()); On 2012/09/03 09:01:04, Joao da Silva wrote: > indent Done.
lgtm! http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... File chrome/browser/policy/policy_browsertest.cc (right): http://codereview.chromium.org/10784009/diff/71001/chrome/browser/policy/poli... chrome/browser/policy/policy_browsertest.cc:271: L" btest_initCompleted(img.src);", On 2012/09/03 11:31:21, qfel wrote: > On 2012/09/03 09:01:04, Joao da Silva wrote: > > else > > domAutomationController.send(false); > > No. If !img, then setupCurrentScreenshot is yet to be called, so do nothing. Right. The comment makes that clear now :-)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qfel@google.com/10784009/63004
Failed to apply patch for chrome/browser/policy/policy_browsertest.cc: While running patch -p1 --forward --force; patching file chrome/browser/policy/policy_browsertest.cc Hunk #1 FAILED at 5. Hunk #2 succeeded at 45 (offset -2 lines). Hunk #3 succeeded at 60 (offset -2 lines). Hunk #4 succeeded at 150 (offset -15 lines). Hunk #5 succeeded at 182 (offset -15 lines). Hunk #6 succeeded at 559 (offset -19 lines). 1 out of 6 hunks FAILED -- saving rejects to file chrome/browser/policy/policy_browsertest.cc.rej
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qfel@google.com/10784009/68012
Try job failure for 10784009-68012 (retry) on linux_chromeos for step "runhooks". It's a second try, previously, step "browser_tests" failed. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qfel@google.com/10784009/68012
Try job failure for 10784009-68012 on win for step "update". http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number... Step "update" is always a major failure. Look at the try server FAQ for more details.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qfel@google.com/10784009/68012
Change committed as 154714 |
