New issue
Advanced search Search tips

Issue 754695 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 3
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug
Proj-Servicification


Sign in to add a comment

Deprecate NetworkChangeNotifier::IPAddressObserver and ConnectionTypeObserver

Project Member Reported by xunji...@chromium.org, Aug 11 2017

Issue description

NetworkChangeNotifier::IPAddressObserver and ConnectionTypeObserver are deprecated. New consumers should use NetworkChangeObserver.

For motivation, please see pauljensen@'s design doc:
https://docs.google.com/document/d/1ch22_dRHPQJ9QArFMmWl_UyVpl-EDXA0PdljGh2kDF0/edit#heading=h.mlge0axnxo7
 
Paul's WIP CL is at https://codereview.chromium.org/11620007/. The main blocker is to test each NCN consumer. 

I can upload a CL to add comment in the header file to mention that these two observer interfaces are deprecated.
Blocking: 754709
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 11 2017

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

commit f234a7f9cb5998b0d7db1ebf1b4a6639e8d1f746
Author: Helen Li <xunjieli@chromium.org>
Date: Fri Aug 11 16:06:51 2017

Mark IPAddressObserver and ConnectionTypeObserver as deprecated

This CL adds comments in network_change_notifier.h to mark IPAddressObserver and
ConnectionTypeObserver as deprecated.

Bug:  754695 
Change-Id: I4659ddcf535d154e8b0e04c833c2d649e070f858
Reviewed-on: https://chromium-review.googlesource.com/612187
Commit-Queue: Helen Li <xunjieli@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493763}
[modify] https://crrev.com/f234a7f9cb5998b0d7db1ebf1b4a6639e8d1f746/net/base/network_change_notifier.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 14 2017

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

commit a02fde012b42cb011ca215fb861dac057645360a
Author: Helen Li <xunjieli@chromium.org>
Date: Mon Aug 14 13:58:27 2017

Make NetworkChangeNotifier::HistogramWatcher a private class

This is to follow up on pauljensen@'s CL at
https://codereview.chromium.org/11620007/ which proposed the change.

Bug:  754695 
Change-Id: Id289623121d53e53b988dd23b11938600249ed99
Reviewed-on: https://chromium-review.googlesource.com/612472
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494043}
[modify] https://crrev.com/a02fde012b42cb011ca215fb861dac057645360a/net/base/network_change_notifier.cc
[modify] https://crrev.com/a02fde012b42cb011ca215fb861dac057645360a/net/base/network_change_notifier.h

Owner: xunji...@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 15 2017

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

commit fa5f3a845f40c4c2b395d2f802e7166ae7289c69
Author: Helen Li <xunjieli@chromium.org>
Date: Tue Aug 15 18:07:31 2017

Use NetworkChangeObserver in tab_loader_delegate.cc

ConnectionTypeObserver is being deprecated. This CL migrates
tab_loader_delegate.cc to using the replacement, NetworkChangeObserver.
NetworkChangeObserver's signals are less noisy because they come after network
connection has been established rather than while it is being established.

For more details, see the linked design doc in the bug below.

Bug:  754695 
Change-Id: I335460ce0cd8e7c015b62bddc76c2f1d33f80bbc
Reviewed-on: https://chromium-review.googlesource.com/615460
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494448}
[modify] https://crrev.com/fa5f3a845f40c4c2b395d2f802e7166ae7289c69/chrome/browser/sessions/tab_loader_delegate.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 15 2017

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

commit 43564b01a2a7542baa822e9e62b57873d2bf2110
Author: Helen Li <xunjieli@chromium.org>
Date: Tue Aug 15 20:52:36 2017

Use NetworkChangeObserver in user_session_manager.cc

ConnectionTypeObserver is being deprecated. This CL migrates
user_session_manager.cc to using the replacement, NetworkChangeObserver.
NetworkChangeObserver's signals are less noisy because they come after network
connection has been established rather than while it is being established.

For more details, see the linked design doc in the bug below.

