Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
265843009:
Restored an option to enable enhanced bookmarks experiment from Finch (Closed)
|
Created:
6 years, 7 months ago by yefimt Modified:
6 years, 7 months ago CC:
chromium-reviews, tfarina, browser-components-watch_chromium.org, extensions-reviews_chromium.org, chromium-apps-reviews_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionRestored an option to enable enhanced bookmarks experiment from Finch. It was removed when we switched to Chrome sync enabled experiment. Now we need both Chrome sync experiment for logged in users and Finch for the rest.
BUG=321393
TBR=rlarocque
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270288
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270589
Patch Set 1 : #
Total comments: 10
Patch Set 2 : #Patch Set 3 : #Patch Set 4 : #
Total comments: 4
Patch Set 5 : #Patch Set 6 : #
Total comments: 8
Patch Set 7 : #
Total comments: 8
Patch Set 8 : #Patch Set 9 : #Patch Set 10 : rebased #
Total comments: 1
Patch Set 11 : #Patch Set 12 : rebased #Patch Set 13 : fixed unit test #Patch Set 14 : #Patch Set 15 : one more unit test fix #
Messages
Total messages: 52 (0 generated)
i can't review the bookmarks code. https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... File chrome/browser/extensions/external_component_loader.cc (right): https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... chrome/browser/extensions/external_component_loader.cc:31: kFinchBookmarksExperimentEnumSize what is the difference between this and the similar enum in bookmarks? https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... chrome/browser/extensions/external_component_loader.cc:38: return !signin->GetAuthenticatedUsername().empty(); could phrase these last 3 lines as "return signin && !signin->GetAuthenticatedUsername().empty()" https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... chrome/browser/extensions/external_component_loader.cc:87: profile_->GetPrefs()->SetInteger( why do you need to store whether the command line flag has opted out? if the user gives the command line flag, it shouldn't work. if they don't, it maybe-should. I.e. persisting a command line flag choice doesn't seem the right idiom for command line flags. https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... chrome/browser/extensions/external_component_loader.cc:116: extension_urls::GetWebstoreUpdateUrl().spec()); I could say the same for finch actually. does prefs need to be involved at all?
https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... File chrome/browser/extensions/external_component_loader.cc (right): https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... chrome/browser/extensions/external_component_loader.cc:31: kFinchBookmarksExperimentEnumSize This one is used for UMA_HISTOGRAM_ENUMERATION only. I want it to have size after values we report and have only values relevant to reported state. On 2014/05/05 20:19:11, kalman wrote: > what is the difference between this and the similar enum in bookmarks? https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... chrome/browser/extensions/external_component_loader.cc:38: return !signin->GetAuthenticatedUsername().empty(); On 2014/05/05 20:19:11, kalman wrote: > could phrase these last 3 lines as "return signin && > !signin->GetAuthenticatedUsername().empty()" Done. https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... chrome/browser/extensions/external_component_loader.cc:87: profile_->GetPrefs()->SetInteger( In case of finch it not really necessary. Just it was convenient to unify it with the rest of the code when I call UpdateBookmarksExperiment() below only if state was changed. On 2014/05/05 20:19:11, kalman wrote: > why do you need to store whether the command line flag has opted out? if the > user gives the command line flag, it shouldn't work. if they don't, it > maybe-should. I.e. persisting a command line flag choice doesn't seem the right > idiom for command line flags. https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... chrome/browser/extensions/external_component_loader.cc:116: extension_urls::GetWebstoreUpdateUrl().spec()); If you are talking about statement above, |prefs_| is not preference, it is dictionary with list of extensions to be loaded from a base class On 2014/05/05 20:19:11, kalman wrote: > I could say the same for finch actually. does prefs need to be involved at all?
https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... File chrome/browser/extensions/external_component_loader.cc (right): https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... chrome/browser/extensions/external_component_loader.cc:31: kFinchBookmarksExperimentEnumSize On 2014/05/05 20:54:24, yefimt wrote: > This one is used for UMA_HISTOGRAM_ENUMERATION only. > I want it to have size after values we report and have only values relevant to > reported state. > > On 2014/05/05 20:19:11, kalman wrote: > > what is the difference between this and the similar enum in bookmarks? > Having it separately has the disadvantage of a repeated enum, and the enum being private means it's harder to refer to meaningfully from histograms.xml*. I'd just use the bookmarks enum for UMA. people reading the UMA can intepret the data themselves. [*] you need to update this. https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... chrome/browser/extensions/external_component_loader.cc:116: extension_urls::GetWebstoreUpdateUrl().spec()); On 2014/05/05 20:54:24, yefimt wrote: > If you are talking about statement above, |prefs_| is not preference, it is > dictionary with list of extensions to be loaded from a base class > > On 2014/05/05 20:19:11, kalman wrote: > > I could say the same for finch actually. does prefs need to be involved at > all? > having prefs_ and GetPrefs() as totally different things is very confusing. but no I just meant modifying GetPrefs() at all. it shouldn't be necessary to mirror the command line nor finch state in preferences.
On 2014/05/05 21:25:18, kalman wrote: > https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... > File chrome/browser/extensions/external_component_loader.cc (right): > > https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... > chrome/browser/extensions/external_component_loader.cc:31: > kFinchBookmarksExperimentEnumSize > On 2014/05/05 20:54:24, yefimt wrote: > > This one is used for UMA_HISTOGRAM_ENUMERATION only. > > I want it to have size after values we report and have only values relevant to > > reported state. > > > > On 2014/05/05 20:19:11, kalman wrote: > > > what is the difference between this and the similar enum in bookmarks? > > > > Having it separately has the disadvantage of a repeated enum, and the enum being > private means it's harder to refer to meaningfully from histograms.xml*. > > I'd just use the bookmarks enum for UMA. people reading the UMA can intepret the > data themselves. > > > [*] you need to update this. > > https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... > chrome/browser/extensions/external_component_loader.cc:116: > extension_urls::GetWebstoreUpdateUrl().spec()); > On 2014/05/05 20:54:24, yefimt wrote: > > If you are talking about statement above, |prefs_| is not preference, it is > > dictionary with list of extensions to be loaded from a base class > > > > On 2014/05/05 20:19:11, kalman wrote: > > > I could say the same for finch actually. does prefs need to be involved at > > all? > > > > having prefs_ and GetPrefs() as totally different things is very confusing. > > but no I just meant modifying GetPrefs() at all. it shouldn't be necessary to > mirror the command line nor finch state in preferences. I agree that when experiment is enabled from Finch then preferences are not needed. But when experiment is enabled from chrome sync, then there are two places in code, and in time, when decision takes place. One is here and another in ProfileSyncService::OnExperimentsChanged(). User preferences is a best way to pass state from one location to another. As sync experiment is per user and each of locations could change state before another.
On 2014/05/05 21:44:11, yefimt wrote: > On 2014/05/05 21:25:18, kalman wrote: > > > https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... > > File chrome/browser/extensions/external_component_loader.cc (right): > > > > > https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... > > chrome/browser/extensions/external_component_loader.cc:31: > > kFinchBookmarksExperimentEnumSize > > On 2014/05/05 20:54:24, yefimt wrote: > > > This one is used for UMA_HISTOGRAM_ENUMERATION only. > > > I want it to have size after values we report and have only values relevant > to > > > reported state. > > > > > > On 2014/05/05 20:19:11, kalman wrote: > > > > what is the difference between this and the similar enum in bookmarks? > > > > > > > Having it separately has the disadvantage of a repeated enum, and the enum > being > > private means it's harder to refer to meaningfully from histograms.xml*. > > > > I'd just use the bookmarks enum for UMA. people reading the UMA can intepret > the > > data themselves. > > > > > > [*] you need to update this. Done > > > > > https://codereview.chromium.org/265843009/diff/60001/chrome/browser/extension... > > chrome/browser/extensions/external_component_loader.cc:116: > > extension_urls::GetWebstoreUpdateUrl().spec()); > > On 2014/05/05 20:54:24, yefimt wrote: > > > If you are talking about statement above, |prefs_| is not preference, it is > > > dictionary with list of extensions to be loaded from a base class > > > > > > On 2014/05/05 20:19:11, kalman wrote: > > > > I could say the same for finch actually. does prefs need to be involved at > > > all? > > > > > > > having prefs_ and GetPrefs() as totally different things is very confusing. > > > > but no I just meant modifying GetPrefs() at all. it shouldn't be necessary to > > mirror the command line nor finch state in preferences. > > I agree that when experiment is enabled from Finch then preferences are not > needed. > But when experiment is enabled from chrome sync, then there are two places in > code, and in time, when decision takes place. One is here and another in > ProfileSyncService::OnExperimentsChanged(). User preferences is a best way to > pass state from one location to another. As sync experiment is per user and each > of locations could change state before another.
I see what you mean. Using prefs as the model here is ... wrong. Better to define a class which has a method like EnhancedBookmarksExperimentEnabled() and checks the command line flag, finch, and prefs.
On 2014/05/05 21:47:14, kalman wrote: > I see what you mean. Using prefs as the model here is ... wrong. Better to > define a class which has a method like EnhancedBookmarksExperimentEnabled() and > checks the command line flag, finch, and prefs. Done
thanks. mostly what I meant. https://codereview.chromium.org/265843009/diff/140001/chrome/browser/bookmark... File chrome/browser/bookmarks/enhanced_bookmarks_features.cc (right): https://codereview.chromium.org/265843009/diff/140001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.cc:26: bool IsBookmarksExperimentEnabled(PrefService* user_prefs, I mean that there should be no UpdateBookmarksExperimentState, just IsBookmarksExperimentState, and that takes into account all of these factors like finch. There shouldn't be any need to write to prefs from here at all, only read. https://codereview.chromium.org/265843009/diff/140001/chrome/browser/bookmark... File chrome/browser/bookmarks/enhanced_bookmarks_features.h (right): https://codereview.chromium.org/265843009/diff/140001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.h:25: std::string* extension_id); run git cl format on this cl before submitting
https://codereview.chromium.org/265843009/diff/140001/chrome/browser/bookmark... File chrome/browser/bookmarks/enhanced_bookmarks_features.cc (right): https://codereview.chromium.org/265843009/diff/140001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.cc:26: bool IsBookmarksExperimentEnabled(PrefService* user_prefs, It cannot be just IsBookmarksExperimentState, because it does some changes, for example sets command line flags to force finch experiment. So it will be ide effecxt for IsXXX function On 2014/05/06 21:35:06, kalman wrote: > I mean that there should be no UpdateBookmarksExperimentState, just > IsBookmarksExperimentState, and that takes into account all of these factors > like finch. There shouldn't be any need to write to prefs from here at all, only > read. https://codereview.chromium.org/265843009/diff/140001/chrome/browser/bookmark... File chrome/browser/bookmarks/enhanced_bookmarks_features.h (right): https://codereview.chromium.org/265843009/diff/140001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.h:25: std::string* extension_id); On 2014/05/06 21:35:06, kalman wrote: > run git cl format on this cl before submitting Done.
i just don't understand why it needs to change prefs.
On 2014/05/06 22:12:21, kalman wrote: > i just don't understand why it needs to change prefs. done
extensions part lgtm. I didn't look too closely at the bookmarks stuff. https://codereview.chromium.org/265843009/diff/180001/chrome/browser/bookmark... File chrome/browser/bookmarks/enhanced_bookmarks_features.cc (right): https://codereview.chromium.org/265843009/diff/180001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.cc:34: } else if (bookmarks_experiment_state == kBookmarksExperimentEnabled) { no else after return https://codereview.chromium.org/265843009/diff/180001/chrome/browser/extensio... File chrome/browser/extensions/external_component_loader.cc (right): https://codereview.chromium.org/265843009/diff/180001/chrome/browser/extensio... chrome/browser/extensions/external_component_loader.cc:45: IsUserSignedin(profile_)); Just a Profile* would be a nicer dependency for this function. Most of these are implementation details, e.g. it can pull out g_browser_process itself. https://codereview.chromium.org/265843009/diff/180001/chrome/browser/extensio... chrome/browser/extensions/external_component_loader.cc:47: if (IsBookmarksExperimentEnabled(profile_->GetPrefs(), &ext_id) && GetBookmarksExperimentExtensionID would be a better name since it gets an extension ID. https://codereview.chromium.org/265843009/diff/180001/chrome/browser/extensio... chrome/browser/extensions/external_component_loader.cc:48: !ext_id.empty()) { This function should have a contract that a return value of "true" will fill in a valid extension ID. I.e. this should be a DCHECK(!ext_id.empty()).
+brettw for chrome/browser/bookmarks https://codereview.chromium.org/265843009/diff/180001/chrome/browser/bookmark... File chrome/browser/bookmarks/enhanced_bookmarks_features.cc (right): https://codereview.chromium.org/265843009/diff/180001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.cc:34: } else if (bookmarks_experiment_state == kBookmarksExperimentEnabled) { On 2014/05/07 21:16:39, kalman wrote: > no else after return Done. https://codereview.chromium.org/265843009/diff/180001/chrome/browser/extensio... File chrome/browser/extensions/external_component_loader.cc (right): https://codereview.chromium.org/265843009/diff/180001/chrome/browser/extensio... chrome/browser/extensions/external_component_loader.cc:45: IsUserSignedin(profile_)); Yes, unfortunately bookmarks are not allowed to depend on Profile On 2014/05/07 21:16:39, kalman wrote: > Just a Profile* would be a nicer dependency for this function. Most of these are > implementation details, e.g. it can pull out g_browser_process itself. https://codereview.chromium.org/265843009/diff/180001/chrome/browser/extensio... chrome/browser/extensions/external_component_loader.cc:47: if (IsBookmarksExperimentEnabled(profile_->GetPrefs(), &ext_id) && On 2014/05/07 21:16:39, kalman wrote: > GetBookmarksExperimentExtensionID would be a better name since it gets an > extension ID. Done. https://codereview.chromium.org/265843009/diff/180001/chrome/browser/extensio... chrome/browser/extensions/external_component_loader.cc:48: !ext_id.empty()) { On 2014/05/07 21:16:39, kalman wrote: > This function should have a contract that a return value of "true" will fill in > a valid extension ID. I.e. this should be a DCHECK(!ext_id.empty()). Done.
https://codereview.chromium.org/265843009/diff/200001/chrome/browser/bookmark... File chrome/browser/bookmarks/enhanced_bookmarks_features.h (right): https://codereview.chromium.org/265843009/diff/200001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.h:14: enum BookmarksExperimentState { Can you have a comment above here giving an overview of the experiment states and what they mean? It seems there are two similar but different experiment types that may mean different things. https://codereview.chromium.org/265843009/diff/200001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.h:24: bool GetBookmarksExperimentExtensionID(const PrefService* user_prefs, This header file is quite confusing. There are no comments on these two new functions. There is a function UpdateBookmarksExperimentState and UpdateBookmarksExperiment and I don't see how these are supposed to be different and why I might call one or the other. https://codereview.chromium.org/265843009/diff/200001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.h:36: // Returns true if enhanced bookmarks experiment is enabled. I think this should say mroe clearly how this is different than the Finch one. https://codereview.chromium.org/265843009/diff/200001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.h:40: bool IsEnhancedBookmarksExperimentEnabledFromFinch(); Does this need to be a public function or can it be in an anon namespace in the .cc file? It doesn't seem to be used elsewhere and its presence is confusing here. Can you do the same with any of the other functions?
https://codereview.chromium.org/265843009/diff/200001/chrome/browser/bookmark... File chrome/browser/bookmarks/enhanced_bookmarks_features.h (right): https://codereview.chromium.org/265843009/diff/200001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.h:14: enum BookmarksExperimentState { On 2014/05/08 20:12:04, brettw wrote: > Can you have a comment above here giving an overview of the experiment states > and what they mean? It seems there are two similar but different experiment > types that may mean different things. Done. https://codereview.chromium.org/265843009/diff/200001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.h:24: bool GetBookmarksExperimentExtensionID(const PrefService* user_prefs, On 2014/05/08 20:12:04, brettw wrote: > This header file is quite confusing. There are no comments on these two new > functions. There is a function UpdateBookmarksExperimentState and > UpdateBookmarksExperiment and I don't see how these are supposed to be different > and why I might call one or the other. Done. https://codereview.chromium.org/265843009/diff/200001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.h:36: // Returns true if enhanced bookmarks experiment is enabled. On 2014/05/08 20:12:04, brettw wrote: > I think this should say mroe clearly how this is different than the Finch one. Done. https://codereview.chromium.org/265843009/diff/200001/chrome/browser/bookmark... chrome/browser/bookmarks/enhanced_bookmarks_features.h:40: bool IsEnhancedBookmarksExperimentEnabledFromFinch(); On 2014/05/08 20:12:04, brettw wrote: > Does this need to be a public function or can it be in an anon namespace in the > .cc file? It doesn't seem to be used elsewhere and its presence is confusing > here. Can you do the same with any of the other functions? Done.
LGTM with function name change: UpdateBookmarksExperiment -> ForceFinchBoomarkExperimentIfNeeded
On 2014/05/09 19:21:49, brettw wrote: > LGTM with function name change: UpdateBookmarksExperiment -> > ForceFinchBoomarkExperimentIfNeeded +tbr=rlarocque for name change in profile_sync_service.cc
The CQ bit was checked by yefim@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yefim@chromium.org/265843009/260001
https://codereview.chromium.org/265843009/diff/260001/chrome/browser/sync/pro... File chrome/browser/sync/profile_sync_service.cc (right): https://codereview.chromium.org/265843009/diff/260001/chrome/browser/sync/pro... chrome/browser/sync/profile_sync_service.cc:1073: ForceFinchBoomarkExperimentIfNeeded(g_browser_process->local_state(), s/Boomark/Bookmark ?
Aside from the typo, LGTM.
The CQ bit was checked by yefim@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yefim@chromium.org/265843009/280001
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are real, and report flakes to chrome-troopers@google.com. The failing builders are: android_dbg_triggered_tests on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...) linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...) linux_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...) mac_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...) mac_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...) win_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...) win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...) win_chromium_x64_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...) linux_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
The CQ bit was checked by yefim@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yefim@chromium.org/265843009/280001
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are real, and report flakes to chrome-troopers@google.com. The failing builders are: linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...) linux_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...) mac_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...) win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...) win_chromium_x64_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
The CQ bit was checked by yefim@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yefim@chromium.org/265843009/300001
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are real, and report flakes to chrome-troopers@google.com. The failing builders are: linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...) linux_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...) mac_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...) win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...) win_chromium_x64_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
The CQ bit was checked by yefim@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yefim@chromium.org/265843009/320001
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are real, and report flakes to chrome-troopers@google.com. The failing builders are: linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
The CQ bit was checked by yefim@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yefim@chromium.org/265843009/340001
Message was sent while issue was closed.
Change committed as 270288
The CQ bit was checked by yefim@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yefim@chromium.org/265843009/360001
The CQ bit was unchecked by yefim@chromium.org
The CQ bit was checked by yefim@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yefim@chromium.org/265843009/360001
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are real, and report flakes to chrome-troopers@google.com. The failing builders are: ios_dbg_simulator on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...) ios_rel_device on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
Message was sent while issue was closed.
Change committed as 270589 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
