New issue
Advanced search Search tips

Issue 614242 link

Starred by 1 user
Status: Fixed
Owner:
yuweih@chromium.org
Closed: Jun 2016
Cc:
yuweih@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

[Android Client] Break down ChromotingJniInstance by threads

Project Member Reported by yuweih@chromium.org, May 24 2016 
Currently ChromotingJniInstance is a ref-counting class running on multiple threads. To be more confusing, it even has a WeakPtrFactory that is only being used on the network thread. We should consider breaking it down to three single-threaded classes, use unique_ptr to hold them and delete them on their right thread.

Comment 1 by yuweih@chromium.org, May 25 2016

Cc: yuweih@chromium.org
 Issue 611893  has been merged into this issue.
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 2 2016 

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

commit b98e6c2bf39155864da71b8b69470ebc58913719
Author: yuweih <yuweih@chromium.org>
Date: Thu Jun 02 17:29:13 2016

ChromotingJniInstance has been a ref-counted class running on multiple threads, which has obscured deletion behavior. This CL drops its ref-counted support and pulls non-network-thread logics out of the class.

Doc: https://docs.google.com/document/d/1ZK-UQSC_rTjNsBa_MFbCmCh3fQRPFuVAwKhWnTZqc_Q/edit?usp=sharing

BUG= 614242 

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

[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/android/BUILD.gn
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/android/client_java_tmpl.gni
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/android/java/src/org/chromium/chromoting/DesktopView.java
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/android/java/src/org/chromium/chromoting/cardboard/CardboardRenderer.java
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/android/java/src/org/chromium/chromoting/cardboard/Cursor.java
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/android/java/src/org/chromium/chromoting/cardboard/Desktop.java
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/android/java/src/org/chromium/chromoting/jni/Client.java
[add] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/android/java/src/org/chromium/chromoting/jni/Display.java
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/client/jni/chromoting_jni_instance.cc
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/client/jni/chromoting_jni_instance.h
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/client/jni/jni_client.cc
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/client/jni/jni_client.h
[add] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/client/jni/jni_display_handler.cc
[add] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/client/jni/jni_display_handler.h
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/client/jni/jni_frame_consumer.cc
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/client/jni/jni_frame_consumer.h
[add] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/client/jni/jni_pairing_secret_fetcher.cc
[add] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/client/jni/jni_pairing_secret_fetcher.h
[modify] https://crrev.com/b98e6c2bf39155864da71b8b69470ebc58913719/remoting/client/jni/remoting_jni_registrar.cc

Comment 4 by yuweih@chromium.org, Jun 27 2016

Status: Fixed (was: Assigned)
►

Sign in to add a comment