Bug:  754695 
Change-Id: Iea47308b7fa8ba97d5236e13055a5dee070071ad
Reviewed-on: https://chromium-review.googlesource.com/615463
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494534}
[modify] https://crrev.com/43564b01a2a7542baa822e9e62b57873d2bf2110/chrome/browser/chromeos/login/session/user_session_manager.cc
[modify] https://crrev.com/43564b01a2a7542baa822e9e62b57873d2bf2110/chrome/browser/chromeos/login/session/user_session_manager.h

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 16 2017

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

commit cc49b6663418d528beb03ec95637f3d5c07c520d
Author: Helen Li <xunjieli@chromium.org>
Date: Wed Aug 16 17:56:45 2017

Use NetworkChangeObserver in data_reduction_proxy_config.cc

ConnectionTypeObserver is being deprecated. This CL migrates
data_reduction_proxy_config.cc to using the replacement, NetworkChangeObserver.
NetworkChangeObserver's signals are less noisy because they come after network
connection has been established rather than while it is being established.

For more details, see the linked design doc in the bug below.

Bug:  754695 
Change-Id: I1a9e46e4b397b1974bd14a2a5f0c65adb52b6a11
Reviewed-on: https://chromium-review.googlesource.com/615465
Commit-Queue: Helen Li <xunjieli@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494852}
[modify] https://crrev.com/cc49b6663418d528beb03ec95637f3d5c07c520d/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/cc49b6663418d528beb03ec95637f3d5c07c520d/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/cc49b6663418d528beb03ec95637f3d5c07c520d/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 18 2017

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

commit 5a9908aa69f6cb361cf7ce5af9b60451123faab9
Author: Helen Li <xunjieli@chromium.org>
Date: Fri Aug 18 17:50:16 2017

Change IntranetRedirectDetector to use NetworkChangeObserver

This CL migrates IntranetRedirectDetector to using NetworkChangeObserver instead
of the deprecated IPAddressObserver.

The NetworkChange signal should come at a more useful time for this detector.
The signal will be less noisy because it comes after the network connection
has been established, rather than while it is being established (in the case
of IPAddressObserver).

For more information, please see linked design doc in the bug below.

Bug:  754695 
Change-Id: I0350ea940b2df5eb858937d1398b1b754853a741
Reviewed-on: https://chromium-review.googlesource.com/611670
Commit-Queue: Helen Li <xunjieli@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495609}
[modify] https://crrev.com/5a9908aa69f6cb361cf7ce5af9b60451123faab9/chrome/browser/intranet_redirect_detector.cc
[modify] https://crrev.com/5a9908aa69f6cb361cf7ce5af9b60451123faab9/chrome/browser/intranet_redirect_detector.h

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 22 2017

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

commit 705f27524a76b857f61d9240f7e401f85fc8e9b1
Author: Helen Li <xunjieli@chromium.org>
Date: Tue Aug 22 12:06:36 2017

Change ResourceRequestAllowedNotifier to use NetworkChangeObserver

This CL migrates ResourceRequestAllowedNotifier to using NetworkChangeObserver
instead of the deprecated ConnectionTypeObserver.

For more information, please see linked design doc in the bug below.

Bug:  754695 
Change-Id: Ie8341e504f66b42554f311731e7c77ab81a1b72a
Reviewed-on: https://chromium-review.googlesource.com/623693
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496283}
[modify] https://crrev.com/705f27524a76b857f61d9240f7e401f85fc8e9b1/components/web_resource/resource_request_allowed_notifier.cc
[modify] https://crrev.com/705f27524a76b857f61d9240f7e401f85fc8e9b1/components/web_resource/resource_request_allowed_notifier.h
[modify] https://crrev.com/705f27524a76b857f61d9240f7e401f85fc8e9b1/components/web_resource/resource_request_allowed_notifier_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 31 2017

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

commit f33bf07a75866800b07c328cd848c438aecb9a22
Author: Helen Li <xunjieli@chromium.org>
Date: Thu Aug 31 16:12:48 2017

Change DataUseAggregator to use NetworkChangeObserver

This CL migrates DataUseAggregator to using NetworkChangeObserver
instead of the deprecated ConnectionTypeObserver.

Note that when device is coming back online, a CONNECTION_NONE will always be
sent before the actual connection type.

For more information, please see linked design doc in the bug below.

