Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2032733002:
Do not crash on null stream in writing to bidirectional streams (Closed)
DescriptionDo not crash on null stream in writing to bidirectional streams
When SendData/SendvData is called, it might be that OnClose
has been invoked without an error, in which case
BidirectionalStream::Delegate::OnFailed would not have been
invoked. This is theoretically possible, but I am not
aware if such case would exist.
This CL removes the assumption that SendData/SendvData
is only called when stream is alive. If stream is
destroyed, propagate that as an ERR_UNEXPECTED to caller,
instead of crashing with a segfault.
BUG=606394
Committed: https://crrev.com/707f895efd414219d609533e82be1ffc60fd9788
Cr-Commit-Position: refs/heads/master@{#398031}
Patch Set 1 #
Total comments: 12
Patch Set 2 : Address comments #Patch Set 3 : self review #Patch Set 4 : #Patch Set 5 : Added spdy tests #
Total comments: 8
Patch Set 6 : Rebased #
Total comments: 6
Patch Set 7 : Self review #Patch Set 8 : Address Andrei's comments #
Total comments: 6
Patch Set 9 : Address Andrei's comments round 2 #
Messages
Total messages: 34 (12 generated)
Description was changed from ========== Do not crash on null stream BUG= ========== to ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propogate that as an ERR_EXPECTED to caller, instead of crashing with a segfault. BUG=606394 ==========
Description was changed from ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propogate that as an ERR_EXPECTED to caller, instead of crashing with a segfault. BUG=606394 ========== to ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This is theoretically possible, but I am not aware if such case would exist. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propagate that as an ERR_EXPECTED to caller, instead of crashing with a segfault. BUG=606394 ==========
Description was changed from ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This is theoretically possible, but I am not aware if such case would exist. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propagate that as an ERR_EXPECTED to caller, instead of crashing with a segfault. BUG=606394 ========== to ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This is theoretically possible, but I am not aware if such case would exist. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propagate that as an ERR_EXPECTED to caller, instead of crashing with a segfault. BUG=606394 ==========
Description was changed from ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This is theoretically possible, but I am not aware if such case would exist. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propagate that as an ERR_EXPECTED to caller, instead of crashing with a segfault. BUG=606394 ========== to ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This is theoretically possible, but I am not aware if such case would exist. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propagate that as an ERR_EXPECTED to caller, instead of crashing with a segfault. BUG=606394 ==========
xunjieli@chromium.org changed reviewers: + kapishnikov@chromium.org, mef@chromium.org
Andrei, Misha: mind taking a look? This is regarding internal bug: 29059108.
Um, does this fix the problem or its symptoms? https://codereview.chromium.org/2032733002/diff/1/net/http/bidirectional_stre... File net/http/bidirectional_stream_impl.h (right): https://codereview.chromium.org/2032733002/diff/1/net/http/bidirectional_stre... net/http/bidirectional_stream_impl.h:77: // Called when an error occurred. Do not cal into the stream after this cal -> call. https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... File net/quic/bidirectional_stream_quic_impl.cc (left): https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... net/quic/bidirectional_stream_quic_impl.cc:241: ResetStream(); Why no longer ResetStream() here? https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... File net/quic/bidirectional_stream_quic_impl.cc (right): https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... net/quic/bidirectional_stream_quic_impl.cc:125: LOG(ERROR) << "Trying to send data when stream has been destroyed."; when -> after https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... net/quic/bidirectional_stream_quic_impl.cc:128: weak_factory_.GetWeakPtr(), ERR_UNEXPECTED)); nit: Should say ERR_UNEXPECTED in CL description. https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... net/quic/bidirectional_stream_quic_impl.cc:162: LOG(ERROR) << "Trying to send data when stream has been destroyed."; when -> after
https://codereview.chromium.org/2032733002/diff/1/net/http/bidirectional_stre... File net/http/bidirectional_stream.h (right): https://codereview.chromium.org/2032733002/diff/1/net/http/bidirectional_stre... net/http/bidirectional_stream.h:85: // point. I think it is important to keep the contract that no other delegate functions can be called after OnFailed(), at least these calls should not be propagated to the bidirectional stream client.
Description was changed from ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This is theoretically possible, but I am not aware if such case would exist. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propagate that as an ERR_EXPECTED to caller, instead of crashing with a segfault. BUG=606394 ========== to ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This is theoretically possible, but I am not aware if such case would exist. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propagate that as an ERR_UNEXPECTED to caller, instead of crashing with a segfault. BUG=606394 ==========
Thanks for the review! > Um, does this fix the problem or its symptoms? Sadly, I don't know whether this will fix the problem, as I don't know a way to confirm. With this CL, we won't crash the app if somehow we enter this state. That's one workaround solution. I am not sure if this is the best we can do. https://codereview.chromium.org/2032733002/diff/1/net/http/bidirectional_stre... File net/http/bidirectional_stream.h (right): https://codereview.chromium.org/2032733002/diff/1/net/http/bidirectional_stre... net/http/bidirectional_stream.h:85: // point. On 2016/06/01 22:09:49, kapishnikov wrote: > I think it is important to keep the contract that no other delegate functions > can be called after OnFailed(), at least these calls should not be propagated to > the bidirectional stream client. Done. https://codereview.chromium.org/2032733002/diff/1/net/http/bidirectional_stre... File net/http/bidirectional_stream_impl.h (right): https://codereview.chromium.org/2032733002/diff/1/net/http/bidirectional_stre... net/http/bidirectional_stream_impl.h:77: // Called when an error occurred. Do not cal into the stream after this On 2016/06/01 21:53:15, mef wrote: > cal -> call. Done. https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... File net/quic/bidirectional_stream_quic_impl.cc (left): https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... net/quic/bidirectional_stream_quic_impl.cc:241: ResetStream(); On 2016/06/01 21:53:15, mef wrote: > Why no longer ResetStream() here? It is redundant. NotifyError will call ResetStream(). https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... File net/quic/bidirectional_stream_quic_impl.cc (right): https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... net/quic/bidirectional_stream_quic_impl.cc:125: LOG(ERROR) << "Trying to send data when stream has been destroyed."; On 2016/06/01 21:53:15, mef wrote: > when -> after Done. https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... net/quic/bidirectional_stream_quic_impl.cc:128: weak_factory_.GetWeakPtr(), ERR_UNEXPECTED)); On 2016/06/01 21:53:15, mef wrote: > nit: Should say ERR_UNEXPECTED in CL description. Done. https://codereview.chromium.org/2032733002/diff/1/net/quic/bidirectional_stre... net/quic/bidirectional_stream_quic_impl.cc:162: LOG(ERROR) << "Trying to send data when stream has been destroyed."; On 2016/06/01 21:53:15, mef wrote: > when -> after Done.
Patchset #4 (id:60001) has been deleted
Patchset #4 (id:80001) has been deleted
Misha, Andrei: A friendly ping..
On 2016/06/03 17:29:08, xunjieli wrote: > Misha, Andrei: A friendly ping.. Is OnClose coming from the server? In this case stream should still be valid until client sends end_of_stream, right?
On 2016/06/03 17:53:53, mef wrote: > On 2016/06/03 17:29:08, xunjieli wrote: > > Misha, Andrei: A friendly ping.. > > Is OnClose coming from the server? > In this case stream should still be valid until client sends end_of_stream, > right? No. OnClose can come from the client as well. For example, when client decided to tear down the connection for whatever reason and all streams will be closed/destroyed (since a connection owns its streams. BidirectionalSteamQuiImpl has a raw pointer to the underlying QuicChromiumClientStream).
On 2016/06/03 17:58:15, xunjieli wrote: > On 2016/06/03 17:53:53, mef wrote: > > On 2016/06/03 17:29:08, xunjieli wrote: > > > Misha, Andrei: A friendly ping.. > > > > Is OnClose coming from the server? > > In this case stream should still be valid until client sends end_of_stream, > > right? > > No. OnClose can come from the client as well. For example, when client decided > to tear down the connection for whatever reason and all streams will be > closed/destroyed (since a connection owns its streams. BidirectionalSteamQuiImpl > has a raw pointer to the underlying QuicChromiumClientStream). So, this means that it is possible that ResetStream is called on the network thread from OnClose or Cancel or NotifyError while there is another task (for example SendData) posted to network thread, which has not been processed yet. In this case ResetStream would null the pointer, and task really should check for that pointer to not being null, right? I'm not sure whether it should report failure in that case though. Supposedly stream is already canceled, or delegate is notified of original error. WDYT?
On 2016/06/03 19:34:56, mef wrote: > On 2016/06/03 17:58:15, xunjieli wrote: > > On 2016/06/03 17:53:53, mef wrote: > > > On 2016/06/03 17:29:08, xunjieli wrote: > > > > Misha, Andrei: A friendly ping.. > > > > > > Is OnClose coming from the server? > > > In this case stream should still be valid until client sends end_of_stream, > > > right? > > > > No. OnClose can come from the client as well. For example, when client decided > > to tear down the connection for whatever reason and all streams will be > > closed/destroyed (since a connection owns its streams. > BidirectionalSteamQuiImpl > > has a raw pointer to the underlying QuicChromiumClientStream). > > So, this means that it is possible that ResetStream is called on the network > thread from OnClose or Cancel or NotifyError while there is another task (for > example SendData) posted to network thread, which has not been processed yet. > > In this case ResetStream would null the pointer, and task really should check > for that pointer to not being null, right? > > I'm not sure whether it should report failure in that case though. Supposedly > stream is already canceled, or delegate is notified of original error. Yes. This matches my understanding too. If delegate is notified of an error (through NotifyError()) or the delegate canceled, we null out delegate to make sure we don't call back the delegate. If there is an failure, we notify exactly once. If failure happens after cancellation, we don't notify delegate.
I think bailing out if stream_ is null is the right thing to do. I'm not so sure about posting error notification. https://codereview.chromium.org/2032733002/diff/140001/net/quic/bidirectional... File net/quic/bidirectional_stream_quic_impl.cc (right): https://codereview.chromium.org/2032733002/diff/140001/net/quic/bidirectional... net/quic/bidirectional_stream_quic_impl.cc:168: LOG(ERROR) << "Trying to send data after stream has been destroyed."; I think logging is fine, but I'm not sure that posting ERR_UNEXPECTED is correct. What if stream got destroyed because of network error? https://codereview.chromium.org/2032733002/diff/140001/net/quic/bidirectional... net/quic/bidirectional_stream_quic_impl.cc:202: weak_factory_.InvalidateWeakPtrs(); should this be part of ResetStream()? https://codereview.chromium.org/2032733002/diff/140001/net/quic/bidirectional... net/quic/bidirectional_stream_quic_impl.cc:328: weak_factory_.InvalidateWeakPtrs(); If we invalidate any pending callbacks here, why do we need to add 'if (delegate_)' checks everywhere? Wouldn't those callbacks get canceled?
The change looks good. I assume that NotifyError() and QUIC delegate methods are
always called on the same thread. There are some comments below.
It would be great to untangle the logic that manipulates |delegate_| and
|stream_| members; otherwise, it is difficult to track all possible states.
E.g., NotifyError() method resets the stream only if the delegate is not NULL.
It is not obvious why it is so. What if in the future, someone adds "delegate_ =
nullptr" in some other method, thinking that it will just disable all calls to
the delegate? Taking NotifyError() as an example, maybe we should have something
like:
if (stream_) {
ResetStream()
}
if (delegate_) {
BidirectionalStreamImpl::Delegate* delegate = delegate_;
delegate_ = nullptr;
// Cancel any pending callback.
delegate->OnFailed(error);
}
weak_factory_.InvalidateWeakPtrs();
https://codereview.chromium.org/2032733002/diff/120001/net/quic/bidirectional...
File net/quic/bidirectional_stream_quic_impl.cc (right):
https://codereview.chromium.org/2032733002/diff/120001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.cc:51:
BidirectionalStreamImpl::Delegate* delegate,
Is delegate optional here, i.e. is it allowed to pass NULL? If yes,
NotifyError() will not do everything it needs to do.
https://codereview.chromium.org/2032733002/diff/120001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.cc:200: stream_->SetDelegate(nullptr);
ResetStream() calls "stream_->SetDelegate(nullptr)". Do we need to call it here
as well?
https://codereview.chromium.org/2032733002/diff/120001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.cc:202: delegate_ = nullptr;
Should we unconditionally set "delegate_ = nullptr", i.e. before if(!stream_)?
Otherwise, there is a risk that other method be called, which will result in
delegate->OnFailed(error) call.
https://codereview.chromium.org/2032733002/diff/120001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.cc:326: ResetStream();
Should we call ResetStream() before "if (!delegate_)" to make sure that it is
always called? E.g., if in the future we add "delegate_ = nullptr" in some other
method, ResetStream() may not always be called by OnClose() method.
Just saw Andrei's comments. Will address those shortly. https://codereview.chromium.org/2032733002/diff/140001/net/quic/bidirectional... File net/quic/bidirectional_stream_quic_impl.cc (right): https://codereview.chromium.org/2032733002/diff/140001/net/quic/bidirectional... net/quic/bidirectional_stream_quic_impl.cc:168: LOG(ERROR) << "Trying to send data after stream has been destroyed."; On 2016/06/03 20:03:19, mef wrote: > I think logging is fine, but I'm not sure that posting ERR_UNEXPECTED is > correct. What if stream got destroyed because of network error? If stream is destroyed gracefully because of a network error, OnClose() will be called with that error, and we will notify the delegate with a net error. The situation we want to guard against is that the stream is destroyed without invoking OnClose(error != OK). So I think ERR_UNEXPECTED is okay, since that signifies a programming error. https://codereview.chromium.org/2032733002/diff/140001/net/quic/bidirectional... net/quic/bidirectional_stream_quic_impl.cc:202: weak_factory_.InvalidateWeakPtrs(); On 2016/06/03 20:03:19, mef wrote: > should this be part of ResetStream()? Great question! If OnClose(OK) is invoked, we do ResetStream(), but we don't want to cancel any pending reads which have not completed yet. https://codereview.chromium.org/2032733002/diff/140001/net/quic/bidirectional... net/quic/bidirectional_stream_quic_impl.cc:328: weak_factory_.InvalidateWeakPtrs(); On 2016/06/03 20:03:19, mef wrote: > If we invalidate any pending callbacks here, why do we need to add 'if > (delegate_)' checks everywhere? Wouldn't those callbacks get canceled? Great question! InvalidateWeakPtrs() only invalidates posted tasks, any future tasks will still go through. I can't come up with a situation where we will later encounter callbacks though. I think we can remove the delegate_ checks in places other than NotifyError. But we can also leave them as a double measure.
Thanks Andrei for noticing the logic flaw. I realized some of it after doing a self review of my patch. Could you take a look to see if the rest is addressed too? Thanks. https://codereview.chromium.org/2032733002/diff/120001/net/quic/bidirectional... File net/quic/bidirectional_stream_quic_impl.cc (right): https://codereview.chromium.org/2032733002/diff/120001/net/quic/bidirectional... net/quic/bidirectional_stream_quic_impl.cc:51: BidirectionalStreamImpl::Delegate* delegate, On 2016/06/03 20:10:08, kapishnikov wrote: > Is delegate optional here, i.e. is it allowed to pass NULL? If yes, > NotifyError() will not do everything it needs to do. Great catch! We need a delegate. I added a check. https://codereview.chromium.org/2032733002/diff/120001/net/quic/bidirectional... net/quic/bidirectional_stream_quic_impl.cc:200: stream_->SetDelegate(nullptr); On 2016/06/03 20:10:08, kapishnikov wrote: > ResetStream() calls "stream_->SetDelegate(nullptr)". Do we need to call it here > as well? Great catch! https://codereview.chromium.org/2032733002/diff/120001/net/quic/bidirectional... net/quic/bidirectional_stream_quic_impl.cc:202: delegate_ = nullptr; On 2016/06/03 20:10:08, kapishnikov wrote: > Should we unconditionally set "delegate_ = nullptr", i.e. before if(!stream_)? > Otherwise, there is a risk that other method be called, which will result in > delegate->OnFailed(error) call. Great catch! I've addressed it a newer patch. https://codereview.chromium.org/2032733002/diff/120001/net/quic/bidirectional... net/quic/bidirectional_stream_quic_impl.cc:326: ResetStream(); On 2016/06/03 20:10:07, kapishnikov wrote: > Should we call ResetStream() before "if (!delegate_)" to make sure that it is > always called? E.g., if in the future we add "delegate_ = nullptr" in some other > method, ResetStream() may not always be called by OnClose() method. Great catch. I've also realized this and addressed it in a newer patch.
https://codereview.chromium.org/2032733002/diff/180001/net/spdy/bidirectional... File net/spdy/bidirectional_stream_spdy_impl.cc (right): https://codereview.chromium.org/2032733002/diff/180001/net/spdy/bidirectional... net/spdy/bidirectional_stream_spdy_impl.cc:162: DCHECK(!stream_); nit: This DCheck will always succeed and can be removed. https://codereview.chromium.org/2032733002/diff/180001/net/spdy/bidirectional... net/spdy/bidirectional_stream_spdy_impl.cc:163: delegate_ = nullptr; Should go ahead of "if (!stream_)". https://codereview.chromium.org/2032733002/diff/180001/net/spdy/bidirectional... net/spdy/bidirectional_stream_spdy_impl.cc:299: void BidirectionalStreamSpdyImpl::NotifyError(int rv) { Should we call "stream_->DetachDelegate()" here? This would be consistent with the QUIC implementation.
Patchset #9 (id:200001) has been deleted
Thanks a lot for the review!! https://codereview.chromium.org/2032733002/diff/180001/net/spdy/bidirectional... File net/spdy/bidirectional_stream_spdy_impl.cc (right): https://codereview.chromium.org/2032733002/diff/180001/net/spdy/bidirectional... net/spdy/bidirectional_stream_spdy_impl.cc:162: DCHECK(!stream_); On 2016/06/03 22:46:41, kapishnikov wrote: > nit: This DCheck will always succeed and can be removed. Done. https://codereview.chromium.org/2032733002/diff/180001/net/spdy/bidirectional... net/spdy/bidirectional_stream_spdy_impl.cc:163: delegate_ = nullptr; On 2016/06/03 22:46:41, kapishnikov wrote: > Should go ahead of "if (!stream_)". Done. https://codereview.chromium.org/2032733002/diff/180001/net/spdy/bidirectional... net/spdy/bidirectional_stream_spdy_impl.cc:299: void BidirectionalStreamSpdyImpl::NotifyError(int rv) { On 2016/06/03 22:46:41, kapishnikov wrote: > Should we call "stream_->DetachDelegate()" here? This would be consistent with > the QUIC implementation. Done, I added a if-conditional though. Since DetachDelegate has a DCHECK(!IsClosed()), it is only legal to call when the stream isn't closed.
A friendly ping. I have 4 more crash reports on this. If possible, should we aim for this week's drop?
Looks good to me. LGTM.
lgtm
On 2016/06/06 14:13:00, mef wrote: > lgtm Thanks! I have a follow-up CL to improve logging so we can better diagnose any resulting issues easier.
The CQ bit was checked by xunjieli@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2032733002/220001
Message was sent while issue was closed.
Description was changed from ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This is theoretically possible, but I am not aware if such case would exist. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propagate that as an ERR_UNEXPECTED to caller, instead of crashing with a segfault. BUG=606394 ========== to ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This is theoretically possible, but I am not aware if such case would exist. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propagate that as an ERR_UNEXPECTED to caller, instead of crashing with a segfault. BUG=606394 ==========
Message was sent while issue was closed.
Committed patchset #9 (id:220001)
Message was sent while issue was closed.
Description was changed from ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This is theoretically possible, but I am not aware if such case would exist. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propagate that as an ERR_UNEXPECTED to caller, instead of crashing with a segfault. BUG=606394 ========== to ========== Do not crash on null stream in writing to bidirectional streams When SendData/SendvData is called, it might be that OnClose has been invoked without an error, in which case BidirectionalStream::Delegate::OnFailed would not have been invoked. This is theoretically possible, but I am not aware if such case would exist. This CL removes the assumption that SendData/SendvData is only called when stream is alive. If stream is destroyed, propagate that as an ERR_UNEXPECTED to caller, instead of crashing with a segfault. BUG=606394 Committed: https://crrev.com/707f895efd414219d609533e82be1ffc60fd9788 Cr-Commit-Position: refs/heads/master@{#398031} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/707f895efd414219d609533e82be1ffc60fd9788 Cr-Commit-Position: refs/heads/master@{#398031} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
