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

Issue 662571 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

[UI] The first attempt to connect after adding VPN / Hidden Networks fails.

Project Member Reported by aashuto...@chromium.org, Nov 4 2016

Issue description

Chrome Version: <From about:version: Google Chrome 56.0.2908.0>
Chrome OS Version: <From about:version: Platform  8959.0.0>
Chrome OS Platform: <Kevin/ELM/SAMUS>
Network info: <VPN, Marvell>

Please specify Cr-* of the system to which this bug/feature applies (add
the label below).

Steps To Reproduce:
(1) Open chrome://settings, Add details for a private network and click connect.

Expected Result:
UI should try to make connection and show the current status

Actual Result:
UI VPN dialog disappears immediately after hitting the connect button and there 
is no connection attempt. However, the private network is listed in the preferred list and connecting now works a
as expected  

How frequently does this problem reproduce? (Always, sometimes, hard to
reproduce?)
Always.  M-56 TOT,

What is the impact to the user, and is there a workaround? If so, what is
it?
Impact: need to go through the list and try connecting again. 

Please provide any additional information below. Attach a screen shot or
log if possible.
Logs attached.
 
log-110416-154533.tar.gz
946 KB Download
Cc: harpreet@chromium.org
Labels: -Type-Bug Type-Bug-Regression
Summary: [UI] The first attempt to connect after adding VPN / Hidden Networks fails. (was: [UI] [VPN] The first attempt to connect after adding VPN fails. )
Owner: steve...@chromium.org
Status: Assigned (was: Untriaged)
Status: Started (was: Assigned)
I can reproduce this. Investigating.

Ugh, we have the same problem when we fail to connect to a wifi network, sigh. I wonder when / how this regressed.

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 10 2017

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

commit 4be001fc71af06d07cb05c30e4b0665efadefed5
Author: stevenjb <stevenjb@chromium.org>
Date: Fri Feb 10 23:39:08 2017

Defer NetworkConfigurationHandler::CreateShillConfiguration callback

Currently the code calls the NetworkConfigurationHandler
::CreateShillConfiguration callback immediately, which is only
valid if Shill updates Chrome with the new network configuration
in the same frame as the configuration was applied. At some point
the Shill behavior Chrome, breaking the (fragile) Chrome network
cofiguration layer. This CL defers the configuration success
callback until the NetworkStateHandler has been updated with
the new configuration (allowing a subsequent connection attempt
to succeed).

This CL also replaces SupportsWeakPtr with the more reliable WeakPtrFactory (apologies for the churn).

BUG= 662571 

Review-Url: https://codereview.chromium.org/2679423008
Cr-Commit-Position: refs/heads/master@{#449787}

[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chrome/browser/chromeos/net/network_throttling_observer_unittest.cc
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/auto_connect_handler_unittest.cc
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/client_cert_resolver_unittest.cc
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/host_resolver_impl_chromeos_unittest.cc
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/managed_network_configuration_handler_unittest.cc
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/network_cert_migrator_unittest.cc
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/network_configuration_handler.cc
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/network_configuration_handler.h
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/network_configuration_handler_unittest.cc
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/network_connection_handler_unittest.cc
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/network_device_handler_unittest.cc
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/network_state_handler_unittest.cc
[modify] https://crrev.com/4be001fc71af06d07cb05c30e4b0665efadefed5/chromeos/network/prohibited_technologies_handler_unittest.cc

Status: Fixed (was: Started)
Labels: -Pri-2 Merge-Request-57 Pri-1
This is actually pretty bad behavior, even if it is not a common use case. I would like to at least merge it to 57.

Project Member

Comment 8 by sheriffbot@chromium.org, Feb 13 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 13 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc15db5aaa57bb3f35137c1773db691af6a1414a

commit dc15db5aaa57bb3f35137c1773db691af6a1414a
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Mon Feb 13 18:48:35 2017

Defer NetworkConfigurationHandler::CreateShillConfiguration callback

Currently the code calls the NetworkConfigurationHandler
::CreateShillConfiguration callback immediately, which is only
valid if Shill updates Chrome with the new network configuration
in the same frame as the configuration was applied. At some point
the Shill behavior Chrome, breaking the (fragile) Chrome network
cofiguration layer. This CL defers the configuration success
callback until the NetworkStateHandler has been updated with
the new configuration (allowing a subsequent connection attempt
to succeed).

This CL also replaces SupportsWeakPtr with the more reliable WeakPtrFactory (apologies for the churn).

BUG= 662571 

Review-Url: https://codereview.chromium.org/2679423008
Cr-Commit-Position: refs/heads/master@{#449787}
(cherry picked from commit 4be001fc71af06d07cb05c30e4b0665efadefed5)

Review-Url: https://codereview.chromium.org/2689223002 .
Cr-Commit-Position: refs/branch-heads/2987@{#479}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chrome/browser/chromeos/net/network_throttling_observer_unittest.cc
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/auto_connect_handler_unittest.cc
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/client_cert_resolver_unittest.cc
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/host_resolver_impl_chromeos_unittest.cc
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/managed_network_configuration_handler_unittest.cc
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/network_cert_migrator_unittest.cc
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/network_configuration_handler.cc
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/network_configuration_handler.h
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/network_configuration_handler_unittest.cc
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/network_connection_handler_unittest.cc
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/network_device_handler_unittest.cc
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/network_state_handler_unittest.cc
[modify] https://crrev.com/dc15db5aaa57bb3f35137c1773db691af6a1414a/chromeos/network/prohibited_technologies_handler_unittest.cc

Cc: -tienchang@chromium.org -steve...@chromium.org aashuto...@chromium.org debayanb@chromium.org

Comment 11 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 13 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment