New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 775593 link

Starred by 1 user
Status: Fixed
Owner:
zhongyi@chromium.org
Closed: Nov 2017
Cc:
█ jri@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 774622



Sign in to add a comment

Clean up connection migration v1

Project Member Reported by zhongyi@chromium.org, Oct 17 2017 
Currently connection migration code is wired up in both QuicStreamFactory and QuicChromiumClientSession. 

When connection migration is caused by platform notification, signals are received by QuicStreamFactory, which notifies session to do migration. 

When connection migration is caused by path degrading or write error, signals are detected by session, which however routers to the QuicStreamFactory, and then back to the session. 

Ultimately the code can be cleaned up and migration is all managed by session. In the case of platform notification, stream factory only needs to broadcast to sessions.
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 18 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/673f22ef3e1177bb89bc9c15dc1cd170a2cbb8e1

commit 673f22ef3e1177bb89bc9c15dc1cd170a2cbb8e1
Author: Zhongyi Shi <zhongyi@chromium.org>
Date: Wed Oct 18 00:00:47 2017

Connection Migration: move QuicStreamFactory::MigrationSessionInner logic 
to QuicChromiumClientSession::Migrate so that we can clean up further to 
move all migration logic to session. 
 

Bug:  775593 
Change-Id: Id70c45fe7d0c05ce3fb34084c336e89d48c50500
Reviewed-on: https://chromium-review.googlesource.com/723782
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509610}
[modify] https://crrev.com/673f22ef3e1177bb89bc9c15dc1cd170a2cbb8e1/net/quic/chromium/quic_chromium_client_session.cc
[modify] https://crrev.com/673f22ef3e1177bb89bc9c15dc1cd170a2cbb8e1/net/quic/chromium/quic_chromium_client_session.h
[modify] https://crrev.com/673f22ef3e1177bb89bc9c15dc1cd170a2cbb8e1/net/quic/chromium/quic_stream_factory.cc
[modify] https://crrev.com/673f22ef3e1177bb89bc9c15dc1cd170a2cbb8e1/net/quic/chromium/quic_stream_factory.h
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 21 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22e91af1f8dd2b6564506bbe7a0d1d93e506afdf

commit 22e91af1f8dd2b6564506bbe7a0d1d93e506afdf
Author: Zhongyi Shi <zhongyi@chromium.org>
Date: Sat Oct 21 00:53:57 2017

Connection Migration Clean-up: 
add QuicChromiumClientSession::MigrateToAlternateNetwork to replace QuicStreamFactory::MaybeMigrateSingleSession.
add QuicChromiumClientSession::MaybeMigrateOrCloseSession() which takes over QuicStreamFactory::MaybeMigrateOrCloseSession logic.
correct comments that have been out of sync.

Bug:  775593 
Change-Id: I9b300bd704bc128e8ab27bd9d70f933848cb64d8
Reviewed-on: https://chromium-review.googlesource.com/726245
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510626}
[modify] https://crrev.com/22e91af1f8dd2b6564506bbe7a0d1d93e506afdf/net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc
[modify] https://crrev.com/22e91af1f8dd2b6564506bbe7a0d1d93e506afdf/net/quic/chromium/quic_chromium_client_session.cc
[modify] https://crrev.com/22e91af1f8dd2b6564506bbe7a0d1d93e506afdf/net/quic/chromium/quic_chromium_client_session.h
[modify] https://crrev.com/22e91af1f8dd2b6564506bbe7a0d1d93e506afdf/net/quic/chromium/quic_chromium_client_session_test.cc
[modify] https://crrev.com/22e91af1f8dd2b6564506bbe7a0d1d93e506afdf/net/quic/chromium/quic_http_stream_test.cc
[modify] https://crrev.com/22e91af1f8dd2b6564506bbe7a0d1d93e506afdf/net/quic/chromium/quic_proxy_client_socket_unittest.cc
[modify] https://crrev.com/22e91af1f8dd2b6564506bbe7a0d1d93e506afdf/net/quic/chromium/quic_stream_factory.cc
[modify] https://crrev.com/22e91af1f8dd2b6564506bbe7a0d1d93e506afdf/net/quic/chromium/quic_stream_factory.h
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 1 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8001500f052d3a126e874e3ef154e80fd3c5bd7b

