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

Issue 794268 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

[CRD iOS] In session connection failure not handled properly

Project Member Reported by yuweih@chromium.org, Dec 12 2017

Issue description

App Version: ToT CRD build (64.0.3261.0)
OS: iOS 11.2

What steps will reproduce the problem?
(1) Connect to a host
(2) Minimize it to the background
(3) Wait for 1-2 minutes
(4) Reopen the app

What is the expected result?
App shows the P2P error message. User can reconnect or go back.

What happens instead?
App gets stuck in the host view. It seems you can still operate but the app will crash afterwards.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 13 2017

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

commit 71b95d6fcc9a85902707560584d880ab4558ceed
Author: Yuwei Huang <yuweih@chromium.org>
Date: Wed Dec 13 20:02:57 2017

[CRD iOS] Properly handle in-session connection failures

* Makes ChromotingSession automatically release its resources when the
  connection is closed or dropped remotely, so that scheduled tasks
  don't crash the app.
* Makes ConnectionViewController pop out the host view and reveal the
  error message when the connection is dropped.

Bug:  794268 
Change-Id: I739dc6efd8dc716f16c8832d34f0f9a9b349af99
Reviewed-on: https://chromium-review.googlesource.com/822169
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523854}
[modify] https://crrev.com/71b95d6fcc9a85902707560584d880ab4558ceed/remoting/client/chromoting_session.cc
[modify] https://crrev.com/71b95d6fcc9a85902707560584d880ab4558ceed/remoting/ios/app/client_connection_view_controller.mm

Comment 2 by yuweih@chromium.org, Dec 13 2017

Labels: Merge-Request-64
This CL only affects Chrome Remote Desktop for iOS and does not affect Chrome.
Project Member

Comment 3 by sheriffbot@chromium.org, Dec 14 2017

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 4 by yuweih@chromium.org, Dec 18 2017

Cc: cma...@chromium.org
Ping?

Comment 5 by cma...@chromium.org, Dec 21 2017

Sorry yuweih@. Please can you provide the rational as to why this should be merged in M64? What is the risk associated with this CL?

Comment 6 by yuweih@chromium.org, Dec 21 2017

This CL fixes a bug that the Chrome Remote Desktop iOS app occasionally crashes when bringing the app back to the foreground.

The risk is generally low:
* Changes are all within the Chrome Remote Desktop iOS project. It doesn't affect Chrome or anything else.
* The change has been well tested on ToT and we have not found any issue so far.
* The only possible impact could be build failure or other issues on the beta buildbots. If this happens we will be happy to revert the change and target it to M65.

Comment 7 by cma...@chromium.org, Dec 21 2017

Labels: -Hotlist-Merge-Review -Merge-Review-64 Merge-Approved-64
Merge approved! Please make sure to verify the fix after merging it.

Comment 8 by yuweih@chromium.org, Dec 22 2017

Thanks!
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 22 2017

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2cfe78cf6e17e03a29131a64a26a1c2e3431a048

commit 2cfe78cf6e17e03a29131a64a26a1c2e3431a048
Author: Yuwei Huang <yuweih@chromium.org>
Date: Fri Dec 22 00:13:07 2017

[CRD iOS] Properly handle in-session connection failures

* Makes ChromotingSession automatically release its resources when the
  connection is closed or dropped remotely, so that scheduled tasks
  don't crash the app.
* Makes ConnectionViewController pop out the host view and reveal the
  error message when the connection is dropped.

Bug:  794268 
Change-Id: I739dc6efd8dc716f16c8832d34f0f9a9b349af99
Reviewed-on: https://chromium-review.googlesource.com/822169
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#523854}(cherry picked from commit 71b95d6fcc9a85902707560584d880ab4558ceed)
Reviewed-on: https://chromium-review.googlesource.com/841662
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#332}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/2cfe78cf6e17e03a29131a64a26a1c2e3431a048/remoting/client/chromoting_session.cc
[modify] https://crrev.com/2cfe78cf6e17e03a29131a64a26a1c2e3431a048/remoting/ios/app/client_connection_view_controller.mm

Status: Fixed (was: Assigned)

Sign in to add a comment