Add a full Blimp integration test.

Add a full Blimp integration test.

- Add a full integration test that spawns both an engine and client in
  the same process and validates that the rendering pipeline produces
  the correct thing.
- Also add Linux support for BlimpContents and exposed a
  BlimpContentsView, which is required to gain access to the right
  classes to validate the rendering path.

Marking this as NOPRESUBMIT because there are some ScopedAllowIO calls during test setup and client tear down.  Please feel free to revert or ping me on any landing issues.

BUG=644262
NOPRESUBMIT=true

Committed: https://crrev.com/452f128c9232adc0c58caef4564f2fbf1b4bd9b0
Cr-Commit-Position: refs/heads/master@{#418633}

[Khushal, 2016-09-13 04:47:23]

khushalsagar@chromium.org changed reviewers:
+ khushalsagar@chromium.org

[Khushal, 2016-09-13 04:47:24]

Added some comments while you're fixing stuff. :)

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/core/compo...
File blimp/client/core/compositor/blimp_compositor_dependencies.cc (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/core/compo...
blimp/client/core/compositor/blimp_compositor_dependencies.cc:35: Shutdown();
I think you're fine doing this. The content code allows IO for thread teardown
(https://cs.chromium.org/chromium/src/content/browser/browser_main_loop.cc?dr=...).
We would hit the same code-path when the TaskGraphRunner for the UI compositor
is killed.

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/core/conte...
File blimp/client/core/contents/android/blimp_view.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/core/conte...
blimp/client/core/contents/android/blimp_view.h:79: // UPDATE COMMENT The
BlimpContentsImpl that this BlimpView is used for.
That's a good idea. :P

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/bli...
File blimp/client/public/blimp_client_context.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/bli...
blimp/client/public/blimp_client_context.h:67: virtual void
ConnectWithAssignment(const Assignment& assignment) = 0;
If its never meant to be used in production code, add ForTesting to this?

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/con...
File blimp/client/public/contents/blimp_contents_view.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/con...
blimp/client/public/contents/blimp_contents_view.h:30: public:
Add a virtual dtor or this could leak memory.

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/con...
blimp/client/public/contents/blimp_contents_view.h:50: BlimpContentsView() =
default;
Is this necessary?

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/support/co...
File blimp/client/support/compositor/display_compositor.cc (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/support/co...
blimp/client/support/compositor/display_compositor.cc:131: context_provider_ =
nullptr;
How about just doing context_provider_ = std::move(context_provider) here and
doing an

if (context_provider_) in the next block?

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/support/co...
blimp/client/support/compositor/display_compositor.cc:142:
host_->SetVisible(true);
Eek. Don't do these 2 lines.

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/support/co...
File blimp/client/support/compositor/display_compositor.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/support/co...
blimp/client/support/compositor/display_compositor.h:29: class DisplayCompositor
: public cc::LayerTreeHostClient,
I'd suggest naming this as Parent/Root compositor which produces the root
surface for the display. There is already a component display_compositor, and
this has nothing to do with it. No moar confusing names!

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/test/compo...
File blimp/client/test/compositor/test_display_compositor.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/test/compo...
blimp/client/test/compositor/test_display_compositor.h:42: void
GetBitmap(CopyRequest* request);
I know you really wanted a sync call but I would say this half here, half there
WaitableEvent logic feels prone to errors later, might be cleaner to put it all
on the caller and make an async API here. Up to you though.

https://codereview.chromium.org/2320923002/diff/20001/blimp/engine/browser_te...
File blimp/engine/browser_tests/integration_test.cc (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/engine/browser_te...
blimp/engine/browser_tests/integration_test.cc:35: explicit
PageLoadWaitBlimpContentsObserver(client::BlimpContents* contents)
How about just creating this object after you load the URL and having it
internally create the waiter and pump the messages till it is signaled?

https://codereview.chromium.org/2320923002/diff/20001/blimp/engine/browser_te...
blimp/engine/browser_tests/integration_test.cc:77:
base::MakeUnique<client::MockCompositorDependencies>();
We can use the real BlimpCompositorDependencies here right?

[David Trainor- moved to gerrit, 2016-09-13 06:18:05]

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/core/compo...
File blimp/client/core/compositor/blimp_compositor_dependencies.cc (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/core/compo...
blimp/client/core/compositor/blimp_compositor_dependencies.cc:35: Shutdown();
On 2016/09/13 04:47:23, Khushal wrote:
> I think you're fine doing this. The content code allows IO for thread teardown
>
(https://cs.chromium.org/chromium/src/content/browser/browser_main_loop.cc?dr=...).
> We would hit the same code-path when the TaskGraphRunner for the UI compositor
> is killed.

Acknowledged.

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/core/conte...
File blimp/client/core/contents/android/blimp_view.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/core/conte...
blimp/client/core/contents/android/blimp_view.h:79: // UPDATE COMMENT The
BlimpContentsImpl that this BlimpView is used for.
On 2016/09/13 04:47:24, Khushal wrote:
> That's a good idea. :P

;)

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/bli...
File blimp/client/public/blimp_client_context.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/bli...
blimp/client/public/blimp_client_context.h:67: virtual void
ConnectWithAssignment(const Assignment& assignment) = 0;
On 2016/09/13 04:47:24, Khushal wrote:
> If its never meant to be used in production code, add ForTesting to this?

It's actually used by other methods internally when I cleaned up the connection
flow (Connect() -> ...... -> ConnectWithAssignment()).  I could add ForTesting
here and call a valid method internally, but that might be more ugly :(.  wdyt?

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/con...
File blimp/client/public/contents/blimp_contents_view.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/con...
blimp/client/public/contents/blimp_contents_view.h:30: public:
On 2016/09/13 04:47:24, Khushal wrote:
> Add a virtual dtor or this could leak memory.

Ah good catch thanks!

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/con...
blimp/client/public/contents/blimp_contents_view.h:50: BlimpContentsView() =
default;
On 2016/09/13 04:47:24, Khushal wrote:
> Is this necessary?

Apparently :/.

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/support/co...
File blimp/client/support/compositor/display_compositor.cc (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/support/co...
blimp/client/support/compositor/display_compositor.cc:131: context_provider_ =
nullptr;
On 2016/09/13 04:47:24, Khushal wrote:
> How about just doing context_provider_ = std::move(context_provider) here and
> doing an
> 
> if (context_provider_) in the next block?

Done.

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/support/co...
blimp/client/support/compositor/display_compositor.cc:142:
host_->SetVisible(true);
On 2016/09/13 04:47:24, Khushal wrote:
> Eek. Don't do these 2 lines.

Hahah yeah caught this earlier too.

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/support/co...
File blimp/client/support/compositor/display_compositor.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/support/co...
blimp/client/support/compositor/display_compositor.h:29: class DisplayCompositor
: public cc::LayerTreeHostClient,
On 2016/09/13 04:47:24, Khushal wrote:
> I'd suggest naming this as Parent/Root compositor which produces the root
> surface for the display. There is already a component display_compositor, and
> this has nothing to do with it. No moar confusing names!

Sigh but sadly this is a compositor for the display.  I could call it I don't
think Parent/Root makes it clear what it's doing (parent and root don't really
imply surface or display to me).

Let's talk tomorrow I'm sure we can come up with a good name :)!

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/test/compo...
File blimp/client/test/compositor/test_display_compositor.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/test/compo...
blimp/client/test/compositor/test_display_compositor.h:42: void
GetBitmap(CopyRequest* request);
On 2016/09/13 04:47:24, Khushal wrote:
> I know you really wanted a sync call but I would say this half here, half
there
> WaitableEvent logic feels prone to errors later, might be cleaner to put it
all
> on the caller and make an async API here. Up to you though.

This is all gone.  Moved to BlimpContentsView like we discussed.  All async.

https://codereview.chromium.org/2320923002/diff/20001/blimp/engine/browser_te...
File blimp/engine/browser_tests/integration_test.cc (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/engine/browser_te...
blimp/engine/browser_tests/integration_test.cc:35: explicit
PageLoadWaitBlimpContentsObserver(client::BlimpContents* contents)
On 2016/09/13 04:47:24, Khushal wrote:
> How about just creating this object after you load the URL and having it
> internally create the waiter and pump the messages till it is signaled?

Wait logic cleaned up a bit.  Let's talk about this tomorrow?

https://codereview.chromium.org/2320923002/diff/20001/blimp/engine/browser_te...
blimp/engine/browser_tests/integration_test.cc:77:
base::MakeUnique<client::MockCompositorDependencies>();
On 2016/09/13 04:47:24, Khushal wrote:
> We can use the real BlimpCompositorDependencies here right?

Acknowledged.

[David Trainor- moved to gerrit, 2016-09-13 18:57:05]

dtrainor@chromium.org changed reviewers:
+ jam@chromium.org

[David Trainor- moved to gerrit, 2016-09-13 18:57:06]

jam@: Ptal at base/thread_restrictions.h.  Thanks!

[Khushal, 2016-09-13 23:47:10]

lgtm.

Looks great. My comments were mostly nits. Thanks!

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/bli...
File blimp/client/public/blimp_client_context.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/bli...
blimp/client/public/blimp_client_context.h:67: virtual void
ConnectWithAssignment(const Assignment& assignment) = 0;
On 2016/09/13 06:18:04, David Trainor wrote:
> On 2016/09/13 04:47:24, Khushal wrote:
> > If its never meant to be used in production code, add ForTesting to this?
> 
> It's actually used by other methods internally when I cleaned up the
connection
> flow (Connect() -> ...... -> ConnectWithAssignment()).  I could add ForTesting
> here and call a valid method internally, but that might be more ugly :(. 
wdyt?

Its fine, doesn't look like something too bad to be on the public API.

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/con...
File blimp/client/public/contents/blimp_contents_view.h (right):

https://codereview.chromium.org/2320923002/diff/20001/blimp/client/public/con...
blimp/client/public/contents/blimp_contents_view.h:50: BlimpContentsView() =
default;
On 2016/09/13 06:18:04, David Trainor wrote:
> On 2016/09/13 04:47:24, Khushal wrote:
> > Is this necessary?
> 
> Apparently :/.

Really? A lot of our other interfaces don't have ctors?

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/compo...
File blimp/client/core/compositor/blimp_compositor.h (right):

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/compo...
blimp/client/core/compositor/blimp_compositor.h:104: void
NotifyWhenDonePendingCommits(base::Closure callback);
May be add, if there are no pending commits, the callback will be run
synchronously.

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/conte...
File blimp/client/core/contents/blimp_contents_view_impl.cc (right):

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/conte...
blimp/client/core/contents/blimp_contents_view_impl.cc:22:
contents_layer_(contents_layer),
nit: std::move is nicer.

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/conte...
blimp/client/core/contents/blimp_contents_view_impl.cc:23:
weak_ptr_factory_(this) {}
DCHECK content_layer_ and blimp_contents?

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/conte...
blimp/client/core/contents/blimp_contents_view_impl.cc:45: if
(!readback_callback_.is_null() || !contents_layer_) {
We should always have a content_layer_ here. This layer is just a container for
the surface layer from the current compositor.

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/public/con...
File blimp/client/public/contents/blimp_contents_view.h (right):

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/public/con...
blimp/client/public/contents/blimp_contents_view.h:66: virtual void
CopyFromCompositingSurface(
This looks fine for now. Basically we want to expose 2 options, you can either
get the frame being currently drawn or you can say, process updates from the
engine-side renderer(if any) and then get that.

Also I feel like tests might have to get more intrusive going forward. So might
be best to let blimp/test use core directly and keep test only APIs internal.
WDYT?

https://codereview.chromium.org/2320923002/diff/80001/blimp/engine/browser_te...
File blimp/engine/browser_tests/waitable_content_pump.cc (right):

https://codereview.chromium.org/2320923002/diff/80001/blimp/engine/browser_te...
blimp/engine/browser_tests/waitable_content_pump.cc:19: WaitFor(event());
just &event_ here.

https://codereview.chromium.org/2320923002/diff/80001/blimp/engine/browser_te...
File blimp/engine/browser_tests/waitable_content_pump.h (right):

https://codereview.chromium.org/2320923002/diff/80001/blimp/engine/browser_te...
blimp/engine/browser_tests/waitable_content_pump.h:20: // waiter.Wait();
Awesome comment. Thanks!

https://codereview.chromium.org/2320923002/diff/80001/blimp/engine/browser_te...
blimp/engine/browser_tests/waitable_content_pump.h:33: void
WaitFor(base::WaitableEvent* event);
If this is meant to be just a helper method for others to use, make it static?

[David Trainor- moved to gerrit, 2016-09-14 00:31:09]

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/compo...
File blimp/client/core/compositor/blimp_compositor.h (right):

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/compo...
blimp/client/core/compositor/blimp_compositor.h:104: void
NotifyWhenDonePendingCommits(base::Closure callback);
On 2016/09/13 23:47:10, Khushal wrote:
> May be add, if there are no pending commits, the callback will be run
> synchronously.

Done.

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/conte...
File blimp/client/core/contents/blimp_contents_view_impl.cc (right):

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/conte...
blimp/client/core/contents/blimp_contents_view_impl.cc:22:
contents_layer_(contents_layer),
On 2016/09/13 23:47:10, Khushal wrote:
> nit: std::move is nicer.

Done.

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/conte...
blimp/client/core/contents/blimp_contents_view_impl.cc:23:
weak_ptr_factory_(this) {}
On 2016/09/13 23:47:10, Khushal wrote:
> DCHECK content_layer_ and blimp_contents?

Done.

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/core/conte...
blimp/client/core/contents/blimp_contents_view_impl.cc:45: if
(!readback_callback_.is_null() || !contents_layer_) {
On 2016/09/13 23:47:10, Khushal wrote:
> We should always have a content_layer_ here. This layer is just a container
for
> the surface layer from the current compositor.

Done.

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/public/con...
File blimp/client/public/contents/blimp_contents_view.h (right):

https://codereview.chromium.org/2320923002/diff/80001/blimp/client/public/con...
blimp/client/public/contents/blimp_contents_view.h:66: virtual void
CopyFromCompositingSurface(
On 2016/09/13 23:47:10, Khushal wrote:
> This looks fine for now. Basically we want to expose 2 options, you can either
> get the frame being currently drawn or you can say, process updates from the
> engine-side renderer(if any) and then get that.
> 
> Also I feel like tests might have to get more intrusive going forward. So
might
> be best to let blimp/test use core directly and keep test only APIs internal.
> WDYT?

Discussed offline.  Removing flush_pending_commits.  Will leave public API for
now.

https://codereview.chromium.org/2320923002/diff/80001/blimp/engine/browser_te...
File blimp/engine/browser_tests/waitable_content_pump.cc (right):

https://codereview.chromium.org/2320923002/diff/80001/blimp/engine/browser_te...
blimp/engine/browser_tests/waitable_content_pump.cc:19: WaitFor(event());
On 2016/09/13 23:47:10, Khushal wrote:
> just &event_ here.

Done.

https://codereview.chromium.org/2320923002/diff/80001/blimp/engine/browser_te...
File blimp/engine/browser_tests/waitable_content_pump.h (right):

https://codereview.chromium.org/2320923002/diff/80001/blimp/engine/browser_te...
blimp/engine/browser_tests/waitable_content_pump.h:20: // waiter.Wait();
On 2016/09/13 23:47:10, Khushal wrote:
> Awesome comment. Thanks!

Acknowledged.

https://codereview.chromium.org/2320923002/diff/80001/blimp/engine/browser_te...
blimp/engine/browser_tests/waitable_content_pump.h:33: void
WaitFor(base::WaitableEvent* event);
On 2016/09/13 23:47:10, Khushal wrote:
> If this is meant to be just a helper method for others to use, make it static?

Done.

[David Trainor- moved to gerrit, 2016-09-14 00:31:32]

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

[commit-bot: I haz the power, 2016-09-14 00:32:04]

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, 2016-09-14 00:36:48]

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

[commit-bot: I haz the power, 2016-09-14 00:36:49]

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...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[David Trainor- moved to gerrit, 2016-09-14 04:23:21]

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

[commit-bot: I haz the power, 2016-09-14 04:23:39]

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, 2016-09-14 04:27:38]

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

[commit-bot: I haz the power, 2016-09-14 04:27:39]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[David Trainor- moved to gerrit, 2016-09-14 15:17:18]

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

[commit-bot: I haz the power, 2016-09-14 15:17:30]

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, 2016-09-14 15:26:25]

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

[commit-bot: I haz the power, 2016-09-14 15:26:26]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[David Trainor- moved to gerrit, 2016-09-14 16:57:51]

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

[commit-bot: I haz the power, 2016-09-14 16:58:08]

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, 2016-09-14 17:07:27]

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

[commit-bot: I haz the power, 2016-09-14 17:07:27]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[David Trainor- moved to gerrit, 2016-09-14 17:49:46]

jam: Friendly ping r.e. base/threading/thread_restrictions.  Thanks!

[jam, 2016-09-14 18:03:49]

On 2016/09/14 17:49:46, David Trainor wrote:
> jam: Friendly ping r.e. base/threading/thread_restrictions.  Thanks!

lgtm

[David Trainor- moved to gerrit, 2016-09-14 18:43:51]

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

[commit-bot: I haz the power, 2016-09-14 18:44:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[David Trainor- moved to gerrit, 2016-09-14 18:49:58]

Description was changed from

==========
Add a full Blimp integration test.

- Add a full integration test that spawns both an engine and client in
  the same process and validates that the rendering pipeline produces
  the correct thing.
- Also add Linux support for BlimpContents and exposed a
  BlimpContentsView, which is required to gain access to the right
  classes to validate the rendering path.

BUG=644262
==========

to

==========
Add a full Blimp integration test.

- Add a full integration test that spawns both an engine and client in
  the same process and validates that the rendering pipeline produces
  the correct thing.
- Also add Linux support for BlimpContents and exposed a
  BlimpContentsView, which is required to gain access to the right
  classes to validate the rendering path.

Marking this as NOTRY because there are some ScopedAllowIO calls during test
setup and client tear down.

BUG=644262
NOTRY=true
==========

[commit-bot: I haz the power, 2016-09-14 18:54:31]

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

[commit-bot: I haz the power, 2016-09-14 18:54:33]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[David Trainor- moved to gerrit, 2016-09-14 18:56:09]

Description was changed from

==========
Add a full Blimp integration test.

- Add a full integration test that spawns both an engine and client in
  the same process and validates that the rendering pipeline produces
  the correct thing.
- Also add Linux support for BlimpContents and exposed a
  BlimpContentsView, which is required to gain access to the right
  classes to validate the rendering path.

Marking this as NOTRY because there are some ScopedAllowIO calls during test
setup and client tear down.

BUG=644262
NOTRY=true
==========

to

==========
Add a full Blimp integration test.

- Add a full integration test that spawns both an engine and client in
  the same process and validates that the rendering pipeline produces
  the correct thing.
- Also add Linux support for BlimpContents and exposed a
  BlimpContentsView, which is required to gain access to the right
  classes to validate the rendering path.

Marking this as NOTRY because there are some ScopedAllowIO calls during test
setup and client tear down.  Please feel free to revert or ping me on any
landing issues.

BUG=644262
NOTRY=true
==========

[David Trainor- moved to gerrit, 2016-09-14 18:56:52]

The CQ bit was checked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2016-09-14 18:56:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
khushalsagar@chromium.org
Link to the patchset: https://codereview.chromium.org/2320923002/#ps160001
(title: "Fix build break with chrome embedder")

[commit-bot: I haz the power, 2016-09-14 18:57:26]

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

[David Trainor- moved to gerrit, 2016-09-14 19:08:19]

Description was changed from

==========
Add a full Blimp integration test.

- Add a full integration test that spawns both an engine and client in
  the same process and validates that the rendering pipeline produces
  the correct thing.
- Also add Linux support for BlimpContents and exposed a
  BlimpContentsView, which is required to gain access to the right
  classes to validate the rendering path.

Marking this as NOTRY because there are some ScopedAllowIO calls during test
setup and client tear down.  Please feel free to revert or ping me on any
landing issues.

BUG=644262
NOTRY=true
==========

to

==========
Add a full Blimp integration test.

- Add a full integration test that spawns both an engine and client in
  the same process and validates that the rendering pipeline produces
  the correct thing.
- Also add Linux support for BlimpContents and exposed a
  BlimpContentsView, which is required to gain access to the right
  classes to validate the rendering path.

Marking this as NOPRESUBMIT because there are some ScopedAllowIO calls during
test setup and client tear down.  Please feel free to revert or ping me on any
landing issues.

BUG=644262
NOPRESUBMIT=true
==========

[David Trainor- moved to gerrit, 2016-09-14 19:08:31]

The CQ bit was unchecked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2016-09-14 19:08:39]

The CQ bit was checked by dtrainor@chromium.org

[commit-bot: I haz the power, 2016-09-14 19:09:24]

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, 2016-09-14 19:12:00]

Description was changed from

==========
Add a full Blimp integration test.

- Add a full integration test that spawns both an engine and client in
  the same process and validates that the rendering pipeline produces
  the correct thing.
- Also add Linux support for BlimpContents and exposed a
  BlimpContentsView, which is required to gain access to the right
  classes to validate the rendering path.

Marking this as NOPRESUBMIT because there are some ScopedAllowIO calls during
test setup and client tear down.  Please feel free to revert or ping me on any
landing issues.

BUG=644262
NOPRESUBMIT=true
==========

to

==========
Add a full Blimp integration test.

- Add a full integration test that spawns both an engine and client in
  the same process and validates that the rendering pipeline produces
  the correct thing.
- Also add Linux support for BlimpContents and exposed a
  BlimpContentsView, which is required to gain access to the right
  classes to validate the rendering path.

Marking this as NOPRESUBMIT because there are some ScopedAllowIO calls during
test setup and client tear down.  Please feel free to revert or ping me on any
landing issues.

BUG=644262
NOPRESUBMIT=true
==========

[commit-bot: I haz the power, 2016-09-14 19:12:05]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-09-14 19:14:12]

Description was changed from

==========
Add a full Blimp integration test.

- Add a full integration test that spawns both an engine and client in
  the same process and validates that the rendering pipeline produces
  the correct thing.
- Also add Linux support for BlimpContents and exposed a
  BlimpContentsView, which is required to gain access to the right
  classes to validate the rendering path.

Marking this as NOPRESUBMIT because there are some ScopedAllowIO calls during
test setup and client tear down.  Please feel free to revert or ping me on any
landing issues.

BUG=644262
NOPRESUBMIT=true
==========

to

==========
Add a full Blimp integration test.

- Add a full integration test that spawns both an engine and client in
  the same process and validates that the rendering pipeline produces
  the correct thing.
- Also add Linux support for BlimpContents and exposed a
  BlimpContentsView, which is required to gain access to the right
  classes to validate the rendering path.

Marking this as NOPRESUBMIT because there are some ScopedAllowIO calls during
test setup and client tear down.  Please feel free to revert or ping me on any
landing issues.

BUG=644262
NOPRESUBMIT=true

Committed: https://crrev.com/452f128c9232adc0c58caef4564f2fbf1b4bd9b0
Cr-Commit-Position: refs/heads/master@{#418633}
==========

[commit-bot: I haz the power, 2016-09-14 19:14:14]

Patchset 9 (id:??) landed as
https://crrev.com/452f128c9232adc0c58caef4564f2fbf1b4bd9b0
Cr-Commit-Position: refs/heads/master@{#418633}