commit 8001500f052d3a126e874e3ef154e80fd3c5bd7b
Author: Zhongyi Shi <zhongyi@chromium.org>
Date: Wed Nov 01 22:15:37 2017

Connection migation clean-up: move connection migration logic OnWriteError/PathDegrading
to session.

Bug:  775593 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: If844eb130c63a24d140237498dfba585e665f2e0
Reviewed-on: https://chromium-review.googlesource.com/726940
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513300}
[modify] https://crrev.com/8001500f052d3a126e874e3ef154e80fd3c5bd7b/net/quic/chromium/quic_chromium_client_session.cc
[modify] https://crrev.com/8001500f052d3a126e874e3ef154e80fd3c5bd7b/net/quic/chromium/quic_stream_factory.cc
[modify] https://crrev.com/8001500f052d3a126e874e3ef154e80fd3c5bd7b/net/quic/chromium/quic_stream_factory.h

Comment 4 by zhongyi@chromium.org, Nov 1 2017

Labels: -Pri-3 Pri-2
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 2 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f124a589423c2305e0d67d9bf0a3d01f5aeda58d

commit f124a589423c2305e0d67d9bf0a3d01f5aeda58d
Author: Zhongyi Shi <zhongyi@chromium.org>
Date: Thu Nov 02 00:15:04 2017

Connection Migration clean-up: deprecate QuicStreamFactory::MaybeMigrateOrCloseSession() and MigrateSessionToNewPeerAddress().
Move connection migration logic to QuicChromiumClientSession::OnNetworkDisconnected/OnNetworkMadeDefault and change QuicStreamFactory to broadcast signals to each session.

Bug:  775593 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I246f31340116e10969286c1a835212be64609148
Reviewed-on: https://chromium-review.googlesource.com/727541
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513332}
[modify] https://crrev.com/f124a589423c2305e0d67d9bf0a3d01f5aeda58d/net/quic/chromium/quic_chromium_client_session.cc
[modify] https://crrev.com/f124a589423c2305e0d67d9bf0a3d01f5aeda58d/net/quic/chromium/quic_chromium_client_session.h
[modify] https://crrev.com/f124a589423c2305e0d67d9bf0a3d01f5aeda58d/net/quic/chromium/quic_stream_factory.cc
[modify] https://crrev.com/f124a589423c2305e0d67d9bf0a3d01f5aeda58d/net/quic/chromium/quic_stream_factory.h
[modify] https://crrev.com/f124a589423c2305e0d67d9bf0a3d01f5aeda58d/net/quic/chromium/quic_stream_factory_test.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 2 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/066985fd732f62e122e1a13ac1212ca746704e3e

commit 066985fd732f62e122e1a13ac1212ca746704e3e
Author: Zhongyi Shi <zhongyi@chromium.org>
Date: Thu Nov 02 09:11:34 2017

Connection Migration Cleanup: move connection migration metrics collection to session.

Bug:  775593 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Id4920d5cdc814eac863a7ed55e8c69ce6a3e9ccf
Reviewed-on: https://chromium-review.googlesource.com/727077
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513432}
[modify] https://crrev.com/066985fd732f62e122e1a13ac1212ca746704e3e/net/quic/chromium/quic_chromium_client_session.cc
[modify] https://crrev.com/066985fd732f62e122e1a13ac1212ca746704e3e/net/quic/chromium/quic_chromium_client_session.h
[modify] https://crrev.com/066985fd732f62e122e1a13ac1212ca746704e3e/net/quic/chromium/quic_stream_factory.cc
[modify] https://crrev.com/066985fd732f62e122e1a13ac1212ca746704e3e/net/quic/chromium/quic_stream_factory.h

Comment 7 by zhongyi@chromium.org, Nov 2 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment