Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
179783003:
Create ScreenOrientationListener and make it SDK-dependant. (Closed)
|
Created:
6 years, 10 months ago by mlamouri (slow - plz ping) Modified:
3 years, 3 months ago CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, timvolodine Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
DescriptionCreate ScreenOrientationListener and make it SDK-dependant.
This CL is adding a ScreenOrientationListener abstract class with one
implementation that is based on the current mechanism used for listening
to screen orientation changes. The other implementation is based on
DisplayListener but this API is only available from API Level 17.
BUG=342714, 346696
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255750
Patch Set 1 #
Total comments: 10
Patch Set 2 : review comments #Patch Set 3 : #
Total comments: 12
Patch Set 4 : review comments #
Total comments: 12
Patch Set 5 : #
Total comments: 3
Patch Set 6 : #Patch Set 7 : #
Total comments: 2
Patch Set 8 : #Patch Set 9 : 500 response #
Total comments: 10
Patch Set 10 : #Patch Set 11 : #
Messages
Total messages: 49 (0 generated)
I use a static instead of ScreenOrientationListener but I could also make the interface a singleton. Let me know what you prefer.
On 2014/02/25 15:27:33, Mounir Lamouri wrote: > I use a static instead of ScreenOrientationListener but I could also make the > interface a singleton. Let me know what you prefer. I actually switched to a singleton approach in the next CL (it made more sense wrt the overall design): https://codereview.chromium.org/177123007/
nice! I have some small comments and suggestions, and most important, I think we can remove the extra interface definitions and roll that up within the implementation class... wdyt? https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationConfigurationListener.java (right): https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationConfigurationListener.java:32: private Context mAppContext = null; nit: use ApplicationStatus.getApplicationContext() from base: https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/... https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationConfigurationListener.java:34: private Set<ScreenOrientationObserver> mObservers = new HashSet<ScreenOrientationObserver>(); nit: use ObserverList from: https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/... https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationConfigurationListener.java:41: //// ScreenOrientationListener //// nit: I'm not sure we have this in java.. in c++, we'd normally have: // ScreenOrientationListener implementation: (ditto for 118 below). https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationConfigurationListener.java:61: observer.onScreenOrientationChanged(mOrientation); is it expecting in this callstack? should we yield, i.e., post a message so the screen orientation will be sent from a non-reentrant stack? https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java (right): https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:15: public interface ScreenOrientationListener { hmm... since the implementation class is in the same package as the client class, this interface is not hiding much.... also, I'm not sure an interface named "Listener" should expose "add/remove" observer.. is this indirection necessary at this level, or could we bundle this up with ScreenOrientationConfigurationListener itself? if we really need it, how about ScreenOrientationObserverList, or ...Manager, or something like that? also, since the inner interface is always fully qualified, could do with just being "Observer"
Note also that I am writing a set of tests for this. It will be in a separate CL if you do not mind. https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationConfigurationListener.java (right): https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationConfigurationListener.java:32: private Context mAppContext = null; On 2014/02/27 09:51:14, bulach wrote: > nit: use ApplicationStatus.getApplicationContext() from base: > > https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/... Done. Would that be safe to set it in ctor? https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationConfigurationListener.java:34: private Set<ScreenOrientationObserver> mObservers = new HashSet<ScreenOrientationObserver>(); On 2014/02/27 09:51:14, bulach wrote: > nit: use ObserverList from: > https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/... Ok. Using it but I will file a bug to get addObserver and removeObserver to return a boolean to tell whether the observer was actually added/removed. I would make the calling code more robust. https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationConfigurationListener.java:41: //// ScreenOrientationListener //// On 2014/02/27 09:51:14, bulach wrote: > nit: I'm not sure we have this in java.. in c++, we'd normally have: > > // ScreenOrientationListener implementation: > > (ditto for 118 below). Done. https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationConfigurationListener.java:61: observer.onScreenOrientationChanged(mOrientation); On 2014/02/27 09:51:14, bulach wrote: > is it expecting in this callstack? should we yield, i.e., post a message so the > screen orientation will be sent from a non-reentrant stack? Done. https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java (right): https://codereview.chromium.org/179783003/diff/1/content/public/android/java/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:15: public interface ScreenOrientationListener { On 2014/02/27 09:51:14, bulach wrote: > hmm... since the implementation class is in the same package as the client > class, this interface is not hiding much.... also, I'm not sure an interface > named "Listener" should expose "add/remove" observer.. is this indirection > necessary at this level, or could we bundle this up with > ScreenOrientationConfigurationListener itself? > > if we really need it, how about ScreenOrientationObserverList, or ...Manager, or > something like that? > also, since the inner interface is always fully qualified, could do with just > being "Observer" I've updated the patch to be the final design. Let's re-discuss that based on the new patch.
Tim, could you have a look at this cl?
https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java (right): https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:50: * This method is known to not correctly detects 180 degrees changes but it is detects->detect https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:143: * Returns a ScreenOrientationListeninr implementation based on the device's ScreenOrientationListener https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:173: mObserverCount++; Can it happen that the same observer is added twice? I understand this is covered by the TODO above, but slightly worried about mObserverCount going out of sync.. https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:196: * @param observer The observer that will no longer received notification. receive https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:204: // the last observer was removed, we should just stop listening. -1 sp after //, and maybe put this comment inside the if. https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:239: while (iter.hasNext()) { alternatively you could just do for(ScreenOrientationObserver : ...) which is shorter
https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java (right): https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:50: * This method is known to not correctly detects 180 degrees changes but it is On 2014/02/28 14:46:49, timvolodine wrote: > detects->detect Done. https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:143: * Returns a ScreenOrientationListeninr implementation based on the device's On 2014/02/28 14:46:49, timvolodine wrote: > ScreenOrientationListener Done. https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:173: mObserverCount++; On 2014/02/28 14:46:49, timvolodine wrote: > Can it happen that the same observer is added twice? > I understand this is covered by the TODO above, but slightly worried about > mObserverCount going out of sync.. Good point! I've added a check when adding and removing an observer. https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:196: * @param observer The observer that will no longer received notification. On 2014/02/28 14:46:49, timvolodine wrote: > receive Done. https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:204: // the last observer was removed, we should just stop listening. On 2014/02/28 14:46:49, timvolodine wrote: > -1 sp after //, and maybe put this comment inside the if. Done. https://codereview.chromium.org/179783003/diff/40001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:239: while (iter.hasNext()) { On 2014/02/28 14:46:49, timvolodine wrote: > alternatively you could just do for(ScreenOrientationObserver : ...) > which is shorter Good to know ;) Changed.
FYI, https://codereview.chromium.org/182623003/ implements the return value in ObserverList add/remove methods.
a few minor comments below: https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java (right): https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:122: protected Context mAppContext = null; No need to provide default null, also below for mObserverCount, mOrientation and sInstance. Will save a few javacode bytes. https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:169: // http://crbug.com/347557 does this still apply? there is a hasObserver() test below.. also you could probably simplify this once the other patch you have has landed. https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:201: // http://crbug.com/347557 Not sure if this applies here as well. If hasObserver is true shouldn't the removeObserver guarantee removal? https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:207: nit: maybe add assert that mObserverCount>=0? https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:216: * the first observer will be added. nit: "will be added" -> "is added". https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:223: * the last observer will be removed. Same as above, I think "is removed" sounds better than "will be removed" in this case.
PTAL https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java (right): https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:122: protected Context mAppContext = null; On 2014/03/05 15:24:57, timvolodine wrote: > No need to provide default null, also below for mObserverCount, mOrientation and > sInstance. Will save a few javacode bytes. Done. https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:169: // http://crbug.com/347557 On 2014/03/05 15:24:57, timvolodine wrote: > does this still apply? there is a hasObserver() test below.. also you could > probably simplify this once the other patch you have has landed. addObserver() is checking if the observer being added is there. Calling hasObserver() is wasting time because hasObserver() is O(n). We don't want to do that multiple times. Anyway, this is being removed in another CL. https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:201: // http://crbug.com/347557 On 2014/03/05 15:24:57, timvolodine wrote: > Not sure if this applies here as well. If hasObserver is true shouldn't the > removeObserver guarantee removal? Same as above. https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:207: On 2014/03/05 15:24:57, timvolodine wrote: > nit: maybe add assert that mObserverCount>=0? Done. https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:216: * the first observer will be added. On 2014/03/05 15:24:57, timvolodine wrote: > nit: "will be added" -> "is added". Done. https://codereview.chromium.org/179783003/diff/60001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:223: * the last observer will be removed. On 2014/03/05 15:24:57, timvolodine wrote: > Same as above, I think "is removed" sounds better than "will be removed" in this > case. Done.
lgtm lgtm % nits and assuming you will fix the addObserver issue at some point ;) https://codereview.chromium.org/179783003/diff/80001/content/public/android/j... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java (right): https://codereview.chromium.org/179783003/diff/80001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:131: private int mObserverCount = 0; no need for default https://codereview.chromium.org/179783003/diff/80001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:136: private int mOrientation = 0; no need for default https://codereview.chromium.org/179783003/diff/80001/content/public/android/j... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:138: private static ScreenOrientationListener sInstance = null; no need for default
good stuff! a few suggestions below, hopefully it'll further clarify the ScreenOrientation implementation. right now it has two roles: it manages the observer list, and also provides some facilities to help with the actual implementation. hopefully we can split them in two and make it clearer. please let me know your thoughts: https://codereview.chromium.org/179783003/diff/120001/content/public/android/... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java (right): https://codereview.chromium.org/179783003/diff/120001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:51: private static class ScreenOrientationConfigurationListener hmm, this seems a strange pattern, i.e., the inner class extending the outer class privately... here's a suggestion: - make the outer class concrete, exposing only the getInstance(), add/removeObsever methods.. not sure how to name it, probably "ScreenOrientationObserverList" or something? - make an inner base class with the shared methods (probably non-static, so that it can call up the outer class...). - then keep these two classes derived from that inner class. the main goals being: 1) the public class exposes just the necessary methods to the outer world. 2) the inner classes hide the implementation details, and don't derive from something that is externally visible. https://codereview.chromium.org/179783003/diff/120001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:221: protected abstract void startListening(); as above, I think these abstract / protected methods should be hidden in internal classes, and not exposed here.
bulach@, this new patch should take into account your comments. PTAL.
lgtm, thanks for your patience :) just small nits, feel free to go once addressed: https://codereview.chromium.org/179783003/diff/160001/content/public/android/... File content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java (right): https://codereview.chromium.org/179783003/diff/160001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:3121: public void onScreenOrientationChanged(int orientation) { nit: perhaps just rename sendOrientationChangeEvent to "onScreenOrientationChanged" and remove this one here? https://codereview.chromium.org/179783003/diff/160001/content/public/android/... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java (right): https://codereview.chromium.org/179783003/diff/160001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:141: protected Context mAppContext; nit: private. but more than that, ApplicationStatus.getApplicationContext() is relatively cheap.. it's better to just use that instead of caching it here (holding a strong reference to it can prevent the GC from collecting it, which is particularly tricky on a singleton like this). https://codereview.chromium.org/179783003/diff/160001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:144: private ObserverList<ScreenOrientationObserver> mObservers = nit: final https://codereview.chromium.org/179783003/diff/160001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:166: if (sInstance == null) { nit: ThreadUtils.assertOnUiThread(); https://codereview.chromium.org/179783003/diff/160001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:175: ? new ScreenOrientationDisplayListener() nit: I think ? and : operators go to the end of the previous line
https://codereview.chromium.org/179783003/diff/160001/content/public/android/... File content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java (right): https://codereview.chromium.org/179783003/diff/160001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:3121: public void onScreenOrientationChanged(int orientation) { On 2014/03/06 10:31:18, bulach wrote: > nit: perhaps just rename sendOrientationChangeEvent to > "onScreenOrientationChanged" and remove this one here? I kept it on purpose to use it for tests actually. I would like to have integration tests that do not require to actually have the device moving and it will be an entry point for them. Hope it makes sense. https://codereview.chromium.org/179783003/diff/160001/content/public/android/... File content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java (right): https://codereview.chromium.org/179783003/diff/160001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:141: protected Context mAppContext; On 2014/03/06 10:31:18, bulach wrote: > nit: private. but more than that, ApplicationStatus.getApplicationContext() is > relatively cheap.. it's better to just use that instead of caching it here > (holding a strong reference to it can prevent the GC from collecting it, which > is particularly tricky on a singleton like this). Done. https://codereview.chromium.org/179783003/diff/160001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:144: private ObserverList<ScreenOrientationObserver> mObservers = On 2014/03/06 10:31:18, bulach wrote: > nit: final Done. https://codereview.chromium.org/179783003/diff/160001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:166: if (sInstance == null) { On 2014/03/06 10:31:18, bulach wrote: > nit: ThreadUtils.assertOnUiThread(); Done. https://codereview.chromium.org/179783003/diff/160001/content/public/android/... content/public/android/java/src/org/chromium/content/browser/ScreenOrientationListener.java:175: ? new ScreenOrientationDisplayListener() On 2014/03/06 10:31:18, bulach wrote: > nit: I think ? and : operators go to the end of the previous line I think it makes it harder to read but if that's the coding style, I will follow it...
The CQ bit was checked by mlamouri@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/179783003/180001
The CQ bit was unchecked by commit-bot@chromium.org
Retried try job too often on win_rel for step(s) base_unittests, browser_tests, interactive_ui_tests, net_unittests, unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
The CQ bit was checked by mlamouri@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/179783003/180001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel
The CQ bit was checked by mlamouri@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/179783003/180001
The CQ bit was unchecked by commit-bot@chromium.org
Retried try job too often on win_rel for step(s) net_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
The CQ bit was checked by mlamouri@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/179783003/180001
The CQ bit was unchecked by commit-bot@chromium.org
Retried try job too often on win_rel for step(s) unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
The CQ bit was checked by mlamouri@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/179783003/180001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/179783003/180001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/179783003/180001
The CQ bit was unchecked by commit-bot@chromium.org
The commit queue went berserk retrying too often for a seemingly flaky test on builder android_dbg: http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
bulach@, I did some changes in the patch to make it work in Android WebView: it's no longer using ApplicationStatus but using a Context passed in addObserver with asserts checking that the Application Context is always the same as the one being used. I also forgot to apply a change before: ContentViewCore.destroy() removes |this| from the observers. PTAL.
lgtm, thanks for your patience!
The CQ bit was checked by mlamouri@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/179783003/200001
The CQ bit was unchecked by commit-bot@chromium.org
Retried try job too often on win_rel for step(s) unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
The CQ bit was checked by mlamouri@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/179783003/200001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/179783003/200001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/179783003/200001
Message was sent while issue was closed.
Change committed as 255750 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
