New issue
Advanced search Search tips

Issue 883121 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 821009



Sign in to add a comment

Migrate metrics::NetworkMetricsProvider to NetworkConnectionTracker

Project Member Reported by rmcelrath@chromium.org, Sep 11

Issue description

metrics::NetworkMetricsProvider currently uses net::NetworkChangeNotifier to receive network changes. 

With network service, that will need to be converted to using NetworkConnectionTracker's observer APIs.
 
Status: Started (was: Assigned)
Labels: -Proj-Servicification-Stable Hotlist-KnownIssue
Cc: rmcelrath@chromium.org
Owner: juncai@chromium.org
Status: Assigned (was: Started)
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 6

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

commit cdc666840aaacc4eef1cb81855f6c41f84d5d37b
Author: Jun Cai <juncai@chromium.org>
Date: Tue Nov 06 02:02:18 2018

Network Service: Migrate NetworkMetricsProvider to NetworkConnectionTracker

This CL migrates NetworkMetricsProvider from NetworkChangeNotifier to
NetworkConnectionTracker, which works with the network service enabled.

The "Initial upload" patch of this CL is the same as:
https://chromium-review.googlesource.com/c/chromium/src/+/1222918

Bug:  883121 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo
Change-Id: Ic6879e48b9515d2281dc9da7c9437bad4c044a99
Reviewed-on: https://chromium-review.googlesource.com/c/1274585
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Tao Bai <michaelbai@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Jun Cai <juncai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605566}
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/android_webview/browser/aw_metrics_service_client.cc
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/chrome/browser/metrics/chrome_metrics_service_client.cc
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/chromecast/browser/metrics/cast_metrics_service_client.cc
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/components/metrics/BUILD.gn
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/components/metrics/net/DEPS
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/components/metrics/net/network_metrics_provider.cc
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/components/metrics/net/network_metrics_provider.h
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/components/metrics/net/network_metrics_provider_unittest.cc
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/content/browser/network_service_instance.cc
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/content/public/browser/network_service_instance.h
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/ios/chrome/browser/metrics/ios_chrome_metrics_service_client.mm
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/services/network/public/cpp/network_connection_tracker.h
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/services/network/test/test_network_connection_tracker.cc
[modify] https://crrev.com/cdc666840aaacc4eef1cb81855f6c41f84d5d37b/services/network/test/test_network_connection_tracker.h

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 6

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

commit 05b5296a2b1916017d27b5671432bc42621de72d
Author: Rayan Kanso <rayankans@chromium.org>
Date: Tue Nov 06 14:04:32 2018

Revert "Network Service: Migrate NetworkMetricsProvider to NetworkConnectionTracker"

This reverts commit cdc666840aaacc4eef1cb81855f6c41f84d5d37b.

Reason for revert: Broke the build
https://logs.chromium.org/logs/chromium/bb/chromium.android/Android_Cronet_ARM64_Builder__dbg_/99962/+/recipes/steps/compile/0/stdout

Sample build error log:
ld.lld: error: undefined symbol: net::websockets::kWebSocketGuid
>>> referenced by web_socket.cc:81 (../../services/network/public/cpp/server/web_socket.cc:81)

Original change's description:
> Network Service: Migrate NetworkMetricsProvider to NetworkConnectionTracker
> 
> This CL migrates NetworkMetricsProvider from NetworkChangeNotifier to
> NetworkConnectionTracker, which works with the network service enabled.
> 
> The "Initial upload" patch of this CL is the same as:
> https://chromium-review.googlesource.com/c/chromium/src/+/1222918
> 
> Bug:  883121 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo
> Change-Id: Ic6879e48b9515d2281dc9da7c9437bad4c044a99
> Reviewed-on: https://chromium-review.googlesource.com/c/1274585
> Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
> Reviewed-by: Tao Bai <michaelbai@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Commit-Queue: Jun Cai <juncai@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#605566}

TBR=michaelbai@chromium.org,jam@chromium.org,fdoray@chromium.org,asvitkine@chromium.org,olivierrobin@chromium.org,juncai@chromium.org,rmcelrath@chromium.org

