New issue
Advanced search Search tips

Issue 887040 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 821009



Sign in to add a comment

Migrate chromeos::assistant::NetworkProviderImpl to NetworkConnectionTracker

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

Issue description

chromeos::assistant::NetworkProviderImpl currently uses net::NetworkChangeNotifier to receive network changes. 

With network service, that will need to be converted to using NetworkConnectionTracker's observer APIs.
 
Owner: rmcelrath@chromium.org
Status: Started (was: Available)
Cc: rmcelrath@chromium.org
Owner: juncai@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 11

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

commit a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a
Author: Jun Cai <juncai@chromium.org>
Date: Thu Oct 11 01:25:50 2018

Network Service: Migrate chromeos::assistant::NetworkProviderImpl to NetworkConnectionTracker

This migrates chromeos::assistant::NetworkProviderImpl from NetworkChangeNotifier to
NetworkConnectionTracker, which works with the network service enabled.

Bug:  887040 
Change-Id: I3099af12316db87a2d641a01eb27a6d9b08acfee
Reviewed-on: https://chromium-review.googlesource.com/c/1270950
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Commit-Queue: Jun Cai <juncai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598620}
[modify] https://crrev.com/a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a/chromeos/services/assistant/assistant_manager_service_impl.cc
[modify] https://crrev.com/a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a/chromeos/services/assistant/assistant_manager_service_impl.h
[modify] https://crrev.com/a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a/chromeos/services/assistant/platform/network_provider_impl.cc
[modify] https://crrev.com/a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a/chromeos/services/assistant/platform/network_provider_impl.h
[modify] https://crrev.com/a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a/chromeos/services/assistant/platform_api_impl.cc
[modify] https://crrev.com/a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a/chromeos/services/assistant/platform_api_impl.h
[modify] https://crrev.com/a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a/chromeos/services/assistant/service.cc
[modify] https://crrev.com/a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a/chromeos/services/assistant/service.h
[modify] https://crrev.com/a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a/chromeos/services/assistant/service_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 11

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

commit 44b736e81aa7494fcb2f8e7319c5c63bfaa756d6
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu Oct 11 03:59:33 2018

Revert "Network Service: Migrate chromeos::assistant::NetworkProviderImpl to NetworkConnectionTracker"

This reverts commit a918dc1f082d2269c8c9e2a71f59fcc5a87ad53a.

Reason for revert:
This causes compile failures:
https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20ChromeOS/57529

See also: https://chromium-review.googlesource.com/c/chromium/src/+/1270950#message-09331dc2fd155dd240b938c70ae0f88b1f2d93fb

Original change's description:
> Network Service: Migrate chromeos::assistant::NetworkProviderImpl to NetworkConnectionTracker
> 
> This migrates chromeos::assistant::NetworkProviderImpl from NetworkChangeNotifier to
> NetworkConnectionTracker, which works with the network service enabled.
> 
> Bug:  887040 
> Change-Id: I3099af12316db87a2d641a01eb27a6d9b08acfee
> Reviewed-on: https://chromium-review.googlesource.com/c/1270950
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> Commit-Queue: Jun Cai <juncai@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598620}

TBR=stevenjb@chromium.org,jam@chromium.org,xiaohuic@chromium.org,juncai@chromium.org,rmcelrath@chromium.org

Change-Id: I37fad10137de8eb06a9a69486f779d7307da8f00
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  887040 
Reviewed-on: https://chromium-review.googlesource.com/c/1275488
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598663}
[modify] https://crrev.com/44b736e81aa7494fcb2f8e7319c5c63bfaa756d6/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/44b736e81aa7494fcb2f8e7319c5c63bfaa756d6/chromeos/services/assistant/assistant_manager_service_impl.cc
[modify] https://crrev.com/44b736e81aa7494fcb2f8e7319c5c63bfaa756d6/chromeos/services/assistant/assistant_manager_service_impl.h
[modify] https://crrev.com/44b736e81aa7494fcb2f8e7319c5c63bfaa756d6/chromeos/services/assistant/platform/network_provider_impl.cc
[modify] https://crrev.com/44b736e81aa7494fcb2f8e7319c5c63bfaa756d6/chromeos/services/assistant/platform/network_provider_impl.h
[modify] https://crrev.com/44b736e81aa7494fcb2f8e7319c5c63bfaa756d6/chromeos/services/assistant/platform_api_impl.cc
[modify] https://crrev.com/44b736e81aa7494fcb2f8e7319c5c63bfaa756d6/chromeos/services/assistant/platform_api_impl.h
[modify] https://crrev.com/44b736e81aa7494fcb2f8e7319c5c63bfaa756d6/chromeos/services/assistant/service.cc
[modify] https://crrev.com/44b736e81aa7494fcb2f8e7319c5c63bfaa756d6/chromeos/services/assistant/service.h
[modify] https://crrev.com/44b736e81aa7494fcb2f8e7319c5c63bfaa756d6/chromeos/services/assistant/service_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 12

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

commit e5128829431a529499fe3b9de8ca4345a080da84
Author: Jun Cai <juncai@chromium.org>
Date: Fri Oct 12 00:51:32 2018

Reland: Network Service: Migrate chromeos::assistant::NetworkProviderImpl to NetworkConnectionTracker

The original CL is:
https://chromium-review.googlesource.com/c/chromium/src/+/1270950
and the Initial upload is the same as the original CL.

The reland CL fixes compile error on chromeos.

TBR=jam@chromium.org, stevenjb@chromium.org, xiaohuic@chromium.org, rmcelrath@chromium.org

Bug:  887040 
Change-Id: Ic0cf9d07f7c7d6c5c59694c1414bc38b83a36456
Reviewed-on: https://chromium-review.googlesource.com/c/1277647
Reviewed-by: Jun Cai <juncai@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Jun Cai <juncai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599042}
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chromeos/services/assistant/BUILD.gn
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chromeos/services/assistant/DEPS
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chromeos/services/assistant/assistant_manager_service_impl.cc
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chromeos/services/assistant/assistant_manager_service_impl.h
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chromeos/services/assistant/platform/network_provider_impl.cc
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chromeos/services/assistant/platform/network_provider_impl.h
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chromeos/services/assistant/platform_api_impl.cc
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chromeos/services/assistant/platform_api_impl.h
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chromeos/services/assistant/service.cc
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chromeos/services/assistant/service.h
[modify] https://crrev.com/e5128829431a529499fe3b9de8ca4345a080da84/chromeos/services/assistant/service_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment