Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1055313002:
Added mock_xhr module w/ unit tests. (Closed)
|
Created:
5 years, 8 months ago by John Williams Modified:
5 years, 8 months ago CC:
chromium-reviews, chromoting-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdded mock_xhr module w/ unit tests.
Also ported gcd_client_unittests to use mock_xhr as a proof of concept,
but left original test as a supplement to xhr_unittests.
BUG=471928
Committed: https://crrev.com/58e816f05221d24de6fe1d31dfaf22f08475d168
Cr-Commit-Position: refs/heads/master@{#324095}
Patch Set 1 #Patch Set 2 : #
Total comments: 48
Patch Set 3 : for review #
Total comments: 2
Patch Set 4 : for re-review #
Total comments: 2
Patch Set 5 : #
Total comments: 2
Patch Set 6 : #Patch Set 7 : merge #Patch Set 8 : #Patch Set 9 : for submit #
Messages
Total messages: 18 (6 generated)
jrw@chromium.org changed reviewers: + jamiewalch@chromium.org, kelvinp@chromium.org
New mock class in preparation for writing unit tests for host_controller. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/base/js... File remoting/webapp/base/js/base.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/base/js... remoting/webapp/base/js/base.js:218: * Returns a copy of the input object with all null/undefined fields Moved from xhr.js. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/gcd_client_with_mock_xhr_unittest.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/gcd_client_with_mock_xhr_unittest.js:1: // Copyright 2015 The Chromium Authors. All rights reserved. This is the original unit test for gcd_client, just renamed. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/mock_xhr.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:1: // Copyright (c) 2015 The Chromium Authors. All rights reserved. The main file for this CL. Needs more docs, I know. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/xhr.js (left): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/xhr.js:325: * Returns a copy of the input object with all null or undefined Moved to base.js. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/xhr.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/xhr.js:355: var value = paramHash[key]; To match semantics of base.copyWithoutNullFields.
Thanks for the code-review comments; they really helped with reviewing this. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/base/js... File remoting/webapp/base/js/base.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/base/js... remoting/webapp/base/js/base.js:231: if (value != null) { Is "value != null" the same as "value !== null && value !== undefined"? It won't, for example, fail for 0 or ''? https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/base/js... File remoting/webapp/base/js/base_unittest.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/base/js... remoting/webapp/base/js/base_unittest.js:75: undefinedField: undefined Add zero and '' fields to address my earlier comment. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/mock_xhr.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:1: // Copyright (c) 2015 The Chromium Authors. All rights reserved. No (c) needed https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:1: // Copyright (c) 2015 The Chromium Authors. All rights reserved. Doesn't sinon already support mocking out XMLHttpRequest? Some comment explaining why we need our own implementation would seem to be in order. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:15: * @param {remoting.Xhr.Params} params Would it make sense to declare this as @extends {XMLHttpRequest}? That way you could explicitly annotate the mock methods with @overrides. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:23: Duplicate blank line. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:49: base.copyWithoutNullFields(params.formContent), You're only calling copyWithoutNullFields if its parameter is null, in which case you know that the result will be {}. Why not use that here, or maybe call it unconditionally? https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:51: withCredentials: !!params.withCredentials, Prefer Boolean(x) to !!x for clarity. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:52: oauthToken: params.oauthToken == null : null params.oauthToken, This doesn't look right. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:61: // I don't think we have a big block comment like this anywhere else. A more natural ordering within the file would be to declare the private and helper functions at the end (that's what we've done elsewhere) so that the public methods are all together at the top. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:88: * @param {number} status I think this (and subsequent) methods should be @private. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:131: remoting.MockXhr.prototype.setResponse_ = function(response) { I think this (and some other) methods should be @private. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:143: /** @type {*} */ Can you be more specific about the type of this? https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:154: * it, or a RegExp, which is matched against URLs with the |test| I don't think "with the test method" adds any value to this comment, since it's implicit in the idea of matching a RegEx. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:160: * response to any method. s/response/respond/ https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:163: * @param {function(!remoting.MockXhr):void} proc The function to call |callback| would be a more conventional name for this. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:179: * Calls |setResponseFor| to add a 200 response with text content. s/200/204/ s/with text content/(no content)/ I would also probably omit "Calls |setResponseFor| to" from these descriptions, since it's not important to the caller how it adds the response. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/mock_xhr_unittest.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr_unittest.js:10: base.debug.throwOnAssert = true; You're setting a global here without restoring it in afterEach. In fact, it seems like a good idea to have this set to true for all unit-tests, but then this isn't the place to set it. Either move this somewhere where it affects all unit-tests, or restore it in afterEach.
https://codereview.chromium.org/1055313002/diff/40001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/gcd_client_with_mock_xhr_unittest.js (right): https://codereview.chromium.org/1055313002/diff/40001/remoting/webapp/crd/js/... remoting/webapp/crd/js/gcd_client_with_mock_xhr_unittest.js:41: setup: function() { use beforeEach and afterEach. See https://api.qunitjs.com/QUnit.module/
https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/base/js... File remoting/webapp/base/js/base.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/base/js... remoting/webapp/base/js/base.js:231: if (value != null) { On 2015/04/03 01:45:56, Jamie wrote: > Is "value != null" the same as "value !== null && value !== undefined"? It > won't, for example, fail for 0 or ''? Correct. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/base/js... File remoting/webapp/base/js/base_unittest.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/base/js... remoting/webapp/base/js/base_unittest.js:75: undefinedField: undefined On 2015/04/03 01:45:57, Jamie wrote: > Add zero and '' fields to address my earlier comment. Done. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/mock_xhr.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:1: // Copyright (c) 2015 The Chromium Authors. All rights reserved. On 2015/04/03 01:45:57, Jamie wrote: > Doesn't sinon already support mocking out XMLHttpRequest? Some comment > explaining why we need our own implementation would seem to be in order. Done. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:1: // Copyright (c) 2015 The Chromium Authors. All rights reserved. On 2015/04/03 01:45:57, Jamie wrote: > No (c) needed Because it's a test file, or because we're phasing it out for all files? https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:15: * @param {remoting.Xhr.Params} params On 2015/04/03 01:45:57, Jamie wrote: > Would it make sense to declare this as @extends {XMLHttpRequest}? That way you > could explicitly annotate the mock methods with @overrides. Doing that causes the compiler to check for private variables in Xhr that are also defined in MockXhr. Since there is one (deferred_), I would rather not rename it to satisfy the compiler, especially since an @extends declaration is basically a lie anyway. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:23: On 2015/04/03 01:45:57, Jamie wrote: > Duplicate blank line. Done. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:49: base.copyWithoutNullFields(params.formContent), On 2015/04/03 01:45:57, Jamie wrote: > You're only calling copyWithoutNullFields if its parameter is null, in which > case you know that the result will be {}. Why not use that here, or maybe call > it unconditionally? Having formContent === undefined is a special case that needs to be preserved because it's forbidden for more than one of {textContent, jsonContent, formContent} to be defined. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:51: withCredentials: !!params.withCredentials, On 2015/04/03 01:45:57, Jamie wrote: > Prefer Boolean(x) to !!x for clarity. Done. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:52: oauthToken: params.oauthToken == null : null params.oauthToken, On 2015/04/03 01:45:57, Jamie wrote: > This doesn't look right. Yeah, I was in a hurry to get this CL out before I had to leave :( https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:88: * @param {number} status On 2015/04/03 01:45:57, Jamie wrote: > I think this (and subsequent) methods should be @private. They need to be called by user code (see the unit tests for examples). https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:131: remoting.MockXhr.prototype.setResponse_ = function(response) { On 2015/04/03 01:45:57, Jamie wrote: > I think this (and some other) methods should be @private. Done. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:143: /** @type {*} */ On 2015/04/03 01:45:57, Jamie wrote: > Can you be more specific about the type of this? It's a pretty ugly type, but OK. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:154: * it, or a RegExp, which is matched against URLs with the |test| On 2015/04/03 01:45:57, Jamie wrote: > I don't think "with the test method" adds any value to this comment, since it's > implicit in the idea of matching a RegEx. Hmm. You're right. I'm used to there being two methods like |match| and |search| in Python. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:160: * response to any method. On 2015/04/03 01:45:57, Jamie wrote: > s/response/respond/ Done. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:163: * @param {function(!remoting.MockXhr):void} proc The function to call On 2015/04/03 01:45:57, Jamie wrote: > |callback| would be a more conventional name for this. Done. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:179: * Calls |setResponseFor| to add a 200 response with text content. On 2015/04/03 01:45:57, Jamie wrote: > s/200/204/ > s/with text content/(no content)/ > > I would also probably omit "Calls |setResponseFor| to" from these descriptions, > since it's not important to the caller how it adds the response. Done. I said "calls |setResponseFor|" because |setResponseFor| is a public method with more extensive documentation. Is there another way I should express this, or should I just copy the full documentation from |setResponseFor|? https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/mock_xhr_unittest.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr_unittest.js:10: base.debug.throwOnAssert = true; On 2015/04/03 01:45:58, Jamie wrote: > You're setting a global here without restoring it in afterEach. In fact, it > seems like a good idea to have this set to true for all unit-tests, but then > this isn't the place to set it. > > Either move this somewhere where it affects all unit-tests, or restore it in > afterEach. Couldn't find a good place to move it to, so I just restored the old value. https://codereview.chromium.org/1055313002/diff/40001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/gcd_client_with_mock_xhr_unittest.js (right): https://codereview.chromium.org/1055313002/diff/40001/remoting/webapp/crd/js/... remoting/webapp/crd/js/gcd_client_with_mock_xhr_unittest.js:41: setup: function() { On 2015/04/03 18:34:22, kelvinp wrote: > use beforeEach and afterEach. > See https://api.qunitjs.com/QUnit.module/ Done.
https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/mock_xhr.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:1: // Copyright (c) 2015 The Chromium Authors. All rights reserved. On 2015/04/04 01:40:26, John Williams wrote: > On 2015/04/03 01:45:57, Jamie wrote: > > No (c) needed > > Because it's a test file, or because we're phasing it out for all files? It's not needed for any new files (for the past year, at least), but there's no requirement to remove it from old files. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:15: * @param {remoting.Xhr.Params} params On 2015/04/04 01:40:26, John Williams wrote: > On 2015/04/03 01:45:57, Jamie wrote: > > Would it make sense to declare this as @extends {XMLHttpRequest}? That way you > > could explicitly annotate the mock methods with @overrides. > > Doing that causes the compiler to check for private variables in Xhr that are > also defined in MockXhr. Since there is one (deferred_), I would rather not > rename it to satisfy the compiler, especially since an @extends declaration is > basically a lie anyway. Acknowledged. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:49: base.copyWithoutNullFields(params.formContent), On 2015/04/04 01:40:26, John Williams wrote: > On 2015/04/03 01:45:57, Jamie wrote: > > You're only calling copyWithoutNullFields if its parameter is null, in which > > case you know that the result will be {}. Why not use that here, or maybe call > > it unconditionally? > > Having formContent === undefined is a special case that needs to be preserved > because it's forbidden for more than one of {textContent, jsonContent, > formContent} to be defined. That's not what this does, I don't think. If params.formContent is undefined, then formContent will be passed as copyWithoutNF(undefined), which is {}. So you're not preserving undefined here. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:143: /** @type {*} */ On 2015/04/04 01:40:26, John Williams wrote: > On 2015/04/03 01:45:57, Jamie wrote: > > Can you be more specific about the type of this? > > It's a pretty ugly type, but OK. Yuck; that is horrible. I was thinking it would be as simple as {remoting.Xhr}, but of course you need the type of the ctor. Can you add a comment to that effect, since (as you point out) this is not very readable. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:179: * Calls |setResponseFor| to add a 200 response with text content. On 2015/04/04 01:40:26, John Williams wrote: > On 2015/04/03 01:45:57, Jamie wrote: > > s/200/204/ > > s/with text content/(no content)/ > > > > I would also probably omit "Calls |setResponseFor| to" from these > descriptions, > > since it's not important to the caller how it adds the response. > > Done. > > I said "calls |setResponseFor|" because |setResponseFor| is a public method with > more extensive documentation. Is there another way I should express this, or > should I just copy the full documentation from |setResponseFor|? Either copy them, or add a "see setResponseFor" comment. https://codereview.chromium.org/1055313002/diff/60001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/mock_xhr.js (right): https://codereview.chromium.org/1055313002/diff/60001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:53: oauthToken: params.oauthToken === undefined ? undefined : params.oauthToken, The RHS is equivalent to params.oauthToken, surely?
https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/mock_xhr.js (right): https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:49: base.copyWithoutNullFields(params.formContent), On 2015/04/06 19:24:03, Jamie wrote: > On 2015/04/04 01:40:26, John Williams wrote: > > On 2015/04/03 01:45:57, Jamie wrote: > > > You're only calling copyWithoutNullFields if its parameter is null, in which > > > case you know that the result will be {}. Why not use that here, or maybe > call > > > it unconditionally? > > > > Having formContent === undefined is a special case that needs to be preserved > > because it's forbidden for more than one of {textContent, jsonContent, > > formContent} to be defined. > > That's not what this does, I don't think. If params.formContent is undefined, > then formContent will be passed as copyWithoutNF(undefined), which is {}. So > you're not preserving undefined here. I changed it to be more explicit about undefined. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:143: /** @type {*} */ On 2015/04/06 19:24:03, Jamie wrote: > On 2015/04/04 01:40:26, John Williams wrote: > > On 2015/04/03 01:45:57, Jamie wrote: > > > Can you be more specific about the type of this? > > > > It's a pretty ugly type, but OK. > > Yuck; that is horrible. I was thinking it would be as simple as {remoting.Xhr}, > but of course you need the type of the ctor. Can you add a comment to that > effect, since (as you point out) this is not very readable. Done. https://codereview.chromium.org/1055313002/diff/20001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:179: * Calls |setResponseFor| to add a 200 response with text content. On 2015/04/06 19:24:03, Jamie wrote: > On 2015/04/04 01:40:26, John Williams wrote: > > On 2015/04/03 01:45:57, Jamie wrote: > > > s/200/204/ > > > s/with text content/(no content)/ > > > > > > I would also probably omit "Calls |setResponseFor| to" from these > > descriptions, > > > since it's not important to the caller how it adds the response. > > > > Done. > > > > I said "calls |setResponseFor|" because |setResponseFor| is a public method > with > > more extensive documentation. Is there another way I should express this, or > > should I just copy the full documentation from |setResponseFor|? > > Either copy them, or add a "see setResponseFor" comment. Done. https://codereview.chromium.org/1055313002/diff/60001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/mock_xhr.js (right): https://codereview.chromium.org/1055313002/diff/60001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:53: oauthToken: params.oauthToken === undefined ? undefined : params.oauthToken, On 2015/04/06 19:24:03, Jamie wrote: > The RHS is equivalent to params.oauthToken, surely? Not sure what I was thinking there. Fixed.
lgtm https://codereview.chromium.org/1055313002/diff/80001/remoting/webapp/crd/js/... File remoting/webapp/crd/js/mock_xhr.js (right): https://codereview.chromium.org/1055313002/diff/80001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:149: * The original value of the remoting.MockXhr constructor. s/MockXhr/Xhr/ (I think). Also, please add something along the lines of "The JSDoc type is that of the remoting.Xhr constructor function." https://codereview.chromium.org/1055313002/diff/80001/remoting/webapp/crd/js/... remoting/webapp/crd/js/mock_xhr.js:165: * called. The handler should generally call one on s/on/of/
The CQ bit was checked by jrw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jamiewalch@chromium.org Link to the patchset: https://codereview.chromium.org/1055313002/#ps120001 (title: "merge")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1055313002/120001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by jrw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jamiewalch@chromium.org Link to the patchset: https://codereview.chromium.org/1055313002/#ps160001 (title: "for submit")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1055313002/160001
Message was sent while issue was closed.
Committed patchset #9 (id:160001)
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/58e816f05221d24de6fe1d31dfaf22f08475d168 Cr-Commit-Position: refs/heads/master@{#324095} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