Change-Id: I49a7da919b912cf33bc0f0f06b9bf419080a968f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  883121 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/c/1319614
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605666}
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/android_webview/browser/aw_metrics_service_client.cc
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/chrome/browser/metrics/chrome_metrics_service_client.cc
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/chromecast/browser/metrics/cast_metrics_service_client.cc
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/components/metrics/BUILD.gn
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/components/metrics/net/DEPS
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/components/metrics/net/network_metrics_provider.cc
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/components/metrics/net/network_metrics_provider.h
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/components/metrics/net/network_metrics_provider_unittest.cc
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/content/browser/network_service_instance.cc
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/content/public/browser/network_service_instance.h
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/ios/chrome/browser/metrics/ios_chrome_metrics_service_client.mm
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/services/network/public/cpp/network_connection_tracker.h
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/services/network/test/test_network_connection_tracker.cc
[modify] https://crrev.com/05b5296a2b1916017d27b5671432bc42621de72d/services/network/test/test_network_connection_tracker.h

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 7

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

commit 0df82fcde5a33197e31ad4f981197b279e7327ec
Author: Jun Cai <juncai@chromium.org>
Date: Wed Nov 07 01:01:28 2018

Reland: Network Service: Migrate NetworkMetricsProvider to NetworkConnectionTracker

The "Initial upload" patch of this CL is the same as:
https://chromium-review.googlesource.com/c/chromium/src/+/1274585

The above original CL was reverted at:
https://chromium-review.googlesource.com/c/chromium/src/+/1319614

The major changes since the "Initial upload" patch set 1:
Removed some unnecessary include files and removed the "//services/network/public/cpp"
dep from "components/metrics/BUILD.gn". These include files and dep caused the
build error Cronet for Android.

TBR=olivierrobin@chromium.org, michaelbai@chromium.org, jam@chromium.org, fdoray@chromium.org, asvitkine@chromium.org

Bug:  883121 
Change-Id: Ifcaf40ea1b72ddc3a6a72be16e4d52190740e5a4
Reviewed-on: https://chromium-review.googlesource.com/c/1320710
Reviewed-by: Jun Cai <juncai@chromium.org>
Commit-Queue: Jun Cai <juncai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605899}
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/android_webview/browser/aw_metrics_service_client.cc
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/chrome/browser/metrics/chrome_metrics_service_client.cc
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/chromecast/browser/metrics/cast_metrics_service_client.cc
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/components/metrics/BUILD.gn
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/components/metrics/metrics_service.h
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/components/metrics/net/DEPS
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/components/metrics/net/network_metrics_provider.cc
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/components/metrics/net/network_metrics_provider.h
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/components/metrics/net/network_metrics_provider_unittest.cc
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/content/browser/network_service_instance.cc
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/content/public/browser/network_service_instance.h
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/ios/chrome/browser/metrics/ios_chrome_metrics_service_client.mm
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/services/network/public/cpp/network_connection_tracker.h
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/services/network/test/test_network_connection_tracker.cc
[modify] https://crrev.com/0df82fcde5a33197e31ad4f981197b279e7327ec/services/network/test/test_network_connection_tracker.h

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 7

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

commit fc45269c31186c63cfa2b99921593806b9b54c36
Author: Jun Cai <juncai@chromium.org>
Date: Wed Nov 07 19:48:16 2018

Revert "Reland: Network Service: Migrate NetworkMetricsProvider to NetworkConnectionTracker"

This reverts commit 0df82fcde5a33197e31ad4f981197b279e7327ec.

Reason for revert: The Builder linux64 trunk keeps failing due to ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange browser test.
https://bugs.chromium.org/p/chromium/issues/detail?id=902784

Bug:  902784 ,  883121 

Original change's description:
> Reland: Network Service: Migrate NetworkMetricsProvider to NetworkConnectionTracker
> 
> The "Initial upload" patch of this CL is the same as:
> https://chromium-review.googlesource.com/c/chromium/src/+/1274585
> 
> The above original CL was reverted at:
> https://chromium-review.googlesource.com/c/chromium/src/+/1319614
> 
> The major changes since the "Initial upload" patch set 1:
> Removed some unnecessary include files and removed the "//services/network/public/cpp"
> dep from "components/metrics/BUILD.gn". These include files and dep caused the
> build error Cronet for Android.
> 
> TBR=olivierrobin@chromium.org, michaelbai@chromium.org, jam@chromium.org, fdoray@chromium.org, asvitkine@chromium.org
> 
> Bug:  883121 
> Change-Id: Ifcaf40ea1b72ddc3a6a72be16e4d52190740e5a4
> Reviewed-on: https://chromium-review.googlesource.com/c/1320710
> Reviewed-by: Jun Cai <juncai@chromium.org>
> Commit-Queue: Jun Cai <juncai@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#605899}

