TetherService incorrectly creates TetherComponent if feature "not supported by chromebook" |
|||
Issue descriptionChrome Version: head TetherService only complains when its FeatureState is kNotSupportedByChromebook or kUnavailableInsufficientSecurity, but doesn't actually return a proper error code at that time, falling through to returning ENABLED, leading to the TetherComponent being incorrectly instantiated [1]. 1) https://cs.chromium.org/chromium/src/chrome/browser/chromeos/tether/tether_service.cc?q=tether_service.cc&sq=package:chromium&dr&l=701
,
Sep 14
It seems the assumption of the line I referenced is that this situation should never occur. However, I've observed it on my real Chromebook. Therefore I'm investigating why this is happening on a real device in the first place as well.
,
Sep 14
On the chromebook which I observed this issue on, I saw the following logs a few minutes afterwards: tether_service.cc:699 - Invalid MultiDevice FeatureState: FeatureState:kNotSupportedByChromebook tether_service.cc:699 - Invalid MultiDevice FeatureState: FeatureState:kNotSupportedByChromebook tether_service.cc:699 - Invalid MultiDevice FeatureState: FeatureState:kNotSupportedByChromebook [etc...] [2 minutes passed] sync_scheduler_impl.cc:108: Timer fired for aggressive recovery, making request... device_reenroller.cc:152: DeviceRenroller::OnForceSyncNowComplete(): The local device metadata's supported software features do no agree with the set extracted from GCM device info. At this point, I no longer see error messages about Tether FeatureState == FeatureState:kNotSupportedByChromebook, and Tether correctly appears in BetterTogether settings. It looks like the root cause of this was DeviceRenroller not acting as soon as it should. (tracked in crbug.com/870770). All that needs to be done for this bug is to add an early return condition like I mentioned.
,
Sep 14
Ah, that makes sense. Once Josh fixes issue 870770, this issue will be minimized, but it is still possible that we hit this state for a small window. To fix, this, we should handle the kNotSupportedByChromebook case the same way that we handle kNotSupportedByPhone and kUnavailableNoVerifiedHost cases. You can just add an extra case there and fall through to NO_AVAILABLE_HOSTS.
,
Sep 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/775d835172264ee7970002b8d45bf41fe06b400e commit 775d835172264ee7970002b8d45bf41fe06b400e Author: Ryan Hansberry <hansberry@chromium.org> Date: Fri Sep 14 21:51:12 2018 Tether: Do not build TetherComponent if feature is not supported. R=khorimoto@chromium.org Bug: 884323 Change-Id: I86ee823ac4d22f72792baf2494b504b53aa680c2 Reviewed-on: https://chromium-review.googlesource.com/1227225 Commit-Queue: Ryan Hansberry <hansberry@chromium.org> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#591487} [modify] https://crrev.com/775d835172264ee7970002b8d45bf41fe06b400e/chrome/browser/chromeos/tether/tether_service.cc
,
Sep 14
|
|||
►
Sign in to add a comment |
|||
Comment 1 by hansberry@chromium.org
, Sep 14