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

Issue 761171 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Disconnecting during a connection attempt can still connect to Wi-Fi

Project Member Reported by khorimoto@chromium.org, Aug 31 2017

Issue description

Repro:
(1) Scan for hosts and get at least one.
(2) Go to the network detail page (with the connect button).
(3) Click connect.
(4) After the host has responded over Bluetooth but before the Wi-Fi connection occurs, click disconnect. You have to time it just right to do this, so it may take a few tries.

Expected: Connection does not succeed in any way.

Actual: From the Tether component's point of view, the disconnection went through, and no host is connected. However, the device has successfully connected to the hotspot's Wi-Fi network. Since the Tether component does not think that a connection exists, it will not send KeepAliveTickles, so the phone will turn off its hotspot at some point.
 
prox-tether-logs.txt
30.4 KB View Download
Haha...the bug is caused by a TODO that I wrote for myself and never completed: https://cs.chromium.org/chromium/src/chromeos/components/tether/tether_connector_impl.cc?l=295.
 Issue 761569  has been merged into this issue.
Merged  issue 761569  into this bug. Same bug, slightly different cause.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 8 2017

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

commit 547423ce0303008ef2d342750cc869162e013ece
Author: Kyle Horimoto <khorimoto@google.com>
Date: Fri Sep 08 21:35:58 2017

[CrOS Tether] Handle tether connection edge cases.

This CL covers two similar edge cases:
(1) User cancels a connection attempt just as the connection to the
    underlying Wi-Fi network is about to succeed. Previously, the Wi-Fi
    connection would succeed despite there being no active host; now,
    the connection is killed in this case.
(2) User disables the Tether component (e.g., by turning off Mobile
    data) just as the connection to the underlying Wi-Fi network is
    about to succeed. Similarly, the connection no longer succeeds.

With this CL, the network becomes disconnected and a
DisconnectTetheringRequest is sent to the Tether host to tell it to
turn off its hotspot in the above cases.

Note: This CL refactors TetherDisconnectorImpl by removing the Wi-Fi
disconnection code to its own class, WifiHotspotDisconnector.

Bug:  761171 , 672263
Change-Id: I16d6a1cba408cc94ce35322f8973fbdcb2a5c219
Reviewed-on: https://chromium-review.googlesource.com/655938
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500695}
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chrome/browser/chromeos/tether/tether_service.cc
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/BUILD.gn
[add] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/fake_wifi_hotspot_disconnector.cc
[add] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/fake_wifi_hotspot_disconnector.h
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/initializer_impl.cc
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/initializer_impl.h
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/tether_connector_impl.cc
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/tether_connector_impl.h
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/tether_connector_impl_unittest.cc
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/tether_disconnector_impl.cc
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/tether_disconnector_impl.h
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/tether_disconnector_impl_unittest.cc
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/wifi_hotspot_connector.cc
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/wifi_hotspot_connector.h
[modify] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/wifi_hotspot_connector_unittest.cc
[add] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/wifi_hotspot_disconnector.h
[add] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/wifi_hotspot_disconnector_impl.cc
[add] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/wifi_hotspot_disconnector_impl.h
[add] https://crrev.com/547423ce0303008ef2d342750cc869162e013ece/chromeos/components/tether/wifi_hotspot_disconnector_impl_unittest.cc

Labels: Merge-Request-61
Project Member

Comment 7 by sheriffbot@chromium.org, Sep 8 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

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

Comment 8 by ketakid@google.com, Sep 11 2017

Labels: -Merge-Review-61 Merge-Approved-61 Merge-Approved-62
approving merge to M61 and M62. Please merge to M62 first and post validation merge to M61.
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 11 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fc4103ff46ebfd282ac4994984adc54ad437e4d9

commit fc4103ff46ebfd282ac4994984adc54ad437e4d9
Author: Kyle Horimoto <khorimoto@google.com>
Date: Mon Sep 11 18:23:39 2017

[CrOS Tether] Handle tether connection edge cases.

This CL covers two similar edge cases:
(1) User cancels a connection attempt just as the connection to the
    underlying Wi-Fi network is about to succeed. Previously, the Wi-Fi
    connection would succeed despite there being no active host; now,
    the connection is killed in this case.
(2) User disables the Tether component (e.g., by turning off Mobile
    data) just as the connection to the underlying Wi-Fi network is
    about to succeed. Similarly, the connection no longer succeeds.

