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

Issue 759137 link

Starred by 1 user
Status: Archived
Owner:
khorimoto@chromium.org
Closed: Aug 2017
Cc:
█ jlklein@chromium.org
█ lesliewatkins@chromium.org
khorimoto@chromium.org
jhawkins@chromium.org
hansberry@chromium.org
█ jonmann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

"Available host" notification should be removed when user starts connecting to network

Project Member Reported by khorimoto@chromium.org, Aug 25 2017 
Currently, it is removed when the user *finishes* connecting to a network.

We currently listen for DefaultNetworkChanged() events (see [1]). Instead, we need to take the same approach used in  issue 757556 : observe NetworkStateHandlerObserver::NetworkConnectionStateChanged(), then on any change call NSH::FirstNetworkByType(NetworkTypePattern::Default()) to check the state of the first network. If there is a connected or connecting network, it will always be the first network.

[1] https://cs.chromium.org/chromium/src/chromeos/components/tether/notification_remover.h?q=DefaultNetworkChanged
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 29 2017 

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

commit e088d8a86a53aee3a1d3537ed07166dcf914443d
Author: Kyle Horimoto <khorimoto@google.com>
Date: Tue Aug 29 19:28:50 2017

[CrOS Tether] Changes to the "available hotspot" notification.

(1) Do not show that notification while connecting to another network.
(2) Do not show that notification if it has already been shown during
    this session and was already closed.
(3) Fix an issue in which NSH::FirstNetworkByType(Default()) would
    return the first Tether network, even if that network was not
    connecting/connected and there also existed a connecting/connected
    network of another type.

Bug:  759137 ,  759078 , 672263
Change-Id: Id59cfc00ebebdda7abc1b64585a0b9cfd4947c63
Reviewed-on: https://chromium-review.googlesource.com/636296
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498197}
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chrome/browser/chromeos/net/tether_notification_presenter.cc
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chrome/browser/chromeos/net/tether_notification_presenter.h
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chrome/browser/chromeos/net/tether_notification_presenter_unittest.cc
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/components/tether/fake_notification_presenter.cc
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/components/tether/fake_notification_presenter.h
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/components/tether/host_scanner.cc
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/components/tether/host_scanner.h
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/components/tether/host_scanner_unittest.cc
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/components/tether/notification_presenter.h
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/components/tether/notification_remover.cc
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/components/tether/notification_remover.h
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/components/tether/notification_remover_unittest.cc
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/network/network_state.cc
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/network/network_state.h
[modify] https://crrev.com/e088d8a86a53aee3a1d3537ed07166dcf914443d/chromeos/network/network_state_handler.cc

Comment 3 by khorimoto@chromium.org, Aug 29 2017

Labels: Merge-Request-61
Project Member

Comment 4 by sheriffbot@chromium.org, Aug 29 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: We are only 6 days from stable.
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 5 by keta...@chromium.org, Aug 29 2017

Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 29 2017

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

commit 13289a5e55ca73e2032379ae84928f43aecc0a4d
Author: Kyle Horimoto <khorimoto@google.com>
Date: Tue Aug 29 21:30:54 2017

[CrOS Tether] Changes to the "available hotspot" notification.

(1) Do not show that notification while connecting to another network.
(2) Do not show that notification if it has already been shown during
    this session and was already closed.
(3) Fix an issue in which NSH::FirstNetworkByType(Default()) would
    return the first Tether network, even if that network was not
    connecting/connected and there also existed a connecting/connected
    network of another type.

TBR=khorimoto@google.com

(cherry picked from commit e088d8a86a53aee3a1d3537ed07166dcf914443d)

Bug:  759137 ,  759078 , 672263
Change-Id: Id59cfc00ebebdda7abc1b64585a0b9cfd4947c63
Reviewed-on: https://chromium-review.googlesource.com/636296
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#498197}
Reviewed-on: https://chromium-review.googlesource.com/642055
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#981}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chrome/browser/chromeos/net/tether_notification_presenter.cc
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chrome/browser/chromeos/net/tether_notification_presenter.h
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chrome/browser/chromeos/net/tether_notification_presenter_unittest.cc
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/components/tether/fake_notification_presenter.cc
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/components/tether/fake_notification_presenter.h
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/components/tether/host_scanner.cc
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/components/tether/host_scanner.h
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/components/tether/host_scanner_unittest.cc
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/components/tether/notification_presenter.h
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/components/tether/notification_remover.cc
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/components/tether/notification_remover.h
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/components/tether/notification_remover_unittest.cc
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/network/network_state.cc
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/network/network_state.h
[modify] https://crrev.com/13289a5e55ca73e2032379ae84928f43aecc0a4d/chromeos/network/network_state_handler.cc

Comment 7 by khorimoto@chromium.org, Aug 29 2017

Status: Fixed (was: Started)

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

Status: Archived (was: Fixed)
►

Sign in to add a comment