Handle LAST_PROVISIONING_FAILED and NO_RECEPTION TetherAvailabilityResponses |
|||||||||||||||||||
Issue descriptionSee b/110237556 for more details. Shiba, how should this situation be handled?
,
Aug 30
Shiba - can you please provide an update here? Thanks!
,
Aug 31
Talked to Shiba offline, and Jesse is handling this.
,
Aug 31
Also, can we remove RVG or is it here for a reason?
,
Aug 31
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.
,
Sep 11
,
Sep 11
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
,
Sep 11
Does this solution also consider b/80411125? Or is that outside the scope of this discussion?
,
Sep 11
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.
,
Sep 20
,
Oct 25
,
Oct 25
,
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
,
Dec 3
,
Dec 13
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.
,
Dec 13
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
,
Dec 13
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.
,
Dec 13
,
Dec 13
,
Dec 13
,
Dec 13
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
,
Dec 19
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 --
,
Dec 19
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 |
|||||||||||||||||||
Comment 1 by jlklein@chromium.org
, Aug 16