Ensure that the DirectWrite font cache works in Chrome canary on Windows 8+ with AppContainer prote…

Ensure that the DirectWrite font cache works in Chrome canary on Windows 8+ with AppContainer protection

The original patch attempting to fix this was reverted due to perf regressions caused by opening font files in the sandbox which
end up being brokered to the browser.

The DirectWrite font cache is mapped as a shared section by the renderer processes. The browser creates
the section. On Windows 8+ with AppContainer protection the renderers are unable to open this shared section
as the BaseNamedObjects object directory is virtualized to \Sessions\SessionId\AppContainerNamedObjects.

This effectively means that the renderers now fallback to the old method of enumerating all fonts while DirectWrite
builds up its font cache. This hurts performance.

To share the font cache shared section handle with the renderer we now open the font cache shared section in
RenderProcessHostImpl::Init() and add it to the TargetPolicy when the renderer is about to be spawned. The sandbox broker reads
the handles from the policy along with other handles like stderr/stdout etc and adds this information to STARTUPINFOEX.
This enables these handles to be shared with the target.

We also need a change in the SharedMemory class to allow us to open a shared section in inheritable mode. This is achieved by
adding a method called set_inheritable to the SharedMemory class.

BUG=481285
R=cpu

Committed: https://crrev.com/5d498ac79386ec8155fb2a768541e2b4b18c7d49
Cr-Commit-Position: refs/heads/master@{#329041}

[ananta, 2015-05-05 18:46:57]

ananta@chromium.org changed reviewers:
+ shrikant@chromium.org

[ananta, 2015-05-05 18:46:58]

+shrikant

[cpu_(ooo_6.6-7.5), 2015-05-05 20:42:39]

pls do a separate cl for the sandbox changes with a test.

https://codereview.chromium.org/1122163002/diff/40001/sandbox/win/src/sandbox...
File sandbox/win/src/sandbox_policy.h (right):

https://codereview.chromium.org/1122163002/diff/40001/sandbox/win/src/sandbox...
sandbox/win/src/sandbox_policy.h:254: virtual HandleList GetHandlesBeingShared()
= 0;
remove this from the interface and place it only on the policy_base class, it
can be non-virtual.

[Shrikant Kelkar, 2015-05-05 20:43:16]

https://codereview.chromium.org/1122163002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/1122163002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:374:
direct_write_font_cache_section_ = direct_write_font_cache_section;
Scope/DuplicateHandle?

https://codereview.chromium.org/1122163002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:393:
base::SharedMemoryHandle direct_write_font_cache_section_;
Scope? Who is the owner?

https://codereview.chromium.org/1122163002/diff/40001/sandbox/win/src/sandbox...
File sandbox/win/src/sandbox_policy.h (right):

https://codereview.chromium.org/1122163002/diff/40001/sandbox/win/src/sandbox...
sandbox/win/src/sandbox_policy.h:16: typedef std::vector<HANDLE> HandleList;
Same raw handles or scoped/refcounted handles?

[ananta, 2015-05-07 21:59:09]

On 2015/05/05 20:42:39, cpu wrote:
> pls do a separate cl for the sandbox changes with a test.
> 
>
https://codereview.chromium.org/1122163002/diff/40001/sandbox/win/src/sandbox...
> File sandbox/win/src/sandbox_policy.h (right):
> 
>
https://codereview.chromium.org/1122163002/diff/40001/sandbox/win/src/sandbox...
> sandbox/win/src/sandbox_policy.h:254: virtual HandleList
GetHandlesBeingShared()
> = 0;
> remove this from the interface and place it only on the policy_base class, it
> can be non-virtual.

Done.

[ananta, 2015-05-07 22:00:48]

https://codereview.chromium.org/1122163002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/1122163002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:374:
direct_write_font_cache_section_ = direct_write_font_cache_section;
On 2015/05/05 20:43:16, Shrikant Kelkar wrote:
> Scope/DuplicateHandle?

This code is no longer needed. Based on a discussion with cpu, we decided to
move this code to content sandbox

https://codereview.chromium.org/1122163002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:393:
base::SharedMemoryHandle direct_write_font_cache_section_;
On 2015/05/05 20:43:15, Shrikant Kelkar wrote:
> Scope? Who is the owner?

Reworked the patch to move the handle sharing and passing the duplicated handle
via the command line code to content sandbox

[Shrikant Kelkar, 2015-05-07 22:50:11]

lgtm

[ananta, 2015-05-07 23:13:03]

ananta@chromium.org changed reviewers:
+ avi@chromium.org

[ananta, 2015-05-07 23:13:04]

+avi for content owners review.

[cpu_(ooo_6.6-7.5), 2015-05-08 03:03:25]

https://codereview.chromium.org/1122163002/diff/80001/content/common/sandbox_...
File content/common/sandbox_win.cc (right):

https://codereview.chromium.org/1122163002/diff/80001/content/common/sandbox_...
content/common/sandbox_win.cc:694: HANDLE shared_handle =
use void*, we don't want future coder to try to close it.

https://codereview.chromium.org/1122163002/diff/80001/content/common/sandbox_...
content/common/sandbox_win.cc:699: }
is this code working? after the renderer process is launched, this process ends
up with no handles to the shared section, so the section is opened in the
renderer but not in the browser.