TBR=michaelbai@chromium.org,jam@chromium.org,fdoray@chromium.org,asvitkine@chromium.org,olivierrobin@chromium.org,juncai@chromium.org

Change-Id: I37832c49552e6d163439d719d5820e1811ccadcb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  883121 
Reviewed-on: https://chromium-review.googlesource.com/c/1324104
Reviewed-by: Jun Cai <juncai@chromium.org>
Commit-Queue: Jun Cai <juncai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606136}
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/android_webview/browser/aw_metrics_service_client.cc
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/chrome/browser/metrics/chrome_metrics_service_client.cc
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/chromecast/browser/metrics/cast_metrics_service_client.cc
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/components/metrics/BUILD.gn
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/components/metrics/metrics_service.h
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/components/metrics/net/DEPS
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/components/metrics/net/network_metrics_provider.cc
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/components/metrics/net/network_metrics_provider.h
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/components/metrics/net/network_metrics_provider_unittest.cc
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/content/browser/network_service_instance.cc
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/content/public/browser/network_service_instance.h
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/ios/chrome/browser/metrics/ios_chrome_metrics_service_client.mm
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/services/network/public/cpp/network_connection_tracker.h
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/services/network/test/test_network_connection_tracker.cc
[modify] https://crrev.com/fc45269c31186c63cfa2b99921593806b9b54c36/services/network/test/test_network_connection_tracker.h

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 15

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

commit 597752e4e042729863024e55e42412f4053dee8b
Author: Jun Cai <juncai@chromium.org>
Date: Thu Nov 15 19:54:25 2018

Reland: Reland: Network Service: Migrate NetworkMetricsProvider to NetworkConnectionTracker

The "Initial upload" patch of this CL is the same as:
https://chromium-review.googlesource.com/c/chromium/src/+/1320710

The above original CL was reverted at:
https://chromium-review.googlesource.com/c/chromium/src/+/1324104

The revert is caused by a browser test failure:
ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange
and more details can be found in the following bug link:
https://bugs.chromium.org/p/chromium/issues/detail?id=902784

The browser test failure is fixed at:
https://chromium-review.googlesource.com/c/chromium/src/+/1336433

So this CL is exactly the same as the "Initial upload".

TBR=olivierrobin@chromium.org, michaelbai@chromium.org, jam@chromium.org, fdoray@chromium.org, asvitkine@chromium.org

Bug:  883121 
Change-Id: I253764764b409e7c6d1522fd5e5d44927c3f3c60
Reviewed-on: https://chromium-review.googlesource.com/c/1323658
Commit-Queue: Jun Cai <juncai@chromium.org>
Reviewed-by: Jun Cai <juncai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608481}
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/android_webview/browser/aw_metrics_service_client.cc
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/chrome/browser/metrics/chrome_metrics_service_client.cc
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/chromecast/browser/metrics/cast_metrics_service_client.cc
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/components/metrics/BUILD.gn
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/components/metrics/metrics_service.h
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/components/metrics/net/DEPS
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/components/metrics/net/network_metrics_provider.cc
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/components/metrics/net/network_metrics_provider.h
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/components/metrics/net/network_metrics_provider_unittest.cc
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/content/browser/network_service_instance.cc
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/content/public/browser/network_service_instance.h
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/ios/chrome/browser/metrics/ios_chrome_metrics_service_client.mm
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/services/network/public/cpp/network_connection_tracker.h
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/services/network/test/test_network_connection_tracker.cc
[modify] https://crrev.com/597752e4e042729863024e55e42412f4053dee8b/services/network/test/test_network_connection_tracker.h

Status: Fixed (was: Started)

Sign in to add a comment