|
|
Created:
3 years, 8 months ago by sammiequon Modified:
3 years, 8 months ago Reviewers:
Daniel Erat CC:
chromium-reviews, oshima+watch_chromium.org, hashimoto+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionCrOS: Add success/failure indicators for biod methods with no callback.
A couple methods in biod_client.h do not have any indicators of success or failure. Modify these functions to uses VoidDBusMethodCallbaks so users have some indication of D-Bus failures.
TEST=chromeos_unittests --gtest_filter="BiodClientTest.*"
BUG=700933
Review-Url: https://codereview.chromium.org/2799043007
Cr-Commit-Position: refs/heads/master@{#463385}
Committed: https://chromium.googlesource.com/chromium/src/+/63e20eb86328f72e4c6f2707d49d66a55f9ee12e
Patch Set 1 #
Total comments: 26
Patch Set 2 : Fixed patch set 1 errors. #
Total comments: 22
Patch Set 3 : Fixed patch set 2 errors. #
Total comments: 6
Patch Set 4 : Empty -> null. #
Messages
Total messages: 28 (15 generated)
Patchset #1 (id:1) has been deleted
sammiequon@chromium.org changed reviewers: + derat@chromium.org
derat@ - Could you take a look? Thanks!
https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.cc (right): https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:74: biod_proxy_->CallMethodWithErrorCallback( instead of using CallMethodWithErrorCallback, i think you can continue to use CallMethod. it will log errors for you, and you can check whether the response is null or not to decide if you should pass failure or success to the callback. applies to all other methods in this change. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:219: void OnVoidMethodSuccess(const VoidDBusMethodCallback& callback, this isn't accessing any of the class's state, so you can just move it into a function in the anon namespace https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:221: if (!response) { much simpler: callback.Run(response ? DBUS_METHOD_CALL_SUCCESS : DBUS_METHOD_CALL_FAILURE); https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.h (right): https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:98: // Irreversibly destroys all records registered. |callback| is called with i'd change the second sentence to something simpler like "|callback| is called asynchronously with the result." applies to other methods too https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:101: delete blank line https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:115: delete blank line https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:122: delete blank line https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:129: delete blank line https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:157: void AddMethodWithErrorCallbackExpectation( per earlier comment, you probably don't need this https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:257: void OnCallMethodWithErrorCallback( or this https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:429: // type. Also, logs will get printed. what's the reason for mentioning that messages will be logged? you're not verifying that in the test, right?
https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.cc (right): https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:74: biod_proxy_->CallMethodWithErrorCallback( On 2017/04/08 00:12:34, Daniel Erat wrote: > instead of using CallMethodWithErrorCallback, i think you can continue to use > CallMethod. it will log errors for you, and you can check whether the response > is null or not to decide if you should pass failure or success to the callback. > > applies to all other methods in this change. Hmm, ok. But when would call with error callback be preferred then? https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:219: void OnVoidMethodSuccess(const VoidDBusMethodCallback& callback, On 2017/04/08 00:12:34, Daniel Erat wrote: > this isn't accessing any of the class's state, so you can just move it into a > function in the anon namespace Done. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:221: if (!response) { On 2017/04/08 00:12:34, Daniel Erat wrote: > much simpler: > > callback.Run(response ? DBUS_METHOD_CALL_SUCCESS : DBUS_METHOD_CALL_FAILURE); Done. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.h (right): https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:98: // Irreversibly destroys all records registered. |callback| is called with On 2017/04/08 00:12:34, Daniel Erat wrote: > i'd change the second sentence to something simpler like "|callback| is called > asynchronously with the result." > > applies to other methods too Done. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:101: On 2017/04/08 00:12:34, Daniel Erat wrote: > delete blank line Done. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:115: On 2017/04/08 00:12:34, Daniel Erat wrote: > delete blank line Oops, done. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:122: On 2017/04/08 00:12:34, Daniel Erat wrote: > delete blank line Done. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:129: On 2017/04/08 00:12:34, Daniel Erat wrote: > delete blank line Done. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:157: void AddMethodWithErrorCallbackExpectation( On 2017/04/08 00:12:34, Daniel Erat wrote: > per earlier comment, you probably don't need this Done. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:257: void OnCallMethodWithErrorCallback( On 2017/04/08 00:12:34, Daniel Erat wrote: > or this Done. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:429: // type. Also, logs will get printed. On 2017/04/08 00:12:35, Daniel Erat wrote: > what's the reason for mentioning that messages will be logged? you're not > verifying that in the test, right? No, I thought it would be a good heads ups for filling up the console. Done.
https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.cc (right): https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:74: biod_proxy_->CallMethodWithErrorCallback( On 2017/04/08 01:03:30, sammiequon wrote: > On 2017/04/08 00:12:34, Daniel Erat wrote: > > instead of using CallMethodWithErrorCallback, i think you can continue to use > > CallMethod. it will log errors for you, and you can check whether the response > > is null or not to decide if you should pass failure or success to the > callback. > > > > applies to all other methods in this change. > > Hmm, ok. But when would call with error callback be preferred then? you mean, when should CallMethodWithErrorCallback be used? it's mostly just useful if the service actually includes additional information in its error response and you want to be able to extract it. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:429: // type. Also, logs will get printed. On 2017/04/08 01:03:30, sammiequon wrote: > On 2017/04/08 00:12:35, Daniel Erat wrote: > > what's the reason for mentioning that messages will be logged? you're not > > verifying that in the test, right? > > No, I thought it would be a good heads ups for filling up the console. Done. i think that switching to CallMethod (and letting the dbus code log the error instead of logging it yourself) may also avoid the log spam when running tests. :-)
https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.cc (left): https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:261: void OnRequestRecordLabel(const LabelCallback& callback, did you re-run the tests? this looks like it was deleted accidentally; it's still being called. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.cc (right): https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:22: void OnVoidMethod(const VoidDBusMethodCallback& callback, nit: OnVoidResponse? https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.h (right): https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:99: // asynchronosly with the result. nit: asynchronously (note 'u', also add below) https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:111: // with D-Bus failure if there is a D-Bus error along the way; called with shorten second sentence here too https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:113: delete blank line after comment https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:129: // used for authentication. |callback| is asynchronosly with the result. fix second sentence (missing 'called' after 'is') https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:329: // Create a fake response with nothing. The destroy all records call should "Create an empty response to simulate success." https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:339: // Create a fake response with nothing. The destroy all records call should "Return an empty response to simulate failure." would be more accurate, i think https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:387: biod::BiometricType::BIOMETRIC_TYPE_FINGERPRINT; oh, this isn't actually an enum class, right? i think you can just do biod::BIOMETRIC_TYPE_FINGERPRINT here and below. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:431: // Verify that by sending a empty reponse the response is an empty string. nit: "the response is" -> "the result is" may be clearer; otherwise you're calling two different things responses
Patchset #3 (id:60001) has been deleted
https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.cc (right): https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:74: biod_proxy_->CallMethodWithErrorCallback( On 2017/04/08 01:06:03, Daniel Erat wrote: > On 2017/04/08 01:03:30, sammiequon wrote: > > On 2017/04/08 00:12:34, Daniel Erat wrote: > > > instead of using CallMethodWithErrorCallback, i think you can continue to > use > > > CallMethod. it will log errors for you, and you can check whether the > response > > > is null or not to decide if you should pass failure or success to the > > callback. > > > > > > applies to all other methods in this change. > > > > Hmm, ok. But when would call with error callback be preferred then? > > you mean, when should CallMethodWithErrorCallback be used? it's mostly just > useful if the service actually includes additional information in its error > response and you want to be able to extract it. Acknowledged. https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2799043007/diff/20001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:429: // type. Also, logs will get printed. On 2017/04/08 01:06:03, Daniel Erat wrote: > On 2017/04/08 01:03:30, sammiequon wrote: > > On 2017/04/08 00:12:35, Daniel Erat wrote: > > > what's the reason for mentioning that messages will be logged? you're not > > > verifying that in the test, right? > > > > No, I thought it would be a good heads ups for filling up the console. Done. > > i think that switching to CallMethod (and letting the dbus code log the error > instead of logging it yourself) may also avoid the log spam when running tests. > :-) I see. I think think spam was the wrong word, it was like 3 lines. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.cc (left): https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:261: void OnRequestRecordLabel(const LabelCallback& callback, On 2017/04/08 01:13:32, Daniel Erat wrote: > did you re-run the tests? this looks like it was deleted accidentally; it's > still being called. Oh no, oops. Good thing you caught it.. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.cc (right): https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.cc:22: void OnVoidMethod(const VoidDBusMethodCallback& callback, On 2017/04/08 01:13:32, Daniel Erat wrote: > nit: OnVoidResponse? Done. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client.h (right): https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:99: // asynchronosly with the result. On 2017/04/08 01:13:32, Daniel Erat wrote: > nit: asynchronously (note 'u', also add below) Oh wow, i knew that I would spell that wrong so I looked extra careful and still made a mistake :S. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:111: // with D-Bus failure if there is a D-Bus error along the way; called with On 2017/04/08 01:13:32, Daniel Erat wrote: > shorten second sentence here too Done. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:113: On 2017/04/08 01:13:32, Daniel Erat wrote: > delete blank line after comment Done. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client.h:129: // used for authentication. |callback| is asynchronosly with the result. On 2017/04/08 01:13:32, Daniel Erat wrote: > fix second sentence (missing 'called' after 'is') Done. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:329: // Create a fake response with nothing. The destroy all records call should On 2017/04/08 01:13:32, Daniel Erat wrote: > "Create an empty response to simulate success." Done. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:339: // Create a fake response with nothing. The destroy all records call should On 2017/04/08 01:13:32, Daniel Erat wrote: > "Return an empty response to simulate failure." would be more accurate, i think Done. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:387: biod::BiometricType::BIOMETRIC_TYPE_FINGERPRINT; On 2017/04/08 01:13:32, Daniel Erat wrote: > oh, this isn't actually an enum class, right? i think you can just do > biod::BIOMETRIC_TYPE_FINGERPRINT here and below. Is it prefer not to. I usually just did it anyways even if its not needed. Done. https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:431: // Verify that by sending a empty reponse the response is an empty string. On 2017/04/08 01:13:32, Daniel Erat wrote: > nit: "the response is" -> "the result is" may be clearer; otherwise you're > calling two different things responses Done.
https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:387: biod::BiometricType::BIOMETRIC_TYPE_FINGERPRINT; On 2017/04/08 01:31:48, sammiequon wrote: > On 2017/04/08 01:13:32, Daniel Erat wrote: > > oh, this isn't actually an enum class, right? i think you can just do > > biod::BIOMETRIC_TYPE_FINGERPRINT here and below. > > Is it prefer not to. I usually just did it anyways even if its not needed. Done. yep, shorter's better. iirc the BIOMETRIC_TYPE_ prefix was added to make it clearer what this constant is even when the BiometricType label is omitted from the identifier.
The CQ bit was checked by sammiequon@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2799043007/diff/40001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:387: biod::BiometricType::BIOMETRIC_TYPE_FINGERPRINT; On 2017/04/08 01:39:23, Daniel Erat wrote: > On 2017/04/08 01:31:48, sammiequon wrote: > > On 2017/04/08 01:13:32, Daniel Erat wrote: > > > oh, this isn't actually an enum class, right? i think you can just do > > > biod::BIOMETRIC_TYPE_FINGERPRINT here and below. > > > > Is it prefer not to. I usually just did it anyways even if its not needed. > Done. > > yep, shorter's better. iirc the BIOMETRIC_TYPE_ prefix was added to make it > clearer what this constant is even when the BiometricType label is omitted from > the identifier. Acknowledged.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
thanks! lgtm with a few comment corrections https://codereview.chromium.org/2799043007/diff/80001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2799043007/diff/80001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:340: // Return an empty response to simulate failure. s/an empty/a null/ https://codereview.chromium.org/2799043007/diff/80001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:402: // Verify that by sending a empty reponse the response is an unknown biometric s/empty/null/ https://codereview.chromium.org/2799043007/diff/80001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:430: // Verify that by sending a empty reponse the result is an empty string. s/empty/null/
Patchset #4 (id:100001) has been deleted
The CQ bit was checked by sammiequon@chromium.org to run a CQ dry run
Thanks! https://codereview.chromium.org/2799043007/diff/80001/chromeos/dbus/biod/biod... File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2799043007/diff/80001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:340: // Return an empty response to simulate failure. On 2017/04/08 14:55:55, Daniel Erat wrote: > s/an empty/a null/ Done. https://codereview.chromium.org/2799043007/diff/80001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:402: // Verify that by sending a empty reponse the response is an unknown biometric On 2017/04/08 14:55:55, Daniel Erat wrote: > s/empty/null/ Done. https://codereview.chromium.org/2799043007/diff/80001/chromeos/dbus/biod/biod... chromeos/dbus/biod/biod_client_unittest.cc:430: // Verify that by sending a empty reponse the result is an empty string. On 2017/04/08 14:55:55, Daniel Erat wrote: > s/empty/null/ Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by sammiequon@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 120001, "attempt_start_ts": 1491856529654270, "parent_rev": "ee1f0a96ff9d1a4f524f88156e3fbbdb53234c1c", "commit_rev": "63e20eb86328f72e4c6f2707d49d66a55f9ee12e"}
Message was sent while issue was closed.
Description was changed from ========== CrOS: Add success/failure indicators for biod methods with no callback. A couple methods in biod_client.h do not have any indicators of success or failure. Modify these functions to uses VoidDBusMethodCallbaks so users have some indication of D-Bus failures. TEST=chromeos_unittests --gtest_filter="BiodClientTest.*" BUG=700933 ========== to ========== CrOS: Add success/failure indicators for biod methods with no callback. A couple methods in biod_client.h do not have any indicators of success or failure. Modify these functions to uses VoidDBusMethodCallbaks so users have some indication of D-Bus failures. TEST=chromeos_unittests --gtest_filter="BiodClientTest.*" BUG=700933 Review-Url: https://codereview.chromium.org/2799043007 Cr-Commit-Position: refs/heads/master@{#463385} Committed: https://chromium.googlesource.com/chromium/src/+/63e20eb86328f72e4c6f2707d49d... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:120001) as https://chromium.googlesource.com/chromium/src/+/63e20eb86328f72e4c6f2707d49d... |