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

Issue 872473 link

Starred by 0 users

Issue metadata

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



Sign in to add a comment

Handle LAST_PROVISIONING_FAILED and NO_RECEPTION TetherAvailabilityResponses

Project Member Reported by khorimoto@chromium.org, Aug 8

Issue description

See b/110237556 for more details.

Shiba, how should this situation be handled?
 
Labels: Pri-3
Shiba - can you please provide an update here? Thanks!
Owner: jessejames@chromium.org
Talked to Shiba offline, and Jesse is handling this.
Also, can we remove RVG or is it here for a reason?
Labels: -Restrict-View-Google -Pri-3 Pri-2
Removed RVG, and bumped this up to P2 because we should handle this new response code in our protocol.

If we don't fix this bug, it will result in us not showing an eligible tether host when we actually should be showing it. IMO, this isn't P1-worthy since it's an edge case.
Labels: -M-70 M-71
Owner: ----
Status: Available (was: Assigned)
Background:
* To enable tethering on the phone, provisioning is required. In this context, provisioning refers to the task of querying the mobile provider for whether the user has a plan which allows tethering.
* Unfortunately, there is no API which allows the phone to try provisioning without actually attempting to enable the mobile hotspot (this is a weird design decision which should likely change).
* When the Chromebook asks the phone if it can provide tethering, the phone does not know if it will pass provisioning.

Previously, the phone would always respond to a tethering request with TETHER_AVAILABLE. A new change on the Android side now causes the phone to respond with LAST_PROVISIONING_FAILED if the last attempt to run provisioning failed. This does not necessarily mean that the *next* attempt will fail, but it is likely that the next attempt will fail. There are two valid cases were the last check would fail and a new check would not: (1) The device was offline last time it checked, or there was some sort of network error when checking, (2) The user's plan didn't used to support tethering, but the user has recently enabled tethering on their mobile data account.

Jesse and I came up with the following plan:
(1) Before launch, accept the LAST_PROVISIONING_FAILED as equivalent to TETHER_AVAILABLE. This simply maintains the current behavior. A change is needed to maintain current behavior because of the Android-side change. This should be a simple change (see [1]).
(2) Eventually, change the logic such that we no longer display a notification to the user if a LAST_PROVISIONING_FAILED response is sent from the phone to the Chromebook. This makes our feature less spammy, as it will no longer notify the user about an action which is likely to fail.

We should fix the first step of the plan for M-71, and we can fix the second step of the plan at a later time.

[1] https://cs.chromium.org/chromium/src/chromeos/components/tether/host_scanner_operation.cc?q=IsTetheringAvailableWithValidDeviceStatus
Does this solution also consider b/80411125? Or is that outside the scope of this discussion?
Summary: Handle LAST_PROVISIONING_FAILED and NO_RECEPTION TetherAvailabilityResponses (was: Handle LAST_PROVISIONING_FAILED TetherAvailabilityResponse)
Re: comment #8: Sure, we can deal with that as well here. Let's make TETHER_AVAILABLE, SETUP_NEEDED, NO_RECEPTION, and LAST_PROVISIONING_FAILED all "success" responses.
Components: -UI>ProximityAuth UI>Multidevice
Labels: -M-71 M-72
Cc: -lesliewatkins@chromium.org hsuregan@chromium.org nohle@chromium.org
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 1

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

commit 4423cfde6d162b2296a00de2d271fb9fb47ed68e
Author: Regan Hsu <hsuregan@chromium.org>
Date: Sat Dec 01 02:21:29 2018

[CrOS Instant Tethering] Handle LAST_PROVISIONING_FAILED enum

A new change on the Android side now causes the phone to respond with
LAST_PROVISIONING_FAILED if the last attempt to run provisioning failed.
This CL addresses the case for when the device was offline last time it
checked, or there was some sort of network error when checking.

Bug:  872473 
Change-Id: Ia28f9077990a5a991b3968a6da100222940c11be
Reviewed-on: https://chromium-review.googlesource.com/c/1357656
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612909}
[modify] https://crrev.com/4423cfde6d162b2296a00de2d271fb9fb47ed68e/chromeos/components/tether/host_scanner_operation.cc
[modify] https://crrev.com/4423cfde6d162b2296a00de2d271fb9fb47ed68e/chromeos/components/tether/host_scanner_operation_unittest.cc
[modify] https://crrev.com/4423cfde6d162b2296a00de2d271fb9fb47ed68e/chromeos/components/tether/proto/tether.proto

