Commit efe13306 authored by Yuwei Huang's avatar Yuwei Huang Committed by Commit Bot

[CRD iOS] Fix a crash when session is disconnected while the cursor is still flinging

The issue is that DisconnectFromHost() invalidates gestureInterpreter
before ClientGesture is destroyed (which is destroyed when the host view
is removed). If the user is interacting with the view after
gestureInterpreter is destroyed but before the view is removed then
ClientGesture will call gestureInterpreter's methods on null, which
crashes the app.

This CL fixes this by always checking _client and
_client.gestureInterpreter before using them.

Bug: 823983
Change-Id: I3889c95daf28b84bbf9aecd26f0024dcb15adbe4
Reviewed-on: https://chromium-review.googlesource.com/972631Reviewed-by: default avatarJamie Walch <jamiewalch@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544830}
parent 76d2b41e
...@@ -29,7 +29,8 @@ GestureInterpreter::GestureInterpreter(RendererProxy* renderer, ...@@ -29,7 +29,8 @@ GestureInterpreter::GestureInterpreter(RendererProxy* renderer,
scroll_animation_( scroll_animation_(
kScrollFlingTimeConstant, kScrollFlingTimeConstant,
base::Bind(&GestureInterpreter::ScrollWithoutAbortAnimations, base::Bind(&GestureInterpreter::ScrollWithoutAbortAnimations,
base::Unretained(this))) { base::Unretained(this))),
weak_factory_(this) {
viewport_.RegisterOnTransformationChangedCallback( viewport_.RegisterOnTransformationChangedCallback(
base::Bind(&RendererProxy::SetTransformation, base::Bind(&RendererProxy::SetTransformation,
base::Unretained(renderer_)), base::Unretained(renderer_)),
...@@ -175,6 +176,10 @@ void GestureInterpreter::OnDesktopSizeChanged(int width, int height) { ...@@ -175,6 +176,10 @@ void GestureInterpreter::OnDesktopSizeChanged(int width, int height) {
viewport_.SetDesktopSize(width, height); viewport_.SetDesktopSize(width, height);
} }
base::WeakPtr<GestureInterpreter> GestureInterpreter::GetWeakPtr() {
return weak_factory_.GetWeakPtr();
}
void GestureInterpreter::PanWithoutAbortAnimations(float translation_x, void GestureInterpreter::PanWithoutAbortAnimations(float translation_x,
float translation_y) { float translation_y) {
if (viewport_.IsViewportReady() && if (viewport_.IsViewportReady() &&
......
...@@ -7,6 +7,8 @@ ...@@ -7,6 +7,8 @@
#include <memory> #include <memory>
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "remoting/client/input/touch_input_strategy.h" #include "remoting/client/input/touch_input_strategy.h"
#include "remoting/client/ui/desktop_viewport.h" #include "remoting/client/ui/desktop_viewport.h"
#include "remoting/client/ui/fling_animation.h" #include "remoting/client/ui/fling_animation.h"
...@@ -77,6 +79,8 @@ class GestureInterpreter { ...@@ -77,6 +79,8 @@ class GestureInterpreter {
void OnSurfaceSizeChanged(int width, int height); void OnSurfaceSizeChanged(int width, int height);
void OnDesktopSizeChanged(int width, int height); void OnDesktopSizeChanged(int width, int height);
base::WeakPtr<GestureInterpreter> GetWeakPtr();
private: private:
void PanWithoutAbortAnimations(float translation_x, float translation_y); void PanWithoutAbortAnimations(float translation_x, float translation_y);
...@@ -110,9 +114,10 @@ class GestureInterpreter { ...@@ -110,9 +114,10 @@ class GestureInterpreter {
FlingAnimation pan_animation_; FlingAnimation pan_animation_;
FlingAnimation scroll_animation_; FlingAnimation scroll_animation_;
base::WeakPtrFactory<GestureInterpreter> weak_factory_;
// GestureInterpreter is neither copyable nor movable. // GestureInterpreter is neither copyable nor movable.
GestureInterpreter(const GestureInterpreter&) = delete; DISALLOW_COPY_AND_ASSIGN(GestureInterpreter);
GestureInterpreter& operator=(const GestureInterpreter&) = delete;
}; };
} // namespace remoting } // namespace remoting
......
...@@ -42,7 +42,8 @@ static remoting::GestureInterpreter::GestureState toGestureState( ...@@ -42,7 +42,8 @@ static remoting::GestureInterpreter::GestureState toGestureState(
UITapGestureRecognizer* _fourFingerTapRecognizer; UITapGestureRecognizer* _fourFingerTapRecognizer;
__weak UIView* _view; __weak UIView* _view;
__weak RemotingClient* _client;
base::WeakPtr<remoting::GestureInterpreter> _gestureInterpreter;
} }
@end @end
...@@ -52,7 +53,7 @@ static remoting::GestureInterpreter::GestureState toGestureState( ...@@ -52,7 +53,7 @@ static remoting::GestureInterpreter::GestureState toGestureState(
- (instancetype)initWithView:(UIView*)view client:(RemotingClient*)client { - (instancetype)initWithView:(UIView*)view client:(RemotingClient*)client {
_view = view; _view = view;
_client = client; _gestureInterpreter = client.gestureInterpreter->GetWeakPtr();
_longPressRecognizer = [[UILongPressGestureRecognizer alloc] _longPressRecognizer = [[UILongPressGestureRecognizer alloc]
initWithTarget:self initWithTarget:self
...@@ -141,23 +142,35 @@ static remoting::GestureInterpreter::GestureState toGestureState( ...@@ -141,23 +142,35 @@ static remoting::GestureInterpreter::GestureState toGestureState(
// Resize the view of the desktop - Zoom in/out. This can occur during a Pan. // Resize the view of the desktop - Zoom in/out. This can occur during a Pan.
- (IBAction)pinchGestureTriggered:(UIPinchGestureRecognizer*)sender { - (IBAction)pinchGestureTriggered:(UIPinchGestureRecognizer*)sender {
if (!_gestureInterpreter) {
return;
}
CGPoint pivot = [sender locationInView:_view]; CGPoint pivot = [sender locationInView:_view];
_client.gestureInterpreter->Zoom(pivot.x, pivot.y, sender.scale, _gestureInterpreter->Zoom(pivot.x, pivot.y, sender.scale,
toGestureState([sender state])); toGestureState([sender state]));
sender.scale = 1.0; // reset scale so next iteration is a relative ratio sender.scale = 1.0; // reset scale so next iteration is a relative ratio
} }
- (IBAction)tapGestureTriggered:(UITapGestureRecognizer*)sender { - (IBAction)tapGestureTriggered:(UITapGestureRecognizer*)sender {
if (!_gestureInterpreter) {
return;
}
CGPoint touchPoint = [sender locationInView:_view]; CGPoint touchPoint = [sender locationInView:_view];
_client.gestureInterpreter->Tap(touchPoint.x, touchPoint.y); _gestureInterpreter->Tap(touchPoint.x, touchPoint.y);
} }
// Change position of the viewport. This can occur during a pinch or long press. // Change position of the viewport. This can occur during a pinch or long press.
- (IBAction)panGestureTriggered:(UIPanGestureRecognizer*)sender { - (IBAction)panGestureTriggered:(UIPanGestureRecognizer*)sender {
if (!_gestureInterpreter) {
return;
}
if ([sender state] == UIGestureRecognizerStateChanged) { if ([sender state] == UIGestureRecognizerStateChanged) {
CGPoint translation = [sender translationInView:_view]; CGPoint translation = [sender translationInView:_view];
_client.gestureInterpreter->Pan(translation.x, translation.y); _gestureInterpreter->Pan(translation.x, translation.y);
// Reset translation so next iteration is relative // Reset translation so next iteration is relative
[sender setTranslation:CGPointZero inView:_view]; [sender setTranslation:CGPointZero inView:_view];
...@@ -167,26 +180,34 @@ static remoting::GestureInterpreter::GestureState toGestureState( ...@@ -167,26 +180,34 @@ static remoting::GestureInterpreter::GestureState toGestureState(
// Do fling on the viewport. This will happen at the end of the one-finger // Do fling on the viewport. This will happen at the end of the one-finger
// panning. // panning.
- (IBAction)flingGestureTriggered:(UIPanGestureRecognizer*)sender { - (IBAction)flingGestureTriggered:(UIPanGestureRecognizer*)sender {
if (!_gestureInterpreter) {
return;
}
if ([sender state] == UIGestureRecognizerStateEnded) { if ([sender state] == UIGestureRecognizerStateEnded) {
CGPoint velocity = [sender velocityInView:_view]; CGPoint velocity = [sender velocityInView:_view];
if (velocity.x != 0 || velocity.y != 0) { if (velocity.x != 0 || velocity.y != 0) {
_client.gestureInterpreter->OneFingerFling(velocity.x, velocity.y); _gestureInterpreter->OneFingerFling(velocity.x, velocity.y);
} }
} }
} }
// Handles the two finger scrolling gesture. // Handles the two finger scrolling gesture.
- (IBAction)scrollGestureTriggered:(UIPanGestureRecognizer*)sender { - (IBAction)scrollGestureTriggered:(UIPanGestureRecognizer*)sender {
if (!_gestureInterpreter) {
return;
}
if ([sender state] == UIGestureRecognizerStateEnded) { if ([sender state] == UIGestureRecognizerStateEnded) {
CGPoint velocity = [sender velocityInView:_view]; CGPoint velocity = [sender velocityInView:_view];
_client.gestureInterpreter->ScrollWithVelocity(velocity.x, velocity.y); _gestureInterpreter->ScrollWithVelocity(velocity.x, velocity.y);
return; return;
} }
CGPoint scrollPoint = [sender locationInView:_view]; CGPoint scrollPoint = [sender locationInView:_view];
CGPoint translation = [sender translationInView:_view]; CGPoint translation = [sender translationInView:_view];
_client.gestureInterpreter->Scroll(scrollPoint.x, scrollPoint.y, _gestureInterpreter->Scroll(scrollPoint.x, scrollPoint.y, translation.x,
translation.x, translation.y); translation.y);
// Reset translation so next iteration is relative // Reset translation so next iteration is relative
[sender setTranslation:CGPointZero inView:_view]; [sender setTranslation:CGPointZero inView:_view];
...@@ -194,22 +215,38 @@ static remoting::GestureInterpreter::GestureState toGestureState( ...@@ -194,22 +215,38 @@ static remoting::GestureInterpreter::GestureState toGestureState(
// Click-Drag mouse operation. This can occur during a Pan. // Click-Drag mouse operation. This can occur during a Pan.
- (IBAction)longPressGestureTriggered:(UILongPressGestureRecognizer*)sender { - (IBAction)longPressGestureTriggered:(UILongPressGestureRecognizer*)sender {
if (!_gestureInterpreter) {
return;
}
CGPoint touchPoint = [sender locationInView:_view]; CGPoint touchPoint = [sender locationInView:_view];
_client.gestureInterpreter->Drag(touchPoint.x, touchPoint.y, _gestureInterpreter->Drag(touchPoint.x, touchPoint.y,
toGestureState([sender state])); toGestureState([sender state]));
} }
- (IBAction)twoFingerTapGestureTriggered:(UITapGestureRecognizer*)sender { - (IBAction)twoFingerTapGestureTriggered:(UITapGestureRecognizer*)sender {
if (!_gestureInterpreter) {
return;
}
CGPoint touchPoint = [sender locationInView:_view]; CGPoint touchPoint = [sender locationInView:_view];
_client.gestureInterpreter->TwoFingerTap(touchPoint.x, touchPoint.y); _gestureInterpreter->TwoFingerTap(touchPoint.x, touchPoint.y);
} }
- (IBAction)threeFingerTapGestureTriggered:(UITapGestureRecognizer*)sender { - (IBAction)threeFingerTapGestureTriggered:(UITapGestureRecognizer*)sender {
if (!_gestureInterpreter) {
return;
}
CGPoint touchPoint = [sender locationInView:_view]; CGPoint touchPoint = [sender locationInView:_view];
_client.gestureInterpreter->ThreeFingerTap(touchPoint.x, touchPoint.y); _gestureInterpreter->ThreeFingerTap(touchPoint.x, touchPoint.y);
} }
- (IBAction)threeFingerPanGestureTriggered:(UIPanGestureRecognizer*)sender { - (IBAction)threeFingerPanGestureTriggered:(UIPanGestureRecognizer*)sender {
if (!_gestureInterpreter) {
return;
}
if ([sender state] != UIGestureRecognizerStateEnded) { if ([sender state] != UIGestureRecognizerStateEnded) {
return; return;
} }
...@@ -227,6 +264,10 @@ static remoting::GestureInterpreter::GestureState toGestureState( ...@@ -227,6 +264,10 @@ static remoting::GestureInterpreter::GestureState toGestureState(
// To trigger the menu. // To trigger the menu.
- (IBAction)fourFingerTapGestureTriggered: - (IBAction)fourFingerTapGestureTriggered:
(UILongPressGestureRecognizer*)sender { (UILongPressGestureRecognizer*)sender {
if (!_gestureInterpreter) {
return;
}
[_delegate menuShouldShow]; [_delegate menuShouldShow];
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment