Add support to the UI Service to run it inside the browser's process.

Add support to the UI Service to run it insider browser's process.

When UI Service runs inside the browser process in its own separate thread,
there is a few things it needs to do differently: 

 1. It shouldn't set its Screen as a global singleton.
 2. It shouldn't set its ClientNativePixmapFactory as a global singleton.
 3. It can't access the ResourceBundle from its own thread - that can only be
    done from the browser thread.

The third point is the source of most of the complexity in this CL. UI Service
needs to manipulate the cursor, which is done through the ui::ImageCursors
object, which needs to load resources. Since resources can't be loaded on the
UI Service's thread, a mechanism is introduced to let the UI Service do this by
posting tasks to the browser's thread task runner, which is passed into the
UI Service's constructor.

This CL doesn't contain browser-side changes to run the UI Service in-process.

BUG=722527
Review-Url: https://codereview.chromium.org/2979933002
Cr-Commit-Position: refs/heads/master@{#486509}
Committed: https://chromium.googlesource.com/chromium/src/+/04cdb9269087e39efc12fc8dc21184bce075d72d

[mfomitchev, 2017-07-13 01:44:14]

The CQ bit was checked by mfomitchev@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-13 01:44:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-13 01:58:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-13 01:58:04]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[mfomitchev, 2017-07-13 01:59:09]

Description was changed from

==========
Add support to the UI Service to run it insider browser's process.

BUG=722527
==========

to

==========
Add support to the UI Service to run it insider browser's process.

When UI Service runs inside the browser process in its own separate thread,
there is a few things it needs to do differently: 

 1. It shouldn't set its Screen as a global singleton.
 2. It shouldn't set its ClientNativePixmapFactory as a global singleton.
 3. It can't access the ResourceBundle from its own thread - that can only be
    done from the browser thread.

The third point is the source of most of the complexity in this CL. UI Service
needs to manipulate the cursor, which is done through the ui::ImageCursors
object, which needs to load resources. Since resources can't be loaded on the
UI Service's thread, a mechanism is introduced to let the UI Service do this by
posting tasks to the browser's thread task runner, which is passed into the
UI Service's constructor.

This CL doesn't contain browser-side changes to run the UI Service in-process.

BUG=722527
==========

[mfomitchev, 2017-07-13 02:02:29]

The CQ bit was checked by mfomitchev@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-13 02:02:41]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mfomitchev, 2017-07-13 02:04:04]

mfomitchev@chromium.org changed reviewers:
+ sky@chromium.org

[mfomitchev, 2017-07-13 02:04:05]

PTAL: Part one of in-process Mus.

[mfomitchev, 2017-07-13 03:08:06]

The CQ bit was checked by mfomitchev@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-13 03:08:17]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mfomitchev, 2017-07-13 03:12:56]

The CQ bit was checked by mfomitchev@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-13 03:13:01]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-13 05:53:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-13 05:53:54]

Dry run: This issue passed the CQ dry run.

[sky, 2017-07-13 17:14:58]

https://codereview.chromium.org/2979933002/diff/60001/services/ui/common/imag...
File services/ui/common/image_cursors_set.h (right):

https://codereview.chromium.org/2979933002/diff/60001/services/ui/common/imag...
services/ui/common/image_cursors_set.h:11: #include "base/memory/weak_ptr.h"
inlucde base/macros too.

https://codereview.chromium.org/2979933002/diff/60001/services/ui/common/imag...
services/ui/common/image_cursors_set.h:21: virtual ~ImageCursorsSet();
Is the virtual needed?

https://codereview.chromium.org/2979933002/diff/60001/services/ui/common/imag...
services/ui/common/image_cursors_set.h:24: void RemoveImageCursors(ImageCursors*
image_cursors);
How about naming this DeleteImageCursors given that it what it does.

https://codereview.chromium.org/2979933002/diff/60001/services/ui/service.h
File services/ui/service.h (right):

https://codereview.chromium.org/2979933002/diff/60001/services/ui/service.h#n...
services/ui/service.h:74: InProcessConfig(const InProcessConfig& config);
Do you really need to declare this?

https://codereview.chromium.org/2979933002/diff/60001/services/ui/service.h#n...
services/ui/service.h:84: // |config| should be null when UI Service runs in
it's own separate process,
Given many callers use null, how about making InProcessConfig an optional arg
with the default null?

https://codereview.chromium.org/2979933002/diff/60001/services/ui/service.h#n...
services/ui/service.h:86: explicit Service(InProcessConfig* config);
const InProcessConfig* ?

https://codereview.chromium.org/2979933002/diff/60001/services/ui/ws/threaded...
File services/ui/ws/threaded_image_cursors.h (right):

https://codereview.chromium.org/2979933002/diff/60001/services/ui/ws/threaded...
services/ui/ws/threaded_image_cursors.h:27: // asynchronously via the provided
task runner.
Please document why we need to step through these hoops.

https://codereview.chromium.org/2979933002/diff/60001/services/ui/ws/window_s...
File services/ui/ws/window_server.h (left):

https://codereview.chromium.org/2979933002/diff/60001/services/ui/ws/window_s...
services/ui/ws/window_server.h:56: explicit WindowServer(WindowServerDelegate*
delegate);
Keep the explicit?

[mfomitchev, 2017-07-13 17:53:06]

https://codereview.chromium.org/2979933002/diff/60001/services/ui/common/imag...
File services/ui/common/image_cursors_set.h (right):

