New issue
Advanced search Search tips

Issue 887056 link

Starred by 1 user
Status: Assigned
Owner:
rmcelrath@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 821009



Sign in to add a comment

Migrate WaitForNetworkCallbackHelper to NetworkConnectionTracker

Project Member Reported by rmcelrath@chromium.org, Sep 19 
WaitForNetworkCallbackHelper currently uses net::NetworkChangeNotifier to be notified of network connection changes. It should be converted to use NetworkConnectionTracker, which will work when the network service is enabled.

Comment 1 by rmcelrath@chromium.org, Sep 21

Owner: rmcelrath@chromium.org
Status: Started (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 3 

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

commit dc2a716c2459cae47dfb1916b55e3c7bb6417c2c
Author: Robbie McElrath <rmcelrath@chromium.org>
Date: Wed Oct 03 18:24:00 2018

Migrate WaitForNetworkCallbackHelper to NetworkConnectionTracker

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

Bug: 887056
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I70cc4863630ffbbd808ed91f5b4cee7dcc003296
Reviewed-on: https://chromium-review.googlesource.com/1239261
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596291}
[modify] https://crrev.com/dc2a716c2459cae47dfb1916b55e3c7bb6417c2c/components/signin/ios/DEPS
[modify] https://crrev.com/dc2a716c2459cae47dfb1916b55e3c7bb6417c2c/components/signin/ios/browser/wait_for_network_callback_helper.cc
[modify] https://crrev.com/dc2a716c2459cae47dfb1916b55e3c7bb6417c2c/components/signin/ios/browser/wait_for_network_callback_helper.h
[modify] https://crrev.com/dc2a716c2459cae47dfb1916b55e3c7bb6417c2c/components/signin/ios/browser/wait_for_network_callback_helper_unittest.cc
[modify] https://crrev.com/dc2a716c2459cae47dfb1916b55e3c7bb6417c2c/ios/chrome/browser/signin/ios_chrome_signin_client.mm
[modify] https://crrev.com/dc2a716c2459cae47dfb1916b55e3c7bb6417c2c/ios/web_view/internal/signin/ios_web_view_signin_client.mm

Comment 3 by rmcelrath@chromium.org, Oct 3

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 10 

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

commit edc8a2fd97e135e052c4af03f8fe41ea2ec647d2
Author: Robbie McElrath <rmcelrath@chromium.org>
Date: Wed Oct 10 18:48:12 2018

Revert "Migrate WaitForNetworkCallbackHelper to NetworkConnectionTracker"

This reverts commit dc2a716c2459cae47dfb1916b55e3c7bb6417c2c.

Reason for revert: This breaks ios sign in. crbug.com/893565

Original change's description:
> Migrate WaitForNetworkCallbackHelper to NetworkConnectionTracker
> 
> This migrates WaitForNetworkCallbackHelper from NetworkChangeNotifier
> to NetworkConnectionTracker, which works with the network service
> enabled.
> 
> Bug: 887056
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
> Change-Id: I70cc4863630ffbbd808ed91f5b4cee7dcc003296
> Reviewed-on: https://chromium-review.googlesource.com/1239261
> Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
> Reviewed-by: Reilly Grant <reillyg@chromium.org>
> Reviewed-by: Eugene But <eugenebut@chromium.org>
> Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#596291}

TBR=reillyg@chromium.org,msarda@chromium.org,eugenebut@chromium.org,rmcelrath@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 887056
Change-Id: I534397f5e47de549f49eef84153a74542cfe711b
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/c/1273989
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598426}
[modify] https://crrev.com/edc8a2fd97e135e052c4af03f8fe41ea2ec647d2/components/signin/ios/DEPS
[modify] https://crrev.com/edc8a2fd97e135e052c4af03f8fe41ea2ec647d2/components/signin/ios/browser/wait_for_network_callback_helper.cc
[modify] https://crrev.com/edc8a2fd97e135e052c4af03f8fe41ea2ec647d2/components/signin/ios/browser/wait_for_network_callback_helper.h
[modify] https://crrev.com/edc8a2fd97e135e052c4af03f8fe41ea2ec647d2/components/signin/ios/browser/wait_for_network_callback_helper_unittest.cc
[modify] https://crrev.com/edc8a2fd97e135e052c4af03f8fe41ea2ec647d2/ios/chrome/browser/signin/ios_chrome_signin_client.mm
[modify] https://crrev.com/edc8a2fd97e135e052c4af03f8fe41ea2ec647d2/ios/web_view/internal/signin/ios_web_view_signin_client.mm

Comment 5 by srikanthg@chromium.org, Oct 12

Status: Assigned (was: Fixed)
Reopening as the fix is reverted.
►

Sign in to add a comment