Bug:  754695 
Change-Id: I8d167ffa7e5f2efb27caf924803b4ab0460acab7
Reviewed-on: https://chromium-review.googlesource.com/641373
Reviewed-by: Scott Little <sclittle@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498881}
[modify] https://crrev.com/f33bf07a75866800b07c328cd848c438aecb9a22/components/data_usage/core/data_use_aggregator.cc
[modify] https://crrev.com/f33bf07a75866800b07c328cd848c438aecb9a22/components/data_usage/core/data_use_aggregator.h
[modify] https://crrev.com/f33bf07a75866800b07c328cd848c438aecb9a22/components/data_usage/core/data_use_aggregator_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 1 2017

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

commit 4489785ae419771cf2a942fad7f22c68d07c1799
Author: Helen Li <xunjieli@chromium.org>
Date: Fri Sep 01 15:59:58 2017

Change DeviceStatusListener to use NetworkChangeObserver

This CL migrates DeviceStatusListener to using NetworkChangeObserver
instead of the deprecated ConnectionTypeObserver.

Note that when device is coming back online, a CONNECTION_NONE will always be
sent before the actual connection type. The actual connection type is sent after
the network has been established.

For more information, please see linked design doc in the bug below.

Bug:  754695 
Change-Id: Ic092099fa94522f8d6e6434c0ce5bfd6db7d772e
Reviewed-on: https://chromium-review.googlesource.com/641954
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499205}
[modify] https://crrev.com/4489785ae419771cf2a942fad7f22c68d07c1799/components/download/internal/android/network_status_listener_android.cc
[modify] https://crrev.com/4489785ae419771cf2a942fad7f22c68d07c1799/components/download/internal/scheduler/device_status_listener.cc
[modify] https://crrev.com/4489785ae419771cf2a942fad7f22c68d07c1799/components/download/internal/scheduler/device_status_listener.h
[modify] https://crrev.com/4489785ae419771cf2a942fad7f22c68d07c1799/components/download/internal/scheduler/device_status_listener_unittest.cc
[modify] https://crrev.com/4489785ae419771cf2a942fad7f22c68d07c1799/components/download/internal/scheduler/network_status_listener.cc
[modify] https://crrev.com/4489785ae419771cf2a942fad7f22c68d07c1799/components/download/internal/scheduler/network_status_listener.h

Blockedon: 780466
Blockedon: 780467
Blockedon: 780468
Blockedon: 780469
Blockedon: 780470
Blockedon: 780474
Blockedon: 780475
Blockedon: 780480
Project Member

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

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

commit 6fcd0ed320b486eda301770da192542d8c751cda
Author: Helen Li <xunjieli@chromium.org>
Date: Wed Nov 01 20:37:05 2017

Fix documentation in components/google/core/browser/google_url_tracker.h

The overridden method is from NetworkChangeObserver and not from
IPAddressObserver.

TBR=isherman@chromium.org

Bug:  754695 
Change-Id: Ic37fccfce1abc1e89ad6300b652416cd73a6f39b
Reviewed-on: https://chromium-review.googlesource.com/749302
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513253}
[modify] https://crrev.com/6fcd0ed320b486eda301770da192542d8c751cda/components/google/core/browser/google_url_tracker.h

Project Member

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

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

commit 7137772545d02aafc21f3d560c6fb2406b699cb3
Author: Helen Li <xunjieli@chromium.org>
Date: Mon Nov 06 23:10:24 2017

Refactor sync_manager_impl.h to use NetworkChangeObserver

IPAddressObserver and ConnectionTypeObserver are deprecated. This CL
modifies SyncManagerImpl to use NetworkChangeObserver instead.

Note that when device is coming online, a CONNECTION_NONE will always
be sent before the actual connection type, so I modified
OnConnectionStatusChange to take into account the connection type before retry.