With this CL, the network becomes disconnected and a
DisconnectTetheringRequest is sent to the Tether host to tell it to
turn off its hotspot in the above cases.

Note: This CL refactors TetherDisconnectorImpl by removing the Wi-Fi
disconnection code to its own class, WifiHotspotDisconnector.

TBR=khorimoto@google.com

(cherry picked from commit 547423ce0303008ef2d342750cc869162e013ece)

Bug:  761171 , 672263
Change-Id: I16d6a1cba408cc94ce35322f8973fbdcb2a5c219
Reviewed-on: https://chromium-review.googlesource.com/655938
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#500695}
Reviewed-on: https://chromium-review.googlesource.com/660414
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#1161}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chrome/browser/chromeos/tether/tether_service.cc
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/BUILD.gn
[add] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/fake_wifi_hotspot_disconnector.cc
[add] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/fake_wifi_hotspot_disconnector.h
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/initializer_impl.cc
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/initializer_impl.h
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/tether_connector_impl.cc
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/tether_connector_impl.h
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/tether_connector_impl_unittest.cc
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/tether_disconnector_impl.cc
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/tether_disconnector_impl.h
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/tether_disconnector_impl_unittest.cc
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/wifi_hotspot_connector.cc
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/wifi_hotspot_connector.h
[modify] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/wifi_hotspot_connector_unittest.cc
[add] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/wifi_hotspot_disconnector.h
[add] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/wifi_hotspot_disconnector_impl.cc
[add] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/wifi_hotspot_disconnector_impl.h
[add] https://crrev.com/fc4103ff46ebfd282ac4994984adc54ad437e4d9/chromeos/components/tether/wifi_hotspot_disconnector_impl_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 11 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/190e9fca59e449854dd04de96208725f02281b9a

commit 190e9fca59e449854dd04de96208725f02281b9a
Author: Kyle Horimoto <khorimoto@google.com>
Date: Mon Sep 11 18:25:29 2017

[CrOS Tether] Handle tether connection edge cases.

This CL covers two similar edge cases:
(1) User cancels a connection attempt just as the connection to the
    underlying Wi-Fi network is about to succeed. Previously, the Wi-Fi
    connection would succeed despite there being no active host; now,
    the connection is killed in this case.
(2) User disables the Tether component (e.g., by turning off Mobile
    data) just as the connection to the underlying Wi-Fi network is
    about to succeed. Similarly, the connection no longer succeeds.

With this CL, the network becomes disconnected and a
DisconnectTetheringRequest is sent to the Tether host to tell it to
turn off its hotspot in the above cases.

Note: This CL refactors TetherDisconnectorImpl by removing the Wi-Fi
disconnection code to its own class, WifiHotspotDisconnector.

TBR=khorimoto@google.com

(cherry picked from commit 547423ce0303008ef2d342750cc869162e013ece)

Bug:  761171 , 672263
Change-Id: I16d6a1cba408cc94ce35322f8973fbdcb2a5c219
Reviewed-on: https://chromium-review.googlesource.com/655938
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#500695}
Reviewed-on: https://chromium-review.googlesource.com/660416
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#139}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chrome/browser/chromeos/tether/tether_service.cc
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/BUILD.gn
[add] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/fake_wifi_hotspot_disconnector.cc
[add] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/fake_wifi_hotspot_disconnector.h
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/initializer_impl.cc
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/initializer_impl.h
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/tether_connector_impl.cc
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/tether_connector_impl.h
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/tether_connector_impl_unittest.cc
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/tether_disconnector_impl.cc
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/tether_disconnector_impl.h
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/tether_disconnector_impl_unittest.cc
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/wifi_hotspot_connector.cc
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/wifi_hotspot_connector.h
[modify] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/wifi_hotspot_connector_unittest.cc
[add] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/wifi_hotspot_disconnector.h
[add] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/wifi_hotspot_disconnector_impl.cc
[add] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/wifi_hotspot_disconnector_impl.h
[add] https://crrev.com/190e9fca59e449854dd04de96208725f02281b9a/chromeos/components/tether/wifi_hotspot_disconnector_impl_unittest.cc

Status: Fixed (was: Started)
 Issue 762183  has been merged into this issue.

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

Status: Archived (was: Fixed)

Comment 14 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment