Bind Application in renderer

Bind Application in renderer.

This involved changing how we get the client handle to the renderer. In the first iteration I was passing this on the command line but that turns out not to work with the sandbox. So instead I an approach used by the Mojo-in-Chrome MojoApplication class and pass the primordial handle via Chrome IPC.

I had to twiddle a bunch of BUILD.gn files in content to get this to work without crashing due to inconsistencies in how MOJO_SHELL_CLIENT was defined.

R=jam@chromium.org,tsepez@chromium.org
http://crbug.com/551253

Committed: https://crrev.com/3edb97198bc5fbc22c5cf13286e8af80449ddfb9
Cr-Commit-Position: refs/heads/master@{#360293}

Committed: https://crrev.com/2c716f9d5ebff610641f4506c17ec81d4b89b7bb
Cr-Commit-Position: refs/heads/master@{#360396}

Committed: https://crrev.com/3f5f999e36c6b9935dcfad5893ca62e3aea606a6
Cr-Commit-Position: refs/heads/master@{#360550}

[Ben Goodger (Google), 2015-11-17 21:45:15]

Description was changed from

==========
Bind Application in renderer

BUG=
==========

to

==========
Bind Application in renderer.

This involved changing how we get the client handle to the renderer. In the
first iteration I was passing this on the command line but that turns out not to
work with the sandbox. So instead I an approach used by the Mojo-in-Chrome
MojoApplication class and pass the primordial handle via Chrome IPC.

I had to twiddle a bunch of BUILD.gn files in content to get this to work
without crashing due to inconsistencies in how MOJO_SHELL_CLIENT was defined.

BUG=
==========

[Ben Goodger (Google), 2015-11-17 21:45:15]

ben@chromium.org changed reviewers:
+ jam@chromium.org, tsepez@chromium.org

[Ben Goodger (Google), 2015-11-17 22:26:56]

Description was changed from

==========
Bind Application in renderer.

This involved changing how we get the client handle to the renderer. In the
first iteration I was passing this on the command line but that turns out not to
work with the sandbox. So instead I an approach used by the Mojo-in-Chrome
MojoApplication class and pass the primordial handle via Chrome IPC.

I had to twiddle a bunch of BUILD.gn files in content to get this to work
without crashing due to inconsistencies in how MOJO_SHELL_CLIENT was defined.

BUG=
==========

to

==========
Bind Application in renderer.

This involved changing how we get the client handle to the renderer. In the
first iteration I was passing this on the command line but that turns out not to
work with the sandbox. So instead I an approach used by the Mojo-in-Chrome
MojoApplication class and pass the primordial handle via Chrome IPC.

I had to twiddle a bunch of BUILD.gn files in content to get this to work
without crashing due to inconsistencies in how MOJO_SHELL_CLIENT was defined.

R=jam@chromium.org,tsepez@chromium.org
http://crbug.com/551253
==========

[Ben Goodger (Google), 2015-11-17 22:27:24]

tsepez: content/common/mojo/mojo_messages.h
jam: *

[Tom Sepez, 2015-11-17 22:30:54]

Messages LGTM (per our conversation).

[jam, 2015-11-18 00:22:02]

lgtm with discussed changes

[Ben Goodger (Google), 2015-11-18 02:23:36]

The CQ bit was checked by ben@chromium.org

[Ben Goodger (Google), 2015-11-18 02:23:37]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1452823003/#ps220001
(title: ".")

[commit-bot: I haz the power, 2015-11-18 02:24:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1452823003/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1452823003/220001

[Ben Goodger (Google), 2015-11-18 03:50:44]

The CQ bit was checked by ben@chromium.org

[Ben Goodger (Google), 2015-11-18 03:50:44]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1452823003/#ps260001
(title: ".")

[commit-bot: I haz the power, 2015-11-18 03:51:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1452823003/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1452823003/260001

[commit-bot: I haz the power, 2015-11-18 04:57:57]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2015-11-18 04:58:42]

Patchset 14 (id:??) landed as
https://crrev.com/3edb97198bc5fbc22c5cf13286e8af80449ddfb9
Cr-Commit-Position: refs/heads/master@{#360293}

[msramek, 2015-11-18 14:55:54]

A revert of this CL (patchset #14 id:260001) has been created in
https://codereview.chromium.org/1460713002/ by msramek@chromium.org.

The reason for reverting is: Speculatively reverting this, as this CL landed
exactly at the time when renderer started crashing on linux_chromium_rel_ng..

[Ben Goodger (Google), 2015-11-18 18:10:46]

Description was changed from

==========
Bind Application in renderer.

This involved changing how we get the client handle to the renderer. In the
first iteration I was passing this on the command line but that turns out not to
work with the sandbox. So instead I an approach used by the Mojo-in-Chrome
MojoApplication class and pass the primordial handle via Chrome IPC.

I had to twiddle a bunch of BUILD.gn files in content to get this to work
without crashing due to inconsistencies in how MOJO_SHELL_CLIENT was defined.

R=jam@chromium.org,tsepez@chromium.org
http://crbug.com/551253

Committed: https://crrev.com/3edb97198bc5fbc22c5cf13286e8af80449ddfb9
Cr-Commit-Position: refs/heads/master@{#360293}
==========

to

==========
Bind Application in renderer.

This involved changing how we get the client handle to the renderer. In the
first iteration I was passing this on the command line but that turns out not to
work with the sandbox. So instead I an approach used by the Mojo-in-Chrome
MojoApplication class and pass the primordial handle via Chrome IPC.

I had to twiddle a bunch of BUILD.gn files in content to get this to work
without crashing due to inconsistencies in how MOJO_SHELL_CLIENT was defined.

R=jam@chromium.org,tsepez@chromium.org
http://crbug.com/551253

Committed: https://crrev.com/3edb97198bc5fbc22c5cf13286e8af80449ddfb9
Cr-Commit-Position: refs/heads/master@{#360293}
==========

[Ben Goodger (Google), 2015-11-18 18:17:35]

The CQ bit was checked by ben@chromium.org

[Ben Goodger (Google), 2015-11-18 18:17:36]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1452823003/#ps280001
(title: ".")

[commit-bot: I haz the power, 2015-11-18 18:18:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1452823003/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1452823003/280001

[commit-bot: I haz the power, 2015-11-18 19:56:22]

Committed patchset #15 (id:280001)

[commit-bot: I haz the power, 2015-11-18 19:58:00]

Patchset 15 (id:??) landed as
https://crrev.com/2c716f9d5ebff610641f4506c17ec81d4b89b7bb
Cr-Commit-Position: refs/heads/master@{#360396}

[skobes, 2015-11-18 22:28:34]

This change is making a bunch of layout tests crash with:

[1:1:1118/141621:108326049405:FATAL:lock.cc(32)] Check failed:
owning_thread_ref_.is_null(). 
#0 0x00000067e2de base::debug::StackTrace::StackTrace()
#1 0x00000057a71f logging::LogMessage::~LogMessage()
#2 0x0000005eb69b base::Lock::CheckUnheldAndMark()
#3 0x000000518e1c base::Lock::Acquire()
#4 0x000000518b43 base::AutoLock::AutoLock()
#5 0x0000005b4014 base::SequenceCheckerImpl::CalledOnValidSequencedThread()
#6 0x000006f074ae IDMap<>::Lookup()
#7 0x000006f0598c
content::RendererBlinkPlatformImpl::SetPlatformEventObserverForTesting()
#8 0x000008459264 content::SetMockGamepadProvider()
#9 0x000007b4cdaa content::BlinkTestRunner::SetGamepadProvider()
#10 0x00000755d7b4 test_runner::GamepadController::Create()
#11 0x0000075748c1 test_runner::TestInterfaces::SetDelegate()
#12 0x0000075ba6c5 test_runner::WebTestInterfaces::SetDelegate()
#13 0x000007b10af0 content::LayoutTestRenderProcessObserver::SetTestDelegate()
#14 0x000007b0ed2e
content::LayoutTestContentRendererClient::WebTestProxyCreated()
#15 0x0000016004b9 base::internal::RunnableAdapter<>::Run()

[alancutter (OOO until 2018), 2015-11-18 23:36:54]

Please modify third_party/WebKit/Tools/whitespace_file.txt in your next patch to
force the trybots to run webkit_tests.

[Ben Goodger (Google), 2015-11-18 23:45:46]

How can I run these locally? Are they failing on Windows too?

On Wed, Nov 18, 2015 at 3:36 PM, <alancutter@chromium.org> wrote:

> Please modify third_party/WebKit/Tools/whitespace_file.txt in your next
> patch to
> force the trybots to run webkit_tests.
>
> https://codereview.chromium.org/1452823003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[alancutter (OOO until 2018), 2015-11-18 23:49:21]

A revert of this CL (patchset #15 id:280001) has been created in
https://codereview.chromium.org/1457173002/ by alancutter@chromium.org.

The reason for reverting is: This change is making hundreds of webkit_tests
crash on linux_chromium_rel_ng with:

[1:1:1118/141621:108326049405:FATAL:lock.cc(32)] Check failed:
owning_thread_ref_.is_null(). 
#0 0x00000067e2de base::debug::StackTrace::StackTrace()
#1 0x00000057a71f logging::LogMessage::~LogMessage()
#2 0x0000005eb69b base::Lock::CheckUnheldAndMark()
#3 0x000000518e1c base::Lock::Acquire()
#4 0x000000518b43 base::AutoLock::AutoLock()
#5 0x0000005b4014 base::SequenceCheckerImpl::CalledOnValidSequencedThread()
#6 0x000006f074ae IDMap<>::Lookup()
#7 0x000006f0598c
content::RendererBlinkPlatformImpl::SetPlatformEventObserverForTesting()
#8 0x000008459264 content::SetMockGamepadProvider().

[alancutter (OOO until 2018), 2015-11-18 23:51:37]

On 2015/11/18 at 23:45:46, ben wrote:
> How can I run these locally? Are they failing on Windows too?

Can't confirm whether it fails on Windows. After discussing with dpranke and
skobes on #blink it appears to only fail on a GN build and not GYP. This would
explain why http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux
didn't flinch.

[skobes, 2015-11-18 23:52:40]

On 2015/11/18 23:51:37, alancutter wrote:
> On 2015/11/18 at 23:45:46, ben wrote:
> > How can I run these locally? Are they failing on Windows too?
> 
> Can't confirm whether it fails on Windows. After discussing with dpranke and
> skobes on #blink it appears to only fail on a GN build and not GYP. This would
> explain why
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux
> didn't flinch.

Not sure about Windows, but it's trivially reproducible on Linux:

$ content_shell --run-layout-test data:text/html,hello

[alancutter (OOO until 2018), 2015-11-18 23:54:19]

On 2015/11/18 at 23:45:46, ben wrote:
> How can I run these locally?

To repro I built blink_tests Debug on Linux using GN and ran:
third_party/WebKit/Tools/Scripts/run-webkit-tests --debug
animations/animation-events-create.html

[Ben Goodger (Google), 2015-11-18 23:55:39]

On 2015/11/18 23:54:19, alancutter wrote:
> On 2015/11/18 at 23:45:46, ben wrote:
> > How can I run these locally?
> 
> To repro I built blink_tests Debug on Linux using GN and ran:
> third_party/WebKit/Tools/Scripts/run-webkit-tests --debug
> animations/animation-events-create.html

Thanks looking now!

[Ben Goodger (Google), 2015-11-19 04:42:49]

Description was changed from

==========
Bind Application in renderer.

This involved changing how we get the client handle to the renderer. In the
first iteration I was passing this on the command line but that turns out not to
work with the sandbox. So instead I an approach used by the Mojo-in-Chrome
MojoApplication class and pass the primordial handle via Chrome IPC.

I had to twiddle a bunch of BUILD.gn files in content to get this to work
without crashing due to inconsistencies in how MOJO_SHELL_CLIENT was defined.

R=jam@chromium.org,tsepez@chromium.org
http://crbug.com/551253

Committed: https://crrev.com/3edb97198bc5fbc22c5cf13286e8af80449ddfb9
Cr-Commit-Position: refs/heads/master@{#360293}

Committed: https://crrev.com/2c716f9d5ebff610641f4506c17ec81d4b89b7bb
Cr-Commit-Position: refs/heads/master@{#360396}
==========

to

==========
Bind Application in renderer.

This involved changing how we get the client handle to the renderer. In the
first iteration I was passing this on the command line but that turns out not to
work with the sandbox. So instead I an approach used by the Mojo-in-Chrome
MojoApplication class and pass the primordial handle via Chrome IPC.

I had to twiddle a bunch of BUILD.gn files in content to get this to work
without crashing due to inconsistencies in how MOJO_SHELL_CLIENT was defined.

R=jam@chromium.org,tsepez@chromium.org
http://crbug.com/551253

Committed: https://crrev.com/3edb97198bc5fbc22c5cf13286e8af80449ddfb9
Cr-Commit-Position: refs/heads/master@{#360293}

Committed: https://crrev.com/2c716f9d5ebff610641f4506c17ec81d4b89b7bb
Cr-Commit-Position: refs/heads/master@{#360396}
==========

[Ben Goodger (Google), 2015-11-19 07:57:06]

The CQ bit was checked by ben@chromium.org

[Ben Goodger (Google), 2015-11-19 07:57:06]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1452823003/#ps380001
(title: ".")

[commit-bot: I haz the power, 2015-11-19 07:57:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1452823003/380001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1452823003/380001

[commit-bot: I haz the power, 2015-11-19 10:38:38]

Committed patchset #20 (id:380001)

[commit-bot: I haz the power, 2015-11-19 10:39:18]

Patchset 20 (id:??) landed as
https://crrev.com/3f5f999e36c6b9935dcfad5893ca62e3aea606a6
Cr-Commit-Position: refs/heads/master@{#360550}