Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
10790116:
Have the VariationsService attempt to fetch the seed when an update is ready. (Closed)
|
Created:
8 years, 5 months ago by SteveT Modified:
8 years, 4 months ago CC:
chromium-reviews, MAD, Ilya Sherman, jar (doing other things) Base URL:
http://git.chromium.org/chromium/src.git@master Visibility:
Public. |
DescriptionHave the VariationsService attempt to fetch the seed when an update is ready.
Includes a unit test to exercise this functionality.
BUG=138384
TEST=Ensure that Chrome makes an HTTP request to the Variations server when it receives an auto update.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150949
Patch Set 1 #Patch Set 2 : Reset timer #
Total comments: 17
Patch Set 3 : asvit nits #Patch Set 4 : Privatization (is ruining our healthcare) #
Total comments: 4
Patch Set 5 : Unit test #
Total comments: 14
Patch Set 6 : asvit nit 2 #
Total comments: 4
Patch Set 7 : missing ctor init #Patch Set 8 : moved comment #Patch Set 9 : merge to tot #
Total comments: 19
Patch Set 10 : Addressed isherman's comments #
Total comments: 2
Patch Set 11 : privatize Observe #Patch Set 12 : merge to tot #
Messages
Total messages: 25 (0 generated)
PTAL.
Would it be possible to add a test for this by making FetchVariationsSeed() virtual? http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.cc (right): http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.cc:200: void VariationsService::Observe(int type, Please define this in the same order as functions are declared in the header file. http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.cc:202: const content::NotificationDetails& details) { Is this guaranteed to be called on the UI thread? http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.cc:207: timer_.Reset(); Nit: Add a comment explaining timer resetting. http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.h (right): http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:60: virtual void Observe(int type, Does this need to be public? http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:143: content::NotificationRegistrar registrar_; Nit: Add a comment.
To be clear on your suggestion... You're suggesting I make FetchVariationsSeed virtual so I can override it in a test class, so I can track if it is called if we pass NOTIFICATION_UPGRADE_RECOMMENDED to the VariationsService? Also, I need to manually test this somehow, and I'm not sure how to "force an AU". I'll have to talk to QA about this... http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.cc (right): http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.cc:200: void VariationsService::Observe(int type, Uh... this is already in the correct place? http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.cc:202: const content::NotificationDetails& details) { I believe it is, though I'll have to check to make sure. I've added a DCHECK to assert this. http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.cc:207: timer_.Reset(); On 2012/07/23 14:42:43, Alexei Svitkine wrote: > Nit: Add a comment explaining timer resetting. Done. http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.h (right): http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:60: virtual void Observe(int type, I guess we can make this private and friend it if we test with it. http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:143: content::NotificationRegistrar registrar_; On 2012/07/23 14:42:43, Alexei Svitkine wrote: > Nit: Add a comment. Done.
On Mon, Jul 23, 2012 at 11:24 AM, <stevet@chromium.org> wrote: > To be clear on your suggestion... You're suggesting I make > FetchVariationsSeed > virtual so I can override it in a test class, so I can track if it is > called if > we pass NOTIFICATION_UPGRADE_**RECOMMENDED to the VariationsService? > Yes, that's what I meant.
http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.cc (right): http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.cc:200: void VariationsService::Observe(int type, On 2012/07/23 15:24:31, SteveT wrote: > Uh... this is already in the correct place? Oops, you're right - I must have misread the order. My bad. http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.h (right): http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:51: void FetchVariationsSeed(); I think this should be private too. Sorry that I didn't catch this earlier. http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:54: virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE; This should be private too, I think. http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:60: virtual void Observe(int type, On 2012/07/23 15:24:31, SteveT wrote: > I guess we can make this private and friend it if we test with it. I think that makes sense to do. The existing tests are already friends. We don't need to expose this to the outside world.
Okay, moved the newly private methods around. I will add a unit test and ping this again when ready... I also need to figure out the manual test, too. http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.h (right): http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:51: void FetchVariationsSeed(); On 2012/07/23 15:32:13, Alexei Svitkine wrote: > I think this should be private too. Sorry that I didn't catch this earlier. Done. http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:54: virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE; On 2012/07/23 15:32:13, Alexei Svitkine wrote: > This should be private too, I think. Done. http://codereview.chromium.org/10790116/diff/8001/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:60: virtual void Observe(int type, SGTM. Will add the FRIEND stuff when I add the test.
Added a unit test. Note that I had to change the Observe method a bit to only reset the timer if it is currently running.
http://codereview.chromium.org/10790116/diff/1004/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.h (right): http://codereview.chromium.org/10790116/diff/1004/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:71: void FetchVariationsSeed(); Nit: Move this below OnURLFetchComplete() so that the virtual implementations are together. http://codereview.chromium.org/10790116/diff/1004/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:73: // net::URLFetcherDelegate implementation: Nit: Change this comment to the same style as the one for Observe(). http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.cc (right): http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.cc:178: DCHECK(type == chrome::NOTIFICATION_UPGRADE_RECOMMENDED); Nit: Add a blank line after this. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.h (right): http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:75: // net::URLFetcherDelegate implementation: Nit: Change this comment to have the same style as the one for Observe. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service_unittest.cc (right): http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:22: class TestVariationsService : public VariationsService { Add a short comment. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:25: virtual ~TestVariationsService() {} Nit: Blank line after this. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:28: void SimulateUpgradeAvailable() { Add a comment. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:39: }; DISALLOW_COPY_AND_ASSIGN? http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:42: fetch_attempted_ = true; Nit: Inline.
http://codereview.chromium.org/10790116/diff/1004/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.h (right): http://codereview.chromium.org/10790116/diff/1004/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:71: void FetchVariationsSeed(); On 2012/07/23 20:26:05, Alexei Svitkine wrote: > Nit: Move this below OnURLFetchComplete() so that the virtual implementations > are together. Oops, this comment was from earlier that I hadn't sent out. Ignore it as its no longer relevant. http://codereview.chromium.org/10790116/diff/1004/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:73: // net::URLFetcherDelegate implementation: On 2012/07/23 20:26:05, Alexei Svitkine wrote: > Nit: Change this comment to the same style as the one for Observe(). Oops, this comment got duplicated since I had it saved from earlier.
Thanks for the look. Over to you. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.cc (right): http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.cc:178: DCHECK(type == chrome::NOTIFICATION_UPGRADE_RECOMMENDED); On 2012/07/23 20:26:05, Alexei Svitkine wrote: > Nit: Add a blank line after this. Done. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service.h (right): http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service.h:75: // net::URLFetcherDelegate implementation: On 2012/07/23 20:26:05, Alexei Svitkine wrote: > Nit: Change this comment to have the same style as the one for Observe. Done. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service_unittest.cc (right): http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:22: class TestVariationsService : public VariationsService { On 2012/07/23 20:26:05, Alexei Svitkine wrote: > Add a short comment. Done. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:25: virtual ~TestVariationsService() {} On 2012/07/23 20:26:05, Alexei Svitkine wrote: > Nit: Blank line after this. Done. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:28: void SimulateUpgradeAvailable() { On 2012/07/23 20:26:05, Alexei Svitkine wrote: > Add a comment. Done. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:39: }; On 2012/07/23 20:26:05, Alexei Svitkine wrote: > DISALLOW_COPY_AND_ASSIGN? Done. http://codereview.chromium.org/10790116/diff/4005/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:42: fetch_attempted_ = true; On 2012/07/23 20:26:05, Alexei Svitkine wrote: > Nit: Inline. Done.
Almost there! http://codereview.chromium.org/10790116/diff/7006/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service_unittest.cc (right): http://codereview.chromium.org/10790116/diff/7006/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:31: // Simulate that an auto-update is ready by sending the appropriate Nit: Comment should be above the method. http://codereview.chromium.org/10790116/diff/7006/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:45: bool fetch_attempted_; Please initialize |fetch_attempted_| in the constructor.
Fixed ALL the things! http://codereview.chromium.org/10790116/diff/7006/chrome/browser/metrics/vari... File chrome/browser/metrics/variations_service_unittest.cc (right): http://codereview.chromium.org/10790116/diff/7006/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:31: // Simulate that an auto-update is ready by sending the appropriate On 2012/07/24 14:44:49, Alexei Svitkine wrote: > Nit: Comment should be above the method. Done. http://codereview.chromium.org/10790116/diff/7006/chrome/browser/metrics/vari... chrome/browser/metrics/variations_service_unittest.cc:45: bool fetch_attempted_; Yeah this caused a test failure earlier :( Done.
LGTM
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevet@chromium.org/10790116/15001
Presubmit check for 10790116-15001 failed and returned exit status 1.
Running presubmit commit checks ...
** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
chrome/browser/metrics
Hey Ilya - mind taking a look for OWNERS?
Following this patch, I am going to move the variations stuff to a variations/ directory as you suggested and set a new OWNERS file.
LGTM once you switch from NotificationService::AllSources() to listening to notifications from a specific source. https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... File chrome/browser/metrics/variations_service.cc (right): https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... chrome/browser/metrics/variations_service.cc:106: content::NotificationService::AllSources()); NotificationService::AllSources() is generally discouraged. Do you know what the source for this notification is? https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... chrome/browser/metrics/variations_service.cc:189: DCHECK(type == chrome::NOTIFICATION_UPGRADE_RECOMMENDED); nit: DCHECK_EQ(chrome::NOTIFICATION_UPGRADE_RECOMMENDED, type); https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... chrome/browser/metrics/variations_service.cc:191: // An upgrade is ready, so attempt to fetch the Variations seed in case there Out of curiosity, when do we capitalize jargon like "variations", "seed", etc.? (Obviously not a big deal, I'm just curious.) https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... File chrome/browser/metrics/variations_service_unittest.cc (right): https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... chrome/browser/metrics/variations_service_unittest.cc:36: content::NotificationService::NoDetails()); nit: This seems a little odd. Why not dispatch the notification in the usual way, and have coverage for being registered for the notification as well? https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... chrome/browser/metrics/variations_service_unittest.cc:40: virtual void FetchVariationsSeed() { nit: OVERRIDE https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... chrome/browser/metrics/variations_service_unittest.cc:77: &message_loop); nit: Why is this needed? If it's just to satisfy the DCHECK, perhaps it would be cleaner to have the VariationsService inherit from base::NonThreadSafe and replace the check with DCHECK(CalledOnValidThread())? https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... chrome/browser/metrics/variations_service_unittest.cc:79: ASSERT_FALSE(test_service.fetch_attempted()); nit: EXPECT_FALSE https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... chrome/browser/metrics/variations_service_unittest.cc:81: ASSERT_TRUE(test_service.fetch_attempted()); nit: EXPECT_TRUE
Some responses - let me know what you think. http://codereview.chromium.org/10790116/diff/15001/chrome/browser/metrics/var... File chrome/browser/metrics/variations_service.cc (right): http://codereview.chromium.org/10790116/diff/15001/chrome/browser/metrics/var... chrome/browser/metrics/variations_service.cc:106: content::NotificationService::AllSources()); This is an UpgradeDetector, which conveniently is a Singleton, so I'll call GetInstance here. Note that none of the other listeners of NOTIFICATION_UPGRADE_RECOMMENDED have a specific source... Do you think it'd be wise to (in a separate patch) change them all to listen to UpgradeDetector::GetInstance() as a source? http://codereview.chromium.org/10790116/diff/15001/chrome/browser/metrics/var... chrome/browser/metrics/variations_service.cc:189: DCHECK(type == chrome::NOTIFICATION_UPGRADE_RECOMMENDED); On 2012/08/07 20:31:51, Ilya Sherman wrote: > nit: DCHECK_EQ(chrome::NOTIFICATION_UPGRADE_RECOMMENDED, type); Done. http://codereview.chromium.org/10790116/diff/15001/chrome/browser/metrics/var... chrome/browser/metrics/variations_service.cc:191: // An upgrade is ready, so attempt to fetch the Variations seed in case there Hm, good question. I think in this file we're sort of using it as the name for the feature (as opposed to the internal code name), so that's why we capitalize it. This is not consistent, though, so I've capitalized "variations" across this file. Do push back if you think we're breaking some important style guide/grammar rule :) http://codereview.chromium.org/10790116/diff/15001/chrome/browser/metrics/var... File chrome/browser/metrics/variations_service_unittest.cc (right): http://codereview.chromium.org/10790116/diff/15001/chrome/browser/metrics/var... chrome/browser/metrics/variations_service_unittest.cc:36: content::NotificationService::NoDetails()); That makes way more sense, and yes, I do like the extra bit of coverage we get from that. Resolved this. Thanks for the suggestion! http://codereview.chromium.org/10790116/diff/15001/chrome/browser/metrics/var... chrome/browser/metrics/variations_service_unittest.cc:40: virtual void FetchVariationsSeed() { On 2012/08/07 20:31:51, Ilya Sherman wrote: > nit: OVERRIDE Done. http://codereview.chromium.org/10790116/diff/15001/chrome/browser/metrics/var... chrome/browser/metrics/variations_service_unittest.cc:77: &message_loop); Yes, this was just to satisfy the DCHECKs on BrowserThread::UI in variations_service.cc. I replaced those checks with NonThreadSafe and CalledOnValidThread, which I agree is cleaner. However, now that we've started calling into UpgradeDetector::GetInstance, we need to keep this TestBrowserThread around, as UpgradeDetector's init code requires it (or else _it_ DCHECKs). Does this seem fine to you? I've seen creating TestBrowserThreads for testing similar classes (that run on a specific Thread) as a common pattern. Is this something we'd like to move away from? http://codereview.chromium.org/10790116/diff/15001/chrome/browser/metrics/var... chrome/browser/metrics/variations_service_unittest.cc:79: ASSERT_FALSE(test_service.fetch_attempted()); On 2012/08/07 20:31:51, Ilya Sherman wrote: > nit: EXPECT_FALSE Done. http://codereview.chromium.org/10790116/diff/15001/chrome/browser/metrics/var... chrome/browser/metrics/variations_service_unittest.cc:81: ASSERT_TRUE(test_service.fetch_attempted()); On 2012/08/07 20:31:51, Ilya Sherman wrote: > nit: EXPECT_TRUE Done.
(Still LGTM) https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... File chrome/browser/metrics/variations_service.cc (right): https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... chrome/browser/metrics/variations_service.cc:106: content::NotificationService::AllSources()); On 2012/08/08 18:11:39, SteveT wrote: > This is an UpgradeDetector, which conveniently is a Singleton, so I'll call > GetInstance here. Lovely :) > Note that none of the other listeners of NOTIFICATION_UPGRADE_RECOMMENDED have a > specific source... Do you think it'd be wise to (in a separate patch) change > them all to listen to UpgradeDetector::GetInstance() as a source? Yes, that would be great -- thanks for offering to clean that up! :) https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... chrome/browser/metrics/variations_service.cc:191: // An upgrade is ready, so attempt to fetch the Variations seed in case there On 2012/08/08 18:11:39, SteveT wrote: > Hm, good question. I think in this file we're sort of using it as the name for > the feature (as opposed to the internal code name), so that's why we capitalize > it. This is not consistent, though, so I've capitalized "variations" across this > file. > > Do push back if you think we're breaking some important style guide/grammar rule > :) Nope, was just curious. Thanks :) https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... File chrome/browser/metrics/variations_service_unittest.cc (right): https://chromiumcodereview.appspot.com/10790116/diff/15001/chrome/browser/met... chrome/browser/metrics/variations_service_unittest.cc:77: &message_loop); On 2012/08/08 18:11:39, SteveT wrote: > Yes, this was just to satisfy the DCHECKs on BrowserThread::UI in > variations_service.cc. I replaced those checks with NonThreadSafe and > CalledOnValidThread, which I agree is cleaner. > > However, now that we've started calling into UpgradeDetector::GetInstance, we > need to keep this TestBrowserThread around, as UpgradeDetector's init code > requires it (or else _it_ DCHECKs). Does this seem fine to you? I've seen > creating TestBrowserThreads for testing similar classes (that run on a specific > Thread) as a common pattern. Is this something we'd like to move away from? It's an ok pattern; it's just often overkill and adds a bit of boilerplate noise to the test implementation, so I prefer to remove it when it's not really needed. If it's needed for the UpgradeDetector code in this case, then that's fine. https://chromiumcodereview.appspot.com/10790116/diff/23001/chrome/browser/met... File chrome/browser/metrics/variations_service.h (right): https://chromiumcodereview.appspot.com/10790116/diff/23001/chrome/browser/met... chrome/browser/metrics/variations_service.h:64: const content::NotificationDetails& details) OVERRIDE; nit: Can this be private now that the test code is not calling it directly?
Thanks for the review. Will commit shortly... https://chromiumcodereview.appspot.com/10790116/diff/23001/chrome/browser/met... File chrome/browser/metrics/variations_service.h (right): https://chromiumcodereview.appspot.com/10790116/diff/23001/chrome/browser/met... chrome/browser/metrics/variations_service.h:64: const content::NotificationDetails& details) OVERRIDE; On 2012/08/08 21:33:47, Ilya Sherman wrote: > nit: Can this be private now that the test code is not calling it directly? Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevet@chromium.org/10790116/27001
Try job failure for 10790116-27001 (retry) on mac_rel for step "browser_tests". It's a second try, previously, step "browser_tests" failed. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevet@chromium.org/10790116/27001
Change committed as 150949 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