Bug:  754695 
Change-Id: I25dd585a84f1c6736546e931f19596e9788b6c15
Reviewed-on: https://chromium-review.googlesource.com/677550
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514278}
[modify] https://crrev.com/7137772545d02aafc21f3d560c6fb2406b699cb3/chrome/browser/sync/test/integration/sync_exponential_backoff_test.cc
[modify] https://crrev.com/7137772545d02aafc21f3d560c6fb2406b699cb3/components/sync/engine_impl/sync_manager_impl.cc
[modify] https://crrev.com/7137772545d02aafc21f3d560c6fb2406b699cb3/components/sync/engine_impl/sync_manager_impl.h
[modify] https://crrev.com/7137772545d02aafc21f3d560c6fb2406b699cb3/components/sync/engine_impl/sync_scheduler.h
[modify] https://crrev.com/7137772545d02aafc21f3d560c6fb2406b699cb3/components/sync/engine_impl/sync_scheduler_impl.cc
[modify] https://crrev.com/7137772545d02aafc21f3d560c6fb2406b699cb3/components/sync/engine_impl/sync_scheduler_impl.h
[modify] https://crrev.com/7137772545d02aafc21f3d560c6fb2406b699cb3/components/sync/engine_impl/sync_scheduler_impl_unittest.cc
[modify] https://crrev.com/7137772545d02aafc21f3d560c6fb2406b699cb3/components/sync/test/engine/fake_sync_scheduler.cc
[modify] https://crrev.com/7137772545d02aafc21f3d560c6fb2406b699cb3/components/sync/test/engine/fake_sync_scheduler.h

Components: Internals>Network>Service
Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
Owner: ----
Project Member

Comment 26 by bugdroid1@chromium.org, Mar 21 2018

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

commit 1410ab6cd7648f285afd3d6c0d6ed23593ff0e97
Author: Yuhong Sha <yuhong.sha@samsung.com>
Date: Wed Mar 21 12:56:33 2018

Change PepperNetworkMonitorHost to use NetworkChangeObserver

NetworkChangeNotifier::IPAddressObserver is being deprecated.
This CL migrates PepperNetworkMonitorHost to use NetworkChangeObserver
instead of the deprecated IPAddressObserver.

For more information, please see linked design doc in the bug below.

Bug:  754695 

Signed-off-by: Yuhong Sha <yuhong.sha@samsung.com>
Change-Id: I2b6e96a1b3a1fcc14edb096401041c28b3052ab1
Reviewed-on: https://chromium-review.googlesource.com/945186
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544675}
[modify] https://crrev.com/1410ab6cd7648f285afd3d6c0d6ed23593ff0e97/content/browser/renderer_host/pepper/pepper_network_monitor_host.cc
[modify] https://crrev.com/1410ab6cd7648f285afd3d6c0d6ed23593ff0e97/content/browser/renderer_host/pepper/pepper_network_monitor_host.h

Comment 27 by dxie@chromium.org, May 22 2018

this doesn't block canary.

Comment 28 by dxie@chromium.org, May 22 2018

Labels: Hotlist-KnownIssue

Comment 29 by mef@chromium.org, Jun 7 2018

Owner: lilyhoughton@chromium.org
Status: Assigned (was: Available)
Please CC me on related CLs.  This is somewhat tricky and I've caught a few problems in the past.
Project Member

Comment 31 by bugdroid1@chromium.org, Jul 17

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

commit d8146f5013ba42fc9933bfb0a8c059d5188e1276
Author: Lily Houghton <lilyhoughton@chromium.org>
Date: Tue Jul 17 04:19:55 2018

Change DataReductionProxy to use NetworkChangeObserver

NetworkChangeNotifier::IPAddress Observer is being deprecated.
This CL migrates DataReductionProxy to use NetworkChangeObserver instead.

Bug:  754695 
Change-Id: I473ad45e88cff01a9f33a183033a9d86f59aef4e
Reviewed-on: https://chromium-review.googlesource.com/1133330
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Lily Houghton <lilyhoughton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575549}
[modify] https://crrev.com/d8146f5013ba42fc9933bfb0a8c059d5188e1276/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.cc
[modify] https://crrev.com/d8146f5013ba42fc9933bfb0a8c059d5188e1276/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.h
[modify] https://crrev.com/d8146f5013ba42fc9933bfb0a8c059d5188e1276/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
[modify] https://crrev.com/d8146f5013ba42fc9933bfb0a8c059d5188e1276/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc
[modify] https://crrev.com/d8146f5013ba42fc9933bfb0a8c059d5188e1276/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.h
[modify] https://crrev.com/d8146f5013ba42fc9933bfb0a8c059d5188e1276/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h

