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

Issue 645233 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Last visit 16 days ago
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

ARC: Add a global timeout for opt-in

Project Member Reported by lhchavez@chromium.org, Sep 8 2016

Issue description

If *anything* goes wrong with ARC's first boot, we usually present the user with a nice dialog that allows them to file feedback or try again, which most of the time takes care of failures. However, this is not the case when there is a timeout involved, at which point the UI just keeps spinning waiting forever for ARC to reply with a failure. We should have a global timeout in Chrome to prevent this from happening.

This is b/29915185.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 9 2016

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

commit 3d3317a6e0dde5de7effb4406c4b60bb7aa91bb5
Author: lhchavez <lhchavez@chromium.org>
Date: Fri Sep 09 23:51:41 2016

arc: Add a global 5 minute timeout for ARC opt-in

This should be a safety net for *all* boot failures we have (or might
have). This change replaces the flag |waiting_for_reply_| with a
base::OneShotTimeout that will log the situation and report a timeout
error to the user.

BUG= 645233 
TEST=Intentionally make ARC not send the reply, see the UI, ARC is still
     running.

Review-Url: https://codereview.chromium.org/2323043003
Cr-Commit-Position: refs/heads/master@{#417770}

[modify] https://crrev.com/3d3317a6e0dde5de7effb4406c4b60bb7aa91bb5/chrome/browser/chromeos/arc/arc_auth_service.cc
[modify] https://crrev.com/3d3317a6e0dde5de7effb4406c4b60bb7aa91bb5/chrome/browser/chromeos/arc/arc_auth_service.h
[modify] https://crrev.com/3d3317a6e0dde5de7effb4406c4b60bb7aa91bb5/chrome/browser/chromeos/arc/arc_optin_uma.h
[modify] https://crrev.com/3d3317a6e0dde5de7effb4406c4b60bb7aa91bb5/tools/metrics/histograms/histograms.xml

Labels: Merge-Request-53 Merge-Request-54

Comment 3 by dimu@chromium.org, Sep 12 2016

Labels: -Merge-Request-53 Merge-Review-53 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M53), manual review required.

Comment 4 by dimu@chromium.org, Sep 12 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)

Comment 5 by dimu@chromium.org, Sep 12 2016

Labels: -Merge-Request-53 Merge-Review-53 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M53), manual review required.

Comment 6 by gov...@chromium.org, Sep 12 2016

Seems like this is specific to Chrome OS. 
ketakid@ (Chrome OS TPM) for M53 review.

Comment 7 by gov...@chromium.org, Sep 12 2016

Labels: OS-Chrome
Labels: -Merge-Review-53 Merge-Approved-53
Fix has been validated on M55. Approving merge to M53 cros.
Labels: -Merge-Approved-53 -Merge-Approved-54 Merge-Merged
Status: Fixed (was: Started)
Labels: VerifyIn-55
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb3ec13c84a856dfc36d80b51fc6455626b1ad4e

commit bb3ec13c84a856dfc36d80b51fc6455626b1ad4e
Author: Luis Hector Chavez <lhchavez@google.com>
Date: Tue Sep 13 23:57:19 2016

[Merge to M54] arc: Add a global 5 minute timeout for ARC opt-in

This should be a safety net for *all* boot failures we have (or might
have). This change replaces the flag |waiting_for_reply_| with a
base::OneShotTimeout that will log the situation and report a timeout
error to the user.

BUG= 645233 
TEST=Intentionally make ARC not send the reply, see the UI, ARC is still
     running.

Review-Url: https://codereview.chromium.org/2323043003
Cr-Commit-Position: refs/heads/master@{#417770}
(cherry picked from commit 3d3317a6e0dde5de7effb4406c4b60bb7aa91bb5)

R=holte@chromium.org, yusukes@chromium.org

Review URL: https://codereview.chromium.org/2332093002 .

Cr-Commit-Position: refs/branch-heads/2840@{#348}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/bb3ec13c84a856dfc36d80b51fc6455626b1ad4e/chrome/browser/chromeos/arc/arc_auth_service.cc
[modify] https://crrev.com/bb3ec13c84a856dfc36d80b51fc6455626b1ad4e/chrome/browser/chromeos/arc/arc_auth_service.h
[modify] https://crrev.com/bb3ec13c84a856dfc36d80b51fc6455626b1ad4e/chrome/browser/chromeos/arc/arc_optin_uma.h
[modify] https://crrev.com/bb3ec13c84a856dfc36d80b51fc6455626b1ad4e/tools/metrics/histograms/histograms.xml

Comment 12 by dchan@google.com, Nov 19 2016

Labels: VerifyIn-56
Status: Verified (was: Fixed)

Sign in to add a comment