Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
905523002:
platformKeys: Add per-extension sign permissions. (Closed)
|
Created:
5 years, 10 months ago by pneubeck (no reviews) Modified:
5 years, 10 months ago CC:
chromium-reviews, extensions-reviews_chromium.org, nkostylev+watch_chromium.org, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@pks_sign_task Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionplatformKeys: Add per-extension sign permissions.
PlatformKeysService now supports persisting whether an extension is allowed to sign data with a key an unlimited number of times.
Currently, these permissions are only granted in the accompanying browser test and not in production, because UI is still missing.
BUG=450167
Committed: https://crrev.com/cbcdfd84a59e7216d315689362ccc41626608987
Cr-Commit-Position: refs/heads/master@{#317053}
Patch Set 1 : #
Total comments: 53
Patch Set 2 : #Patch Set 3 : Addressed comment. #
Messages
Total messages: 24 (13 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Patchset #2 (id:80001) has been deleted
Patchset #1 (id:60001) has been deleted
Patchset #1 (id:100001) has been deleted
Patchset #1 (id:120001) has been deleted
pneubeck@chromium.org changed reviewers: + kaliamoorthi@chromium.org
ptal
pneubeck@chromium.org changed reviewers: + atwilson@chromium.org - kaliamoorthi@chromium.org
-Prabhu Drew, Prabhu is busy as well until branch point. ptal instead (or assign to someone else). Thanks!
kaliamoorthi@chromium.org changed reviewers: + kaliamoorthi@chromium.org
I did it anyway. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... File chrome/browser/chromeos/platform_keys/platform_keys_service.cc (right): https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:25: bool sign_once = false; Why do we have two bools for sign_once and sign_unlimited. This can result in inconsistent state. How about an enum that eliminates these two bools or a single variable sign_unlimited which when false implies sign once? https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:100: if (!entry.sign_once && !entry.sign_unlimited) Move this above the new. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:103: if (entry.sign_once) { Is there a reason why the bool is written only when true? https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:177: // The original key entry before setting the new value |new_permission_value|. This is confusing, original key entry and new permission value. Please make consistent. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:248: } Should there be a default unreached(); return here? I get this is an enum and all but it is better to be defensive. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:490: if (!interactive_ || matches_.empty()) { This seem to unnecessarily continue through the rest of the states. I guess UpdatePermission can be skipped in this situation that may eliminate the check for interactive_ again in UpdatePermission. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:667: bool interactive, Can't you deduce this by the fact that SelectDelegate is set or not? Having additional flag leaves room for making a mistake. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/extensio... File chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc (right): https://codereview.chromium.org/905523002/diff/140001/chrome/browser/extensio... chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc:234: new TestSelectDelegate(client_cert1_ /* select no cert */))); Incorrect comment.
LGTM once comments addressed https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... File chrome/browser/chromeos/platform_keys/platform_keys.h (right): https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys.h:116: // Returns the DER encoding of the X.509 Subject Public Key Info of the public nit: "a DER encoding" since there isn't a single canonical DER encoding. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... File chrome/browser/chromeos/platform_keys/platform_keys_service.cc (right): https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:21: // The base64 encoded DER of a X.509 Subject Public Key Info. Nit: "the..." -> "a base64-encoded DER" https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:27: // True if the key can be used for signing unlimited often. nit: "signing an unlimited number of times" - "unlimited often" sounds odd because unlimited is an adjective, not an adverb. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:39: // From older versions of ChromeOS, this key can hold a list of base64 encoded nit: base64-encoded https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:74: if (entry->GetAsString(&new_entry.spki_b64)) { You talk about this above, but not bad to explain what is happening here in a comment about old vs new state entries. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:98: nit:permissions, not premissions. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:122: if (entry.spki_b64 == public_key_spki_der_b64) Thing to remember here is that DER encodings are not canonical. I think it doesn't matter here, but I've been bitten by this before, trying to compare two identical keys by their DER and having it fail so wanted to point it out. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:236: LOG(ERROR) << "Requested one-time sign permission on existing key."; Should this be a CHECK()? Or at least a DCHECK()? Or can this happen due to an extension misusing the API? https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:324: if (!callback_.is_null()) { Is this necessary? I thought calling Run() on a null callback was a no-op? https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:548: // updated correctly. What does "ensures that the permissions were updated correctly" mean? You don't have a CHECK or DCHECK for this case below, it appears - if there's a bug in the update permissions code, the end result will be that the user selects a cert but the extension never sees it. Seems like if we ever encounter this situation (interactive_ && selected_cert_ && filtered_certs->empty()) it seems like a logic error that should be flagged? https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:612: select_delegate_ = delegate.Pass(); DCHECK(interactive_) might be appropriate here? Or get rid of interactive_ entirely as Prabhu suggests? Also, perhaps DCHECK(!select_delegate_) would be useful to ensure that we aren't setting the delegate twice? Not sure what the contract is around this. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... File chrome/browser/chromeos/platform_keys/platform_keys_service.h (right): https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:45: class SelectDelegate { Would be nice if we had some class-level documentation here - this is an interesting class that is called into, and which itself calls another callback back when it is done with Select(). This is an unusual enough pattern that it should be documented. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:48: using Callback = nit: I'm not a huge fan of overloading names (SelectDelegate::Callback vs Callback - IMO they make the code harder to read as the user has to be aware of what the current namespace is). Perhaps something more descriptive like CertificateSelectedCallback? https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:56: // destructed. |callback| will not be called after this delegate is What do you mean |callback| will not be called after this delegate is destructed? What are you trying to say here? Because typically one does not describe behavior in class documentation around things that may or may not happen after the object itself is destroyed? Are you trying to say that if the object is destroyed mid-Select(), it should not invoke the callback from its destructor? That's OK, but there are better ways to deal with this type of dependency (like using weak pointers that are reset before the delegate is freed). https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:65: private: This class has no data members - why are you restricting assignment constructors? https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:158: // SelectDelegate. Afterwards filters only the certificates that the extension I couldn't quite understand what "Afterwards filters only the certificates that the extension has unlimited sign permission for". Shouldn't certificates be filtered *before* the user is prompted to select one, not "afterwards"? https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:175: class SelectTask; Should these classes be ordered somehow? https://codereview.chromium.org/905523002/diff/140001/chrome/browser/extensio... File chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc (right): https://codereview.chromium.org/905523002/diff/140001/chrome/browser/extensio... chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc:118: chromeos::PlatformKeysService& GetPlatformKeysService() { Don't return non-const references.
New patchsets have been uploaded after l-g-t-m from atwilson@chromium.org
ptal https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... File chrome/browser/chromeos/platform_keys/platform_keys.h (right): https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys.h:116: // Returns the DER encoding of the X.509 Subject Public Key Info of the public On 2015/02/18 19:53:34, Andrew T Wilson wrote: > nit: "a DER encoding" since there isn't a single canonical DER encoding. Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... File chrome/browser/chromeos/platform_keys/platform_keys_service.cc (right): https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:21: // The base64 encoded DER of a X.509 Subject Public Key Info. On 2015/02/18 19:53:35, Andrew T Wilson wrote: > Nit: "the..." -> "a base64-encoded DER" Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:25: bool sign_once = false; On 2015/02/18 17:04:09, kaliamoorthi wrote: > Why do we have two bools for sign_once and sign_unlimited. This can result in > inconsistent state. How about an enum that eliminates these two bools or a > single variable sign_unlimited which when false implies sign once? These two permissions origin from the two different API platformKeys and enterprise.platformKeys. They're only used at a time if an extension would happen to use both for the same key. I added a more detailed explanation. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:27: // True if the key can be used for signing unlimited often. On 2015/02/18 19:53:35, Andrew T Wilson wrote: > nit: "signing an unlimited number of times" - "unlimited often" sounds odd > because unlimited is an adjective, not an adverb. Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:39: // From older versions of ChromeOS, this key can hold a list of base64 encoded On 2015/02/18 19:53:34, Andrew T Wilson wrote: > nit: base64-encoded Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:74: if (entry->GetAsString(&new_entry.spki_b64)) { On 2015/02/18 19:53:35, Andrew T Wilson wrote: > You talk about this above, but not bad to explain what is happening here in a > comment about old vs new state entries. Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:98: On 2015/02/18 19:53:35, Andrew T Wilson wrote: > nit:permissions, not premissions. Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:100: if (!entry.sign_once && !entry.sign_unlimited) On 2015/02/18 17:04:09, kaliamoorthi wrote: > Move this above the new. Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:103: if (entry.sign_once) { On 2015/02/18 17:04:09, kaliamoorthi wrote: > Is there a reason why the bool is written only when true? added a comment that defaults values are not stored. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:122: if (entry.spki_b64 == public_key_spki_der_b64) On 2015/02/18 19:53:35, Andrew T Wilson wrote: > Thing to remember here is that DER encodings are not canonical. I think it > doesn't matter here, but I've been bitten by this before, trying to compare two > identical keys by their DER and having it fail so wanted to point it out. Good point. I added a clarification that DER is actually unique (thanks Wikipedia :-) ) https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:177: // The original key entry before setting the new value |new_permission_value|. On 2015/02/18 17:04:09, kaliamoorthi wrote: > This is confusing, original key entry and new permission value. Please make > consistent. Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:236: LOG(ERROR) << "Requested one-time sign permission on existing key."; On 2015/02/18 19:53:34, Andrew T Wilson wrote: > Should this be a CHECK()? Or at least a DCHECK()? Or can this happen due to an > extension misusing the API? 'should never occur'... so DCHECK (according to the wisdom of our chormium style guide). https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:248: } On 2015/02/18 17:04:09, kaliamoorthi wrote: > Should there be a default unreached(); return here? > > I get this is an enum and all but it is better to be defensive. The defensive thing is to not use default. As it is, the compiler can check whether all enum values are covered. If not it fails to compile. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:324: if (!callback_.is_null()) { On 2015/02/18 19:53:35, Andrew T Wilson wrote: > Is this necessary? I thought calling Run() on a null callback was a no-op? no it's not: CallbackBase::CallbackBase(BindStateBase* bind_state) : bind_state_(bind_state), polymorphic_invoke_(NULL) {...} class Callback ... { Callback() : CallbackBase(NULL) { } R Run(...) { PolymorphicInvoke f = reinterpret_cast<PolymorphicInvoke>(polymorphic_invoke_); return f(bind_state_.get(), internal::CallbackForward(args)...); } } https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:490: if (!interactive_ || matches_.empty()) { On 2015/02/18 17:04:09, kaliamoorthi wrote: > This seem to unnecessarily continue through the rest of the states. I guess > UpdatePermission can be skipped in this situation that may eliminate the check > for interactive_ again in UpdatePermission. I did this to make the transitions as constant as possible. I changed it to skip both steps, see the modification of Step(). Actually that's rather readable as it's well documented now in Step(). https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:548: // updated correctly. On 2015/02/18 19:53:35, Andrew T Wilson wrote: > What does "ensures that the permissions were updated correctly" mean? You don't > have a CHECK or DCHECK for this case below, it appears - if there's a bug in the > update permissions code, the end result will be that the user selects a cert but > the extension never sees it. Seems like if we ever encounter this situation > (interactive_ && selected_cert_ && filtered_certs->empty()) it seems like a > logic error that should be flagged? Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:612: select_delegate_ = delegate.Pass(); On 2015/02/18 19:53:35, Andrew T Wilson wrote: > DCHECK(interactive_) might be appropriate here? Or get rid of interactive_ > entirely as Prabhu suggests? > > Also, perhaps DCHECK(!select_delegate_) would be useful to ensure that we aren't > setting the delegate twice? Not sure what the contract is around this. There's some confusion about the scoped of interactive_ and select_delegate_. The delegate is set once (e.g. in the factory) and lives up-to the destruction of PlatformKeysService. interactive is a property of a single SelectClientCertificate call, of which there can be many during the lifetime of PlatformKeysService. Some of these calls can be interactive some non-interactive. The delegate is never used for non-interactive calls but might be used for interactive ones. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.cc:667: bool interactive, On 2015/02/18 17:04:09, kaliamoorthi wrote: > Can't you deduce this by the fact that SelectDelegate is set or not? Having > additional flag leaves room for making a mistake. see reply to Drew's comment above. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... File chrome/browser/chromeos/platform_keys/platform_keys_service.h (right): https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:45: class SelectDelegate { On 2015/02/18 19:53:35, Andrew T Wilson wrote: > Would be nice if we had some class-level documentation here - this is an > interesting class that is called into, and which itself calls another callback > back when it is done with Select(). This is an unusual enough pattern that it > should be documented. Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:48: using Callback = On 2015/02/18 19:53:35, Andrew T Wilson wrote: > nit: I'm not a huge fan of overloading names (SelectDelegate::Callback vs > Callback - IMO they make the code harder to read as the user has to be aware of > what the current namespace is). Perhaps something more descriptive like > CertificateSelectedCallback? Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:56: // destructed. |callback| will not be called after this delegate is On 2015/02/18 19:53:35, Andrew T Wilson wrote: > What do you mean |callback| will not be called after this delegate is > destructed? What are you trying to say here? Because typically one does not > describe behavior in class documentation around things that may or may not > happen after the object itself is destroyed? > Are you trying to say that if the object is destroyed mid-Select(), it should > not invoke the callback from its destructor? That's OK, but there are better > ways to deal with this type of dependency (like using weak pointers that are > reset before the delegate is freed). Yes, that's the point. This is part of the contract and fits explicit ownership rather well: After SelectDelegate is destructed there shouldn't be anything left that can call back. Why would a weak pointer be preferred here? The Delegate can do that himself to fulfill the contract. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:65: private: On 2015/02/18 19:53:35, Andrew T Wilson wrote: > This class has no data members - why are you restricting assignment > constructors? the assignment operator is not virtual. calling it on a SelectDelegate type will be valid but a no-op. So nobody will notice the error. Not a huge deal, just prevents a small class of mistakes. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:158: // SelectDelegate. Afterwards filters only the certificates that the extension On 2015/02/18 19:53:35, Andrew T Wilson wrote: > I couldn't quite understand what "Afterwards filters only the certificates that > the extension has unlimited sign permission for". Shouldn't certificates be > filtered *before* the user is prompted to select one, not "afterwards"? I made a bit more verbose. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:175: class SelectTask; On 2015/02/18 19:53:35, Andrew T Wilson wrote: > Should these classes be ordered somehow? Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/extensio... File chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc (right): https://codereview.chromium.org/905523002/diff/140001/chrome/browser/extensio... chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc:118: chromeos::PlatformKeysService& GetPlatformKeysService() { On 2015/02/18 19:53:35, Andrew T Wilson wrote: > Don't return non-const references. Done. https://codereview.chromium.org/905523002/diff/140001/chrome/browser/extensio... chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc:234: new TestSelectDelegate(client_cert1_ /* select no cert */))); On 2015/02/18 17:04:09, kaliamoorthi wrote: > Incorrect comment. Done.
https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... File chrome/browser/chromeos/platform_keys/platform_keys_service.h (right): https://codereview.chromium.org/905523002/diff/140001/chrome/browser/chromeos... chrome/browser/chromeos/platform_keys/platform_keys_service.h:56: // destructed. |callback| will not be called after this delegate is On 2015/02/19 11:08:40, pneubeck wrote: > On 2015/02/18 19:53:35, Andrew T Wilson wrote: > > What do you mean |callback| will not be called after this delegate is > > destructed? What are you trying to say here? Because typically one does not > > describe behavior in class documentation around things that may or may not > > happen after the object itself is destroyed? > > > Are you trying to say that if the object is destroyed mid-Select(), it should > > not invoke the callback from its destructor? That's OK, but there are better > > ways to deal with this type of dependency (like using weak pointers that are > > reset before the delegate is freed). > > Yes, that's the point. This is part of the contract and fits explicit ownership > rather well: After SelectDelegate is destructed there shouldn't be anything left > that can call back. > Why would a weak pointer be preferred here? The Delegate can do that himself to > fulfill the contract. OK, then you should word this prescriptively, not descriptively (i.e. "|callback| must not be invoked after this delegate is destroyed").
LGTM
LGTM
New patchsets have been uploaded after l-g-t-m from kaliamoorthi@chromium.org
The CQ bit was checked by pneubeck@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/905523002/180001
Message was sent while issue was closed.
Committed patchset #3 (id:180001)
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/cbcdfd84a59e7216d315689362ccc41626608987 Cr-Commit-Position: refs/heads/master@{#317053} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