https://codereview.chromium.org/2979933002/diff/60001/services/ui/common/imag...
services/ui/common/image_cursors_set.h:11: #include "base/memory/weak_ptr.h"
On 2017/07/13 17:14:58, sky wrote:
> inlucde base/macros too.

Done.

https://codereview.chromium.org/2979933002/diff/60001/services/ui/common/imag...
services/ui/common/image_cursors_set.h:21: virtual ~ImageCursorsSet();
On 2017/07/13 17:14:58, sky wrote:
> Is the virtual needed?

Removed

https://codereview.chromium.org/2979933002/diff/60001/services/ui/common/imag...
services/ui/common/image_cursors_set.h:24: void RemoveImageCursors(ImageCursors*
image_cursors);
On 2017/07/13 17:14:58, sky wrote:
> How about naming this DeleteImageCursors given that it what it does.

Done.

https://codereview.chromium.org/2979933002/diff/60001/services/ui/service.h
File services/ui/service.h (right):

https://codereview.chromium.org/2979933002/diff/60001/services/ui/service.h#n...
services/ui/service.h:74: InProcessConfig(const InProcessConfig& config);
On 2017/07/13 17:14:58, sky wrote:
> Do you really need to declare this?

Nope, removed.
I had Service ctor take InProcessConfig by value at first.

https://codereview.chromium.org/2979933002/diff/60001/services/ui/service.h#n...
services/ui/service.h:84: // |config| should be null when UI Service runs in
it's own separate process,
On 2017/07/13 17:14:58, sky wrote:
> Given many callers use null, how about making InProcessConfig an optional arg
> with the default null?

Done.

https://codereview.chromium.org/2979933002/diff/60001/services/ui/service.h#n...
services/ui/service.h:86: explicit Service(InProcessConfig* config);
On 2017/07/13 17:14:58, sky wrote:
> const InProcessConfig* ?

Done.

https://codereview.chromium.org/2979933002/diff/60001/services/ui/ws/threaded...
File services/ui/ws/threaded_image_cursors.h (right):

https://codereview.chromium.org/2979933002/diff/60001/services/ui/ws/threaded...
services/ui/ws/threaded_image_cursors.h:27: // asynchronously via the provided
task runner.
On 2017/07/13 17:14:58, sky wrote:
> Please document why we need to step through these hoops.

Done.

https://codereview.chromium.org/2979933002/diff/60001/services/ui/ws/window_s...
File services/ui/ws/window_server.h (left):

https://codereview.chromium.org/2979933002/diff/60001/services/ui/ws/window_s...
services/ui/ws/window_server.h:56: explicit WindowServer(WindowServerDelegate*
delegate);
On 2017/07/13 17:14:58, sky wrote:
> Keep the explicit?

Done.

[sky, 2017-07-13 20:35:07]

LGTM

[mfomitchev, 2017-07-13 20:40:36]

The CQ bit was checked by mfomitchev@chromium.org

[commit-bot: I haz the power, 2017-07-13 20:41:32]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-13 22:53:14]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1499978436703810,
"parent_rev": "597a27482d43f2ac88c1c35f94533889845b80ec", "commit_rev":
"eb52ff70a829f4f507aee635195c95737b8e864a"}

[commit-bot: I haz the power, 2017-07-13 22:53:30]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1499978436703810,
"parent_rev": "beff6adae6fa9a2c58c05cfa3f11b2ad159e0121", "commit_rev":
"04cdb9269087e39efc12fc8dc21184bce075d72d"}

[commit-bot: I haz the power, 2017-07-13 22:53:45]

Description was changed from

==========
Add support to the UI Service to run it insider browser's process.

When UI Service runs inside the browser process in its own separate thread,
there is a few things it needs to do differently: 

 1. It shouldn't set its Screen as a global singleton.
 2. It shouldn't set its ClientNativePixmapFactory as a global singleton.
 3. It can't access the ResourceBundle from its own thread - that can only be
    done from the browser thread.

The third point is the source of most of the complexity in this CL. UI Service
needs to manipulate the cursor, which is done through the ui::ImageCursors
object, which needs to load resources. Since resources can't be loaded on the
UI Service's thread, a mechanism is introduced to let the UI Service do this by
posting tasks to the browser's thread task runner, which is passed into the
UI Service's constructor.

This CL doesn't contain browser-side changes to run the UI Service in-process.

BUG=722527
==========

to

==========
Add support to the UI Service to run it insider browser's process.

When UI Service runs inside the browser process in its own separate thread,
there is a few things it needs to do differently: 

 1. It shouldn't set its Screen as a global singleton.
 2. It shouldn't set its ClientNativePixmapFactory as a global singleton.
 3. It can't access the ResourceBundle from its own thread - that can only be
    done from the browser thread.

The third point is the source of most of the complexity in this CL. UI Service
needs to manipulate the cursor, which is done through the ui::ImageCursors
object, which needs to load resources. Since resources can't be loaded on the
UI Service's thread, a mechanism is introduced to let the UI Service do this by
posting tasks to the browser's thread task runner, which is passed into the
UI Service's constructor.

This CL doesn't contain browser-side changes to run the UI Service in-process.

BUG=722527

Review-Url: https://codereview.chromium.org/2979933002
Cr-Commit-Position: refs/heads/master@{#486509}
Committed:
https://chromium.googlesource.com/chromium/src/+/04cdb9269087e39efc12fc8dc211...
==========

[commit-bot: I haz the power, 2017-07-13 22:53:47]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/04cdb9269087e39efc12fc8dc211...