Status: Fixed (was: Available)
Labels: Merge-Request-72
Status: Started (was: Fixed)
Requesting merge to M-72. The fix is very small (~3 lines) and we are confident that it's not risky.

User-facing effect of this issue is as follows:
(1) User tries to use Instant Tethering, and phone does not have a connection to the Internet (e.g., reception is spotty).
(2) When the user gets reception back and tries again, the laptop rejects the request, and Instant Tethering cannot be performed.

The proposed patch adds handling for the "LAST_PROVISIONING_FAILED" response code and accepts it as a valid response from the phone.
Project Member

Comment 16 by sheriffbot@chromium.org, Dec 13

Labels: -Merge-Request-72 Merge-Reject-72 Hotlist-Merge-Reject
The bug is marked as P3 or Feature. It should not be merged as M72 is in beta. 
Please contact the approriate milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Pri-2 -Type-Feature Pri-1 Type-Bug
This shouldn't have been marked as a feature, as comment #16 says. The user-visible impact is a bug which prevents use of the feature without any means of fixing the issue.
Owner: khorimoto@chromium.org
Labels: -Hotlist-Merge-Reject -Merge-Reject-72 Merge-Approved-72
Status: Fixed (was: Started)
Project Member

Comment 21 by bugdroid1@chromium.org, Dec 13

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ecd68a86ddbb15c62d4c466e2f71d6e7e64ab72b

commit ecd68a86ddbb15c62d4c466e2f71d6e7e64ab72b
Author: Regan Hsu <hsuregan@chromium.org>
Date: Thu Dec 13 20:03:49 2018

[CrOS Instant Tethering] Handle LAST_PROVISIONING_FAILED enum

A new change on the Android side now causes the phone to respond with
LAST_PROVISIONING_FAILED if the last attempt to run provisioning failed.
This CL addresses the case for when the device was offline last time it
checked, or there was some sort of network error when checking.

Bug:  872473 
Change-Id: Ia28f9077990a5a991b3968a6da100222940c11be
Reviewed-on: https://chromium-review.googlesource.com/c/1357656
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612909}(cherry picked from commit 4423cfde6d162b2296a00de2d271fb9fb47ed68e)
Reviewed-on: https://chromium-review.googlesource.com/c/1376715
Cr-Commit-Position: refs/branch-heads/3626@{#336}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/ecd68a86ddbb15c62d4c466e2f71d6e7e64ab72b/chromeos/components/tether/host_scanner_operation.cc
[modify] https://crrev.com/ecd68a86ddbb15c62d4c466e2f71d6e7e64ab72b/chromeos/components/tether/host_scanner_operation_unittest.cc
[modify] https://crrev.com/ecd68a86ddbb15c62d4c466e2f71d6e7e64ab72b/chromeos/components/tether/proto/tether.proto

Labels: CommitLog-Audit-Violation Merge-Without-Approval
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision ecd68a86ddbb15c62d4c466e2f71d6e7e64ab72b was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/ecd68a86ddbb15c62d4c466e2f71d6e7e64ab72b

Commit: ecd68a86ddbb15c62d4c466e2f71d6e7e64ab72b
Author: hsuregan@chromium.org
Commiter: khorimoto@chromium.org
Date: 2018-12-13 20:03:49 +0000 UTC

[CrOS Instant Tethering] Handle LAST_PROVISIONING_FAILED enum

A new change on the Android side now causes the phone to respond with
LAST_PROVISIONING_FAILED if the last attempt to run provisioning failed.
This CL addresses the case for when the device was offline last time it
checked, or there was some sort of network error when checking.

Bug:  872473 
Change-Id: Ia28f9077990a5a991b3968a6da100222940c11be
Reviewed-on: https://chromium-review.googlesource.com/c/1357656
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612909}(cherry picked from commit 4423cfde6d162b2296a00de2d271fb9fb47ed68e)
Reviewed-on: https://chromium-review.googlesource.com/c/1376715
Cr-Commit-Position: refs/branch-heads/3626@{#336}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment