Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1249933004:
Add code to detect new added ReceiveError on Linux (Closed)
|
Created:
5 years, 5 months ago by juncai Modified:
5 years, 4 months ago Reviewers:
Reilly Grant (use Gerrit) CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd code to detect new added ReceiveError on Linux.
This patch added code to detect new added ReceiveError on
Linux.
BUG=414105
Committed: https://crrev.com/923ede74ebc57633583795a84b33e994f7dcd4fa
Cr-Commit-Position: refs/heads/master@{#341216}
Patch Set 1 : add code to detect new added ReceiveError on Linux #Patch Set 2 : added code to check if parity check is enabled #Patch Set 3 : updated code to remove break and parity error sequences, etc. #Patch Set 4 : updated code to handle byte sequence is split across multiple calls to read(), added unit tests code. #
Total comments: 4
Patch Set 5 : added more test code. #
Total comments: 8
Patch Set 6 : updated test code. #
Total comments: 10
Patch Set 7 : updated test code. #
Total comments: 36
Patch Set 8 : added more test code, updated some test code, added some DCHECKs #
Total comments: 2
Patch Set 9 : updated DCHECKs #
Total comments: 4
Patch Set 10 : updated comments, changed variable name. #
Messages
Total messages: 27 (3 generated)
juncai@chromium.org changed reviewers: + reillyg@chromium.org
Please review this patch.
Two things missing from here:
1) The parity error and break sequences aren't filtered out of the byte stream
sent to clients.
2) The sequence {'\377', '\377'} is not replaced with {'\377'}.
On 2015/07/23 20:35:13, reillyg wrote:
> Two things missing from here:
>
> 1) The parity error and break sequences aren't filtered out of the byte stream
> sent to clients.
> 2) The sequence {'\377', '\377'} is not replaced with {'\377'}.
Done.
Please review the change.
Please add some unit tests for this function. What happens if the byte sequence is split across two calls to read()?
On 2015/07/24 19:31:36, reillyg wrote: > Please add some unit tests for this function. What happens if the byte sequence > is split across two calls to read()? Done.
Please review the change.
These tests are pretty exhaustive but they don't include bytes received before or after the break/parity error so I'm not sure that that data is handled correctly. https://codereview.chromium.org/1249933004/diff/60001/device/serial/serial_io... File device/serial/serial_io_handler_posix.h (right): https://codereview.chromium.org/1249933004/diff/60001/device/serial/serial_io... device/serial/serial_io_handler_posix.h:15: enum ByteRead { These names still aren't very clear to me. Maybe something like: // Linux reports breaks and parity errors by inserting the sequence '\377\0x' into // the byte stream where 'x' is '\0' for a break and the corrupted byte for a // parity error. enum class ErrorDetectState { NO_ERROR, MARK_377_SEEN, MARK_0_SEEN }; https://codereview.chromium.org/1249933004/diff/60001/device/serial/serial_io... device/serial/serial_io_handler_posix.h:46: FRIEND_TEST_ALL_PREFIXES(SerialIoHandlerPosixTest, NoErrorReadOnce); You can avoid adding all of these friend declarations by putting the code that access private members of this class into a helper function that's a member of SerialIoHandlerPosixTest.
On 2015/07/28 22:06:03, reillyg wrote: > These tests are pretty exhaustive but they don't include bytes received before > or after the break/parity error so I'm not sure that that data is handled > correctly. > > https://codereview.chromium.org/1249933004/diff/60001/device/serial/serial_io... > File device/serial/serial_io_handler_posix.h (right): > > https://codereview.chromium.org/1249933004/diff/60001/device/serial/serial_io... > device/serial/serial_io_handler_posix.h:15: enum ByteRead { > These names still aren't very clear to me. Maybe something like: > > // Linux reports breaks and parity errors by inserting the sequence '\377\0x' > into > // the byte stream where 'x' is '\0' for a break and the corrupted byte for a > // parity error. > enum class ErrorDetectState { > NO_ERROR, > MARK_377_SEEN, > MARK_0_SEEN > }; > > https://codereview.chromium.org/1249933004/diff/60001/device/serial/serial_io... > device/serial/serial_io_handler_posix.h:46: > FRIEND_TEST_ALL_PREFIXES(SerialIoHandlerPosixTest, NoErrorReadOnce); > You can avoid adding all of these friend declarations by putting the code that > access private members of this class into a helper function that's a member of > SerialIoHandlerPosixTest. Done.
Please review the change. https://codereview.chromium.org/1249933004/diff/60001/device/serial/serial_io... File device/serial/serial_io_handler_posix.h (right): https://codereview.chromium.org/1249933004/diff/60001/device/serial/serial_io... device/serial/serial_io_handler_posix.h:15: enum ByteRead { On 2015/07/28 22:06:03, reillyg wrote: > These names still aren't very clear to me. Maybe something like: > > // Linux reports breaks and parity errors by inserting the sequence '\377\0x' > into > // the byte stream where 'x' is '\0' for a break and the corrupted byte for a > // parity error. > enum class ErrorDetectState { > NO_ERROR, > MARK_377_SEEN, > MARK_0_SEEN > }; Done. https://codereview.chromium.org/1249933004/diff/60001/device/serial/serial_io... device/serial/serial_io_handler_posix.h:46: FRIEND_TEST_ALL_PREFIXES(SerialIoHandlerPosixTest, NoErrorReadOnce); On 2015/07/28 22:06:03, reillyg wrote: > You can avoid adding all of these friend declarations by putting the code that > access private members of this class into a helper function that's a member of > SerialIoHandlerPosixTest. Done.
https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... File device/serial/serial_io_handler_posix.h (right): https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... device/serial/serial_io_handler_posix.h:18: enum ErrorDetectState { NO_ERROR, MARK_377_SEEN, MARK_0_SEEN }; This needs to be an enum class to use short names (NO_ERROR) instead of long names (ERROR_DETECT_STATE_NO_ERROR). https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... File device/serial/serial_io_handler_posix_unittest.cc (right): https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... device/serial/serial_io_handler_posix_unittest.cc:18: int call_CheckReceiveError(char* buffer, Don't mix underscores and camel-case in function names. Since there is a lot of repetition below I suggest building a common test function that simply takes an input state and expected output state. https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... device/serial/serial_io_handler_posix_unittest.cc:43: int get_num_chars_ignored() { No need to prefix getters with "get_", "num_chars_ignored()" is the correct function name. https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... device/serial/serial_io_handler_posix_unittest.cc:70: buffer[0] = 'a'; This would be more readable if you built the buffer with an array literal.
Please review the change. https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... File device/serial/serial_io_handler_posix.h (right): https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... device/serial/serial_io_handler_posix.h:18: enum ErrorDetectState { NO_ERROR, MARK_377_SEEN, MARK_0_SEEN }; On 2015/07/29 17:34:18, reillyg wrote: > This needs to be an enum class to use short names (NO_ERROR) instead of long > names (ERROR_DETECT_STATE_NO_ERROR). Done. https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... File device/serial/serial_io_handler_posix_unittest.cc (right): https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... device/serial/serial_io_handler_posix_unittest.cc:18: int call_CheckReceiveError(char* buffer, On 2015/07/29 17:34:18, reillyg wrote: > Don't mix underscores and camel-case in function names. Since there is a lot of > repetition below I suggest building a common test function that simply takes an > input state and expected output state. Done. https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... device/serial/serial_io_handler_posix_unittest.cc:43: int get_num_chars_ignored() { On 2015/07/29 17:34:18, reillyg wrote: > No need to prefix getters with "get_", "num_chars_ignored()" is the correct > function name. Done. https://codereview.chromium.org/1249933004/diff/80001/device/serial/serial_io... device/serial/serial_io_handler_posix_unittest.cc:70: buffer[0] = 'a'; On 2015/07/29 17:34:18, reillyg wrote: > This would be more readable if you built the buffer with an array literal. Done.
https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... File device/serial/serial_io_handler_posix_unittest.cc (right): https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:42: char get_chars_ignored(int index) { You don't need these wrappers anymore. https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:46: void initialize(ErrorDetectState error_detect_state, initialize -> Initialize https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:58: void test_helper(char* buffer, test_helper -> TestHelper https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:100: memcpy(buffer, "abc", bytes_read); Use an array literal instead of memcpy: char buffer[30] = { 'a', 'b', 'c' };
https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... File device/serial/serial_io_handler_posix.cc (right): https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:315: ReadCompleted(0, serial::RECEIVE_ERROR_BREAK); Sorry, I should have caught this earlier. ReadCompleted can't be called more than once here because it clears the "read pending" status. For lack of a better way to report status we should probably have break have priority over parity errors.
Please review the change. https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... File device/serial/serial_io_handler_posix.cc (right): https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:315: ReadCompleted(0, serial::RECEIVE_ERROR_BREAK); On 2015/07/30 00:09:12, reillyg wrote: > Sorry, I should have caught this earlier. ReadCompleted can't be called more > than once here because it clears the "read pending" status. For lack of a better > way to report status we should probably have break have priority over parity > errors. Done. https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... File device/serial/serial_io_handler_posix_unittest.cc (right): https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:42: char get_chars_ignored(int index) { On 2015/07/30 00:00:41, reillyg wrote: > You don't need these wrappers anymore. Done. https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:46: void initialize(ErrorDetectState error_detect_state, On 2015/07/30 00:00:41, reillyg wrote: > initialize -> Initialize Done. https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:58: void test_helper(char* buffer, On 2015/07/30 00:00:41, reillyg wrote: > test_helper -> TestHelper Done. https://codereview.chromium.org/1249933004/diff/100001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:100: memcpy(buffer, "abc", bytes_read); On 2015/07/30 00:00:41, reillyg wrote: > Use an array literal instead of memcpy: > > char buffer[30] = { 'a', 'b', 'c' }; Done.
Looking good. Just a few more suggestions. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... File device/serial/serial_io_handler_posix.cc (right): https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:500: Some DCHECKs may be helpful for documenting the invariants in this algorithm. For example, here you can say: DCHECK_LE(new_bytes_read, 2); https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:527: --new_bytes_read; To document the assumption that this is valid I suggest adding, DCHECK_GE(new_bytes_read, 2); to the beginning of this case. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:535: new_bytes_read -= 3; Similarly, DCHECK_GE(new_bytes_read, 3); at the top of this case. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... File device/serial/serial_io_handler_posix_unittest.cc (right): https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:18: void Initialize(ErrorDetectState error_detect_state, This parameter is never set to anything but NO_ERROR so it can be removed. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:510: 0, "\377\000a", 3, false, false); \000 -> \0 https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:523: 0, "b\377\000ac", 5, false, false); \000 -> \0 https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:578: 0, "\377\000a", 3, false, false); \000 -> \0 https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:598: 1, "\377\000a", 3, false, false); \000 -> \0 https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:617: 0, "\377\000ac", 4, false, false); \000 -> \0 https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:636: 1, "\377\0", 2, false, false); Add another call after this to check that "a" is produced when there's more buffer space available. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:654: 1, "\377\0", 2, false, false); Same here. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:672: 0, "\377\000a", 3, false, false); \000 -> \0 https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:692: 1, "\377\000a", 3, false, false); \000 -> \0 https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:712: 0, "\377\000ac", 4, false, false); \000 -> \0 https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:737: "\000a", 2, "\377", 1, false, false); \000 -> \0 https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:785: 0, "\377\000a", 3, false, false); \000 -> \0 https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:810: 1, "\377\000a", 3, false, false); \000 -> \0 https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:835: 0, "\377\000ac", 4, false, false); \000 -> \0
Please review the change. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... File device/serial/serial_io_handler_posix.cc (right): https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:500: On 2015/07/30 17:38:34, reillyg wrote: > Some DCHECKs may be helpful for documenting the invariants in this algorithm. > For example, here you can say: > > DCHECK_LE(new_bytes_read, 2); Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:527: --new_bytes_read; On 2015/07/30 17:38:34, reillyg wrote: > To document the assumption that this is valid I suggest adding, > > DCHECK_GE(new_bytes_read, 2); > > to the beginning of this case. Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:535: new_bytes_read -= 3; On 2015/07/30 17:38:34, reillyg wrote: > Similarly, > > DCHECK_GE(new_bytes_read, 3); > > at the top of this case. Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... File device/serial/serial_io_handler_posix_unittest.cc (right): https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:18: void Initialize(ErrorDetectState error_detect_state, On 2015/07/30 17:38:35, reillyg wrote: > This parameter is never set to anything but NO_ERROR so it can be removed. Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:510: 0, "\377\000a", 3, false, false); On 2015/07/30 17:38:35, reillyg wrote: > \000 -> \0 Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:523: 0, "b\377\000ac", 5, false, false); On 2015/07/30 17:38:34, reillyg wrote: > \000 -> \0 Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:578: 0, "\377\000a", 3, false, false); On 2015/07/30 17:38:34, reillyg wrote: > \000 -> \0 Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:598: 1, "\377\000a", 3, false, false); On 2015/07/30 17:38:35, reillyg wrote: > \000 -> \0 Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:617: 0, "\377\000ac", 4, false, false); On 2015/07/30 17:38:35, reillyg wrote: > \000 -> \0 Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:636: 1, "\377\0", 2, false, false); On 2015/07/30 17:38:35, reillyg wrote: > Add another call after this to check that "a" is produced when there's more > buffer space available. Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:654: 1, "\377\0", 2, false, false); On 2015/07/30 17:38:35, reillyg wrote: > Same here. Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:672: 0, "\377\000a", 3, false, false); On 2015/07/30 17:38:35, reillyg wrote: > \000 -> \0 Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:692: 1, "\377\000a", 3, false, false); On 2015/07/30 17:38:35, reillyg wrote: > \000 -> \0 Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:712: 0, "\377\000ac", 4, false, false); On 2015/07/30 17:38:35, reillyg wrote: > \000 -> \0 Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:737: "\000a", 2, "\377", 1, false, false); On 2015/07/30 17:38:35, reillyg wrote: > \000 -> \0 Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:785: 0, "\377\000a", 3, false, false); On 2015/07/30 17:38:35, reillyg wrote: > \000 -> \0 Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:810: 1, "\377\000a", 3, false, false); On 2015/07/30 17:38:35, reillyg wrote: > \000 -> \0 Done. https://codereview.chromium.org/1249933004/diff/120001/device/serial/serial_i... device/serial/serial_io_handler_posix_unittest.cc:835: 0, "\377\000ac", 4, false, false); On 2015/07/30 17:38:35, reillyg wrote: > \000 -> \0 Done.
https://codereview.chromium.org/1249933004/diff/140001/device/serial/serial_i... File device/serial/serial_io_handler_posix.cc (right): https://codereview.chromium.org/1249933004/diff/140001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:542: DCHECK_GE(new_bytes_read, 3); I meant putting these at the top level of the case as they aren't specific to any of these conditionals within.
Please review the change. https://codereview.chromium.org/1249933004/diff/140001/device/serial/serial_i... File device/serial/serial_io_handler_posix.cc (right): https://codereview.chromium.org/1249933004/diff/140001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:542: DCHECK_GE(new_bytes_read, 3); On 2015/07/30 20:07:03, reillyg wrote: > I meant putting these at the top level of the case as they aren't specific to > any of these conditionals within. Done.
lgtm after you resolve these two comments which are just wording the comments and variable naming. https://codereview.chromium.org/1249933004/diff/160001/device/serial/serial_i... File device/serial/serial_io_handler_posix.cc (right): https://codereview.chromium.org/1249933004/diff/160001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:556: // chars_ignored_[0], chars_ignored_[1], buffer[...]. // Now new_bytes_read bytes should be returned to the caller (including the // previously ignored characters that were stored at chars_ignored_[]) and are // now stored at: chars_ignored_[0], chars_ignored_[1], buffer[...]. https://codereview.chromium.org/1249933004/diff/160001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:560: // tmp[] stores the characters that need to be ignored for this read. I think that the term "stash" is better than "ignore" so I suggest renaming the variables. // Stash up to 2 characters that are potentially part of a break/parity error // sequence. The buffer may also not be large enough to store all the bytes. // tmp[] stores the characters that need to be stashed for this read.
https://codereview.chromium.org/1249933004/diff/160001/device/serial/serial_i... File device/serial/serial_io_handler_posix.cc (right): https://codereview.chromium.org/1249933004/diff/160001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:556: // chars_ignored_[0], chars_ignored_[1], buffer[...]. On 2015/07/30 21:19:43, reillyg wrote: > // Now new_bytes_read bytes should be returned to the caller (including the > // previously ignored characters that were stored at chars_ignored_[]) and are > // now stored at: chars_ignored_[0], chars_ignored_[1], buffer[...]. Done. https://codereview.chromium.org/1249933004/diff/160001/device/serial/serial_i... device/serial/serial_io_handler_posix.cc:560: // tmp[] stores the characters that need to be ignored for this read. On 2015/07/30 21:19:43, reillyg wrote: > I think that the term "stash" is better than "ignore" so I suggest renaming the > variables. > > // Stash up to 2 characters that are potentially part of a break/parity error > // sequence. The buffer may also not be large enough to store all the bytes. > // tmp[] stores the characters that need to be stashed for this read. Done.
The CQ bit was checked by juncai@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from reillyg@chromium.org Link to the patchset: https://codereview.chromium.org/1249933004/#ps180001 (title: " updated comments, changed variable name.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1249933004/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1249933004/180001
Message was sent while issue was closed.
Committed patchset #10 (id:180001)
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/923ede74ebc57633583795a84b33e994f7dcd4fa Cr-Commit-Position: refs/heads/master@{#341216} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