Do you mean to open the section every time a renderer is opened? I was expecting
you to do the opening only once.

[Avi (use Gerrit), 2015-05-08 03:04:53]

lgtm

OK

[ananta, 2015-05-08 04:10:18]

https://codereview.chromium.org/1122163002/diff/80001/content/common/sandbox_...
File content/common/sandbox_win.cc (right):

https://codereview.chromium.org/1122163002/diff/80001/content/common/sandbox_...
content/common/sandbox_win.cc:699: }
On 2015/05/08 03:03:24, cpu wrote:
> is this code working? after the renderer process is launched, this process
ends
> up with no handles to the shared section, so the section is opened in the
> renderer but not in the browser.
> 
> Do you mean to open the section every time a renderer is opened? I was
expecting
> you to do the opening only once.
> 

The shared section is created by the browser via a call to
content::LoadCacheFile. It remains valid for the duration of the process. The
open here is fine and should work for each renderer.

[ananta, 2015-05-08 19:08:19]

https://codereview.chromium.org/1122163002/diff/80001/content/common/sandbox_...
File content/common/sandbox_win.cc (right):

https://codereview.chromium.org/1122163002/diff/80001/content/common/sandbox_...
content/common/sandbox_win.cc:694: HANDLE shared_handle =
On 2015/05/08 03:03:24, cpu wrote:
> use void*, we don't want future coder to try to close it.

Done.

[cpu_(ooo_6.6-7.5), 2015-05-08 19:56:15]

lgtm

[cpu_(ooo_6.6-7.5), 2015-05-08 19:56:19]

The CQ bit was checked by cpu@chromium.org

[cpu_(ooo_6.6-7.5), 2015-05-08 19:56:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
shrikant@chromium.org, avi@chromium.org
Link to the patchset: https://codereview.chromium.org/1122163002/#ps120001
(title: "Attempt to fix redness")

[commit-bot: I haz the power, 2015-05-08 19:56:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1122163002/120001

[commit-bot: I haz the power, 2015-05-08 20:02:08]

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

[commit-bot: I haz the power, 2015-05-08 20:02:10]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ananta, 2015-05-08 20:23:17]

The CQ bit was checked by ananta@chromium.org

[ananta, 2015-05-08 20:23:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
shrikant@chromium.org, cpu@chromium.org, avi@chromium.org
Link to the patchset: https://codereview.chromium.org/1122163002/#ps140001
(title: "Rebased to tip")

[commit-bot: I haz the power, 2015-05-08 20:24:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1122163002/140001

[commit-bot: I haz the power, 2015-05-09 04:20:03]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-05-09 04:21:51]

Patchset 8 (id:??) landed as
https://crrev.com/5d498ac79386ec8155fb2a768541e2b4b18c7d49
Cr-Commit-Position: refs/heads/master@{#329041}