Project Member

Comment 32 by bugdroid1@chromium.org, Jul 17

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

commit 8fba4f87846d8b6788670485f18a276a25e60f05
Author: Lily Houghton <lilyhoughton@chromium.org>
Date: Tue Jul 17 22:11:15 2018

Change CloudPolicyRefreshScheduler to use NetworkChangeObserver.

NetworkChangeNotifier::IPAddressObserver is being deprecated.
This CL migrates CloudPolicyRefreshScheduler to use NetworkChangeObserver
instead.

Bug:  754695 
Change-Id: I4a3c1dd2320151ae78e504d732e6b67cec9bc78b
Reviewed-on: https://chromium-review.googlesource.com/1133333
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Lily Houghton <lilyhoughton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575797}
[modify] https://crrev.com/8fba4f87846d8b6788670485f18a276a25e60f05/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.cc
[modify] https://crrev.com/8fba4f87846d8b6788670485f18a276a25e60f05/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.h
[modify] https://crrev.com/8fba4f87846d8b6788670485f18a276a25e60f05/components/policy/core/common/cloud/cloud_policy_refresh_scheduler_unittest.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Jul 18

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

commit c7577d5d6a35432d7f6f87ae3200f072f31a061f
Author: Lily Houghton <lilyhoughton@chromium.org>
Date: Wed Jul 18 20:31:06 2018

Change AffiliationFetchThrottler to use NetworkChangeObserver

ConnectionTypeObserver is deprecated.  This CL modifies AffiliationFetchThrottler
to use NetworkChangeObserver instead.

For more details, see the linked design doc in the bug below.

Bug:  754695 
Change-Id: I38b53ce245a74c19be41aab2558a9c3e7fc7b1d1
Reviewed-on: https://chromium-review.googlesource.com/1134019
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Lily Houghton <lilyhoughton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576188}
[modify] https://crrev.com/c7577d5d6a35432d7f6f87ae3200f072f31a061f/components/password_manager/core/browser/android_affiliation/affiliation_fetch_throttler.cc
[modify] https://crrev.com/c7577d5d6a35432d7f6f87ae3200f072f31a061f/components/password_manager/core/browser/android_affiliation/affiliation_fetch_throttler.h

Project Member

Comment 34 by bugdroid1@chromium.org, Jul 23

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

commit 2d0fd0cbb64de1dbac1e3586cad2305e41a56f04
Author: Lily Houghton <lilyhoughton@chromium.org>
Date: Mon Jul 23 16:19:12 2018

Replace LoggingNetworkChangeObserver in IOSIOThread with class from net/

IPAddressObserver and ConnectionTypeObserver are deprecated in favor of
NetworkChangeObserver.  This CL replaces a class that subclasses them with
an (almost identical) internal class which does not expose them directly.

For more details see the design doc linked in the bug below.

Bug:  754695 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ic65ce6aa4df57893504e9eaf3042fb2deb355115
Reviewed-on: https://chromium-review.googlesource.com/1133325
Commit-Queue: Lily Houghton <lilyhoughton@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577181}
[modify] https://crrev.com/2d0fd0cbb64de1dbac1e3586cad2305e41a56f04/ios/components/io_thread/ios_io_thread.h
[modify] https://crrev.com/2d0fd0cbb64de1dbac1e3586cad2305e41a56f04/ios/components/io_thread/ios_io_thread.mm

Owner: pauljensen@chromium.org
AFAICT all non-net consumers have moved to NetworkChangeObserver. 

There is https://chromium-review.googlesource.com/c/chromium/src/+/1158616 to make NetworkChangeNotifier::IPAddressObserver and ConnectionTypeObserver private, but I don't know whether this is necessary.

Moving to Paul to decide.
Cc: -pauljensen@chromium.org
Status: Started (was: Assigned)
I patched in http://crrev/c/1158616 and it looks like only components/metrics/net/network_metrics_provider.h uses the deprecated APIs, at least when I built content shell for Android.

