Remote assistance on Chrome OS Part IV - It2MeHost

remoting/host/chromoting_host_context.h

Index: remoting/host/chromoting_host_context.h
diff --git a/remoting/host/chromoting_host_context.h b/remoting/host/chromoting_host_context.h
index e4638ffd3b7cec19d2483762aa349a24a8d5beac..2dbb34a848535286e6ad905fde93d077ff81a76f 100644
--- a/remoting/host/chromoting_host_context.h
+++ b/remoting/host/chromoting_host_context.h
@@ -21,8 +21,6 @@ class AutoThreadTaskRunner;
 // process.  This class is virtual only for testing purposes (see below).
 class ChromotingHostContext {
  public:
-  ~ChromotingHostContext();
-
   // Create threads and URLRequestContextGetter for use by a host.
   // During shutdown the caller should tear-down the ChromotingHostContext and
   // then continue to run until |ui_task_runner| is no longer referenced.
@@ -30,6 +28,27 @@ class ChromotingHostContext {
   static scoped_ptr<ChromotingHostContext> Create(
       scoped_refptr<AutoThreadTaskRunner> ui_task_runner);
 
+#if defined(OS_CHROMEOS)
+  // Attaches task runners to the relevant browser threads for the chromoting
+  // host.  Must be called on the UI thread of the browser process.
+  // remoting::UrlRequestContextGetter returns BasicURLRequestContext under
+  // the hood which spawns two new threads per instance.  Since
+  // ChromotingHostContext can be destroyed from any thread, as its owner
+  // (It2MeHost) is ref-counted, joining the created threads during shutdown
+  // violates the "Disallow IO" thread restrictions on some task runners (e.g.
+  // the IO Thread of the browser process).
+  // Instead, we re-use the |url_request_context_getter| in the browser process.

[Wez, 2014/10/29 18:27:51]

If this is the only reason you need to re-use the browser's getter then can't
you pass the chromoting host context off to an IO capable thread to be deleted,
so that there is no reference to the getter remaining on a non-IO thread?

[kelvinp, 2014/10/29 22:20:17]

Your suggestion involves changing the threading model on shut down that is risky
and requires sufficient testing.  Given the existing approach works fine and we
would like to land the feature in M40.  Unless that are significant advantages
of not passing the |url_request_context_getter|. I would prefer to leave the
code as it is.

[Wez, 2014/10/30 01:08:59]

Acknowledged.

+  static scoped_ptr<ChromotingHostContext> CreateForChromeOS(
+      scoped_refptr<net::URLRequestContextGetter> url_request_context_getter);
+#endif  // defined(OS_CHROMEOS)
+
+  ~ChromotingHostContext();
+
+  scoped_ptr<ChromotingHostContext> Copy();
+
+  // Task runner for the thread that is used for the UI.
+  scoped_refptr<AutoThreadTaskRunner> ui_task_runner();
+
   // Task runner for the thread used for audio capture and encoding.
   scoped_refptr<AutoThreadTaskRunner> audio_task_runner();
 
@@ -49,9 +68,6 @@ class ChromotingHostContext {
   // libjingle code may be run.
   scoped_refptr<AutoThreadTaskRunner> network_task_runner();
 
-  // Task runner for the thread that is used for the UI.
-  scoped_refptr<AutoThreadTaskRunner> ui_task_runner();
-
   // Task runner for the thread used by the ScreenRecorder to capture
   // the screen.
   scoped_refptr<AutoThreadTaskRunner> video_capture_task_runner();
@@ -62,7 +78,18 @@ class ChromotingHostContext {
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter();
 
  private:
-  ChromotingHostContext(AutoThreadTaskRunner* ui_task_runner);
+  ChromotingHostContext(
+      scoped_refptr<AutoThreadTaskRunner> ui_task_runner,
+      scoped_refptr<AutoThreadTaskRunner> audio_task_runner,
+      scoped_refptr<AutoThreadTaskRunner> file_task_runner,
+      scoped_refptr<AutoThreadTaskRunner> input_task_runner,
+      scoped_refptr<AutoThreadTaskRunner> network_task_runner,
+      scoped_refptr<AutoThreadTaskRunner> video_capture_task_runner,
+      scoped_refptr<AutoThreadTaskRunner> video_encode_task_runner,
+      scoped_refptr<net::URLRequestContextGetter> url_request_context_getter);
+
+  // Caller-supplied UI thread. This is usually the application main thread.
+  scoped_refptr<AutoThreadTaskRunner> ui_task_runner_;
 
   // Thread for audio capture and encoding.
   scoped_refptr<AutoThreadTaskRunner> audio_task_runner_;
@@ -76,9 +103,6 @@ class ChromotingHostContext {
   // Thread for network operations.
   scoped_refptr<AutoThreadTaskRunner> network_task_runner_;
 
-  // Caller-supplied UI thread. This is usually the application main thread.
-  scoped_refptr<AutoThreadTaskRunner> ui_task_runner_;
-
   // Thread for screen capture.
   scoped_refptr<AutoThreadTaskRunner> video_capture_task_runner_;