[CRD iOS] Fix crash for executing OpenGL commands when app goes to the background

iOS doesn't allow app executing OpenGL commands after it goes to the
background. Disabling the video stream on the host isn't good enough as
the coming video frame will still arrive.

This CL fixes the crash by destroying the OpenGL context when the app
becomes inactive, and recreates the context when the app becomes active
again.

Note that even after we fix the crash, the session will still be
disconnected, since the app will eventually enter the Suspended state
and stop processing network events. See link about an iOS app's
lifecycle:
https://developer.apple.com/library/content/documentation/iPhone/Conceptual/iPhoneOSProgrammingGuide/TheAppLifeCycle/TheAppLifeCycle.html#//apple_ref/doc/uid/TP40007072-CH2-SW3

Bug: 829449
Change-Id: I507b8e2a856b3e019db99faff188bd5f312c5a51
Reviewed-on: https://chromium-review.googlesource.com/1013244
Commit-Queue: Yuwei Huang <[email protected]>
Reviewed-by: Jamie Walch <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#551097}(cherry picked from commit 9e9ae0c)
Reviewed-on: https://chromium-review.googlesource.com/1015187
Reviewed-by: Yuwei Huang <[email protected]>
Cr-Commit-Position: refs/branch-heads/3396@{#61}
Cr-Branched-From: 9ef2aa8-refs/heads/master@{#550428}

Author: ywh233

remoting/client/display/gl_renderer.cc

@@ -53,8 +53,8 @@ void GlRenderer::OnPixelTransformationChanged(
     const std::array<float, 9>& matrix) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!canvas_) {
-    LOG(WARNING) << "Trying to set transformation matrix when the canvas is "
-        "not ready.";
+    VLOG(1) << "Trying to set transformation matrix when the canvas is not "
+            << "ready.";
     return;
   }
   canvas_->SetTransformationMatrix(matrix);

remoting/ios/app/host_view_controller.mm

@@ -46,7 +46,6 @@ @interface HostViewController ()<ClientKeyboardDelegate,
   ClientGestures* _clientGestures;
   ClientKeyboard* _clientKeyboard;
   CGSize _keyboardSize;
-  BOOL _surfaceCreated;
   HostSettings* _settings;
 
   // Used to blur the content when the app enters background.
@@ -80,7 +79,6 @@ - (id)initWithClient:(RemotingClient*)client {
   if (self) {
     _client = client;
     _keyboardSize = CGSizeZero;
-    _surfaceCreated = NO;
     _blocksKeyboard = NO;
     _settings =
         [[RemotingPreferences instance] settingsForHost:client.hostInfo.hostId];
@@ -197,10 +195,7 @@ - (BOOL)prefersStatusBarHidden {
 
 - (void)viewDidAppear:(BOOL)animated {
   [super viewDidAppear:animated];
-  if (!_surfaceCreated) {
-    [_client.displayHandler onSurfaceCreated:_hostView];
-    _surfaceCreated = YES;
-  }
+  [_client.displayHandler createRendererContext:_hostView];
 
   // |_clientKeyboard| should always be the first responder even when the soft
   // keyboard is not visible, so that input from physical keyboard can still be
@@ -272,7 +267,7 @@ - (void)viewDidLayoutSubviews {
   [super viewDidLayoutSubviews];
 
   // Pass the actual size of the view to the renderer.
-  [_client.displayHandler onSurfaceChanged:_hostView.bounds];
+  [_client.displayHandler setSurfaceSize:_hostView.bounds];
 
   // Start the animation on the host's visible area.
   _surfaceSizeAnimationLink.paused = NO;
@@ -646,6 +641,7 @@ - (void)applicationDidBecomeActive:(UIApplication*)application {
     LOG(DFATAL) << "Blur view does not exist.";
     return;
   }
+  [_client.displayHandler createRendererContext:_hostView];
   [_client setVideoChannelEnabled:YES];
   [_blurView removeFromSuperview];
   _blurView = nil;
@@ -668,6 +664,7 @@ - (void)applicationWillResignActive:(UIApplication*)application {
     [_blurView.bottomAnchor constraintEqualToAnchor:_hostView.bottomAnchor],
   ]];
   [_client setVideoChannelEnabled:NO];
+  [_client.displayHandler destroyRendererContext];
 }
 
 @end

remoting/ios/display/gl_display_handler.h

@@ -42,13 +42,16 @@ class CursorShapeStub;
 @interface GlDisplayHandler : NSObject {
 }
 
-- (void)stop;
+// Called once the renderer can start drawing on the view. Do nothing if the
+// surface is already created.
+- (void)createRendererContext:(EAGLView*)view;
 
-// Called once the GLKView created.
-- (void)onSurfaceCreated:(EAGLView*)view;
+// Called when the renderer should stop drawing on the view. Do nothing if the
+// surface is not created.
+- (void)destroyRendererContext;
 
-// Called every time the GLKView dimension is initialized or changed.
-- (void)onSurfaceChanged:(const CGRect&)frame;
+// Called every time the view dimension is initialized or changed.
+- (void)setSurfaceSize:(const CGRect&)frame;
 
 // Must be called immediately after the object is constructed.
 - (std::unique_ptr<remoting::RendererProxy>)CreateRendererProxy;

remoting/ios/display/gl_display_handler.mm

@@ -56,9 +56,9 @@
 
   void OnFrameReceived(std::unique_ptr<webrtc::DesktopFrame> frame,
                        const base::Closure& done);
-  void Stop();
-  void SurfaceCreated(EAGLView* view);
-  void SurfaceChanged(int width, int height);
+  void CreateRendererContext(EAGLView* view);
+  void DestroyRendererContext();
+  void SetSurfaceSize(int width, int height);
 
   std::unique_ptr<protocol::FrameConsumer> GrabFrameConsumer();
 
@@ -76,12 +76,13 @@ void OnFrameReceived(std::unique_ptr<webrtc::DesktopFrame> frame,
   std::unique_ptr<DualBufferFrameConsumer> owned_frame_consumer_;
   base::WeakPtr<DualBufferFrameConsumer> frame_consumer_;
 
-  // TODO(yuweih): Release references once the surface is destroyed.
   EAGLContext* eagl_context_;
   std::unique_ptr<GlRenderer> renderer_;
-  //  GlDemoScreen *demo_screen_;
   __weak id<GlDisplayHandlerDelegate> handler_delegate_;
 
+  // Valid only when the surface is created.
+  __weak EAGLView* view_;
+
   // Used on display thread.
   base::WeakPtr<Core> weak_ptr_;
   base::WeakPtrFactory<Core> weak_factory_;
@@ -190,21 +191,20 @@ void OnFrameReceived(std::unique_ptr<webrtc::DesktopFrame> frame,
       }));
 }
 
-void Core::Stop() {
-  DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
-
-  eagl_context_ = nil;
-  // demo_screen_ = nil;
-}
-
-void Core::SurfaceCreated(EAGLView* view) {
+void Core::CreateRendererContext(EAGLView* view) {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
   DCHECK(eagl_context_);
 
+  if (view_) {
+    return;
+  }
+  view_ = view;
+
   runtime_->ui_task_runner()->PostTask(FROM_HERE, base::BindBlockArc(^() {
                                          [view startWithContext:eagl_context_];
                                        }));
 
+  // TODO(yuweih): Rename methods in GlRenderer.
   renderer_->OnSurfaceCreated(
       std::make_unique<GlCanvas>(static_cast<int>([eagl_context_ API])));
 
@@ -215,7 +215,22 @@ void OnFrameReceived(std::unique_ptr<webrtc::DesktopFrame> frame,
                             frame_consumer_));
 }
 
-void Core::SurfaceChanged(int width, int height) {
+void Core::DestroyRendererContext() {
+  DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
+
+  if (!view_) {
+    return;
+  }
+
+  renderer_->OnSurfaceDestroyed();
+  __weak EAGLView* view = view_;
+  runtime_->ui_task_runner()->PostTask(FROM_HERE, base::BindBlockArc(^() {
+                                         [view stop];
+                                       }));
+  view_ = nil;
+}
+
+void Core::SetSurfaceSize(int width, int height) {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
   renderer_->OnSurfaceChanged(width, height);
 }
@@ -250,12 +265,6 @@ - (void)dealloc {
 
 #pragma mark - Public
 
-- (void)stop {
-  _runtime->display_task_runner()->PostTask(
-      FROM_HERE,
-      base::Bind(&remoting::GlDisplayHandler::Core::Stop, _core->GetWeakPtr()));
-}
-
 - (std::unique_ptr<remoting::RendererProxy>)CreateRendererProxy {
   return _core->GrabRendererProxy();
 }
@@ -270,17 +279,25 @@ - (void)stop {
       _core->GetWeakPtr(), _runtime->display_task_runner());
 }
 
-- (void)onSurfaceCreated:(EAGLView*)view {
+- (void)createRendererContext:(EAGLView*)view {
+  _runtime->display_task_runner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&remoting::GlDisplayHandler::Core::CreateRendererContext,
+                     _core->GetWeakPtr(), view));
+}
+
+- (void)destroyRendererContext {
   _runtime->display_task_runner()->PostTask(
-      FROM_HERE, base::Bind(&remoting::GlDisplayHandler::Core::SurfaceCreated,
-                            _core->GetWeakPtr(), view));
+      FROM_HERE,
+      base::BindOnce(&remoting::GlDisplayHandler::Core::DestroyRendererContext,
+                     _core->GetWeakPtr()));
 }
 
-- (void)onSurfaceChanged:(const CGRect&)frame {
+- (void)setSurfaceSize:(const CGRect&)frame {
   _runtime->display_task_runner()->PostTask(
       FROM_HERE,
-      base::Bind(&remoting::GlDisplayHandler::Core::SurfaceChanged,
-                 _core->GetWeakPtr(), frame.size.width, frame.size.height));
+      base::BindOnce(&remoting::GlDisplayHandler::Core::SetSurfaceSize,
+                     _core->GetWeakPtr(), frame.size.width, frame.size.height));
 }
 
 #pragma mark - Properties