I put up http://crrev/c/1161966 to convert network_metrics_provider
Project Member

Comment 37 by bugdroid1@chromium.org, Aug 3

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

commit 4f763482dcaac615d1338c527560fadb016ede60
Author: Paul Jensen <pauljensen@chromium.org>
Date: Fri Aug 03 21:02:13 2018

Convert NetworkMetricsProvider from deprecated ConnectionTypeObserver

Convert it to "new" (2012) NeworkChangeObserver.

Bug:  754695 
Change-Id: Ifc63ddff076da40fd1a18d052b73f03e778cbd56
Reviewed-on: https://chromium-review.googlesource.com/1161966
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580647}
[modify] https://crrev.com/4f763482dcaac615d1338c527560fadb016ede60/components/metrics/net/network_metrics_provider.cc
[modify] https://crrev.com/4f763482dcaac615d1338c527560fadb016ede60/components/metrics/net/network_metrics_provider.h
[modify] https://crrev.com/4f763482dcaac615d1338c527560fadb016ede60/components/metrics/net/network_metrics_provider_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 39 by bugdroid1@chromium.org, Sep 13

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

commit 0b89497ac30beb8de0f4fcfe37ff4f63fc4441de
Author: Wang Hui <wanghui07050707@gmail.com>
Date: Thu Sep 13 18:37:03 2018

Refactor host_resolver_impl.h to use NetworkChangeObserver

Combine the IPAddressChanged and  ConnectionTypeChanged signals

Bug:  754695 

Signed-off-by: Wang Hui <wanghui07050707@gmail.com>
Change-Id: I942eed13c59aa4f1bc021f766f8664abd5920f1b
Reviewed-on: https://chromium-review.googlesource.com/1206550
Reviewed-by: Bence Béky <bnc@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591081}
[modify] https://crrev.com/0b89497ac30beb8de0f4fcfe37ff4f63fc4441de/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/0b89497ac30beb8de0f4fcfe37ff4f63fc4441de/net/dns/host_resolver_impl.h
[modify] https://crrev.com/0b89497ac30beb8de0f4fcfe37ff4f63fc4441de/net/dns/host_resolver_impl_unittest.cc

Project Member

Comment 40 by bugdroid1@chromium.org, Sep 14

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

commit f47bbab27c0de5810f7db4d75c886ff0620d3fe5
Author: Paul Jensen <pauljensen@chromium.org>
Date: Fri Sep 14 16:34:04 2018

Revert "Refactor host_resolver_impl.h to use NetworkChangeObserver"

This reverts commit 0b89497ac30beb8de0f4fcfe37ff4f63fc4441de.

Reason for revert: This is a very sensitive network change observer and I don't think the conversion to OnNetworkChanged was done properly.  OnNetworkChanged isn't a simple conversion, please see the doc in the linked bug or the comment above OnNetworkChanged.  Observers need to be edge triggered.

Original change's description:
> Refactor host_resolver_impl.h to use NetworkChangeObserver
> 
> Combine the IPAddressChanged and  ConnectionTypeChanged signals
> 
> Bug:  754695 
> 
> Signed-off-by: Wang Hui <wanghui07050707@gmail.com>
> Change-Id: I942eed13c59aa4f1bc021f766f8664abd5920f1b
> Reviewed-on: https://chromium-review.googlesource.com/1206550
> Reviewed-by: Bence Béky <bnc@chromium.org>
> Commit-Queue: Bence Béky <bnc@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#591081}

TBR=bnc@chromium.org,wanghui07050707@gmail.com

Change-Id: Iaee4f0b5967da4d9be6efc25611ef3b02d9c22e4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  754695 
Reviewed-on: https://chromium-review.googlesource.com/1226304
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591369}
[modify] https://crrev.com/f47bbab27c0de5810f7db4d75c886ff0620d3fe5/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/f47bbab27c0de5810f7db4d75c886ff0620d3fe5/net/dns/host_resolver_impl.h
[modify] https://crrev.com/f47bbab27c0de5810f7db4d75c886ff0620d3fe5/net/dns/host_resolver_impl_unittest.cc

Sign in to add a comment