Add BlimpDocument, pull out functions in BlimpCompositor.

Add BlimpDocument, pull out functions in BlimpCompositor.

Previously BlimpCompositor is heavy weight class that also include
render_widget_id and input manager, and we have the following ownership:

BlimpContentImpl -> BlimpCompositorManager -> BlimpCompositor.

This CL:
1. Rename BlimpCompositorManager to BlimpDocumentManager.
2. Add BlimpDocument, which owns a BlimpCompositor. It matches the idea
of helium document. So more feature with document scope can goes in
this layer.
3. Unit test for new class, and also rename old unit tests.

The ownership chains now become the following:

BlimpContentImpl->BlimpDocumentManager->BlimpDocument->BlimpCompositor.

BUG=644326
Committed: https://crrev.com/d78db80d1a9953c01fe68f94adee0f4460cb2caf
Cr-Commit-Position: refs/heads/master@{#423629}

[xingliu, 2016-09-30 22:09:22]

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

[commit-bot: I haz the power, 2016-09-30 22:10:02]

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

[xingliu, 2016-09-30 23:11:38]

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

[commit-bot: I haz the power, 2016-09-30 23:12:21]

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

[xingliu, 2016-09-30 23:14:01]

xingliu@chromium.org changed reviewers:
+ dtrainor@chromium.org, khushalsagar@chromium.org

[xingliu, 2016-09-30 23:14:01]

Added BlimpDocument, renamed BlimpCompositorManager.

[commit-bot: I haz the power, 2016-10-01 00:51:09]

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

[commit-bot: I haz the power, 2016-10-01 00:51:10]

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

[Khushal, 2016-10-03 20:05:00]

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/DEPS
File blimp/client/DEPS (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/DEPS#newco...
blimp/client/DEPS:16: "+third_party/WebKit/public/platform/WebInputEvent.h",
Why did we need to change this?

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/feedb...
File blimp/client/core/feedback/BUILD.gn (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/feedb...
blimp/client/core/feedback/BUILD.gn:21: "//skia",
Is skia needed here?

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
File blimp/client/core/render_widget/blimp_document.cc (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document.cc:28: compositor_.reset();
nit: Don't need to reset.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
File blimp/client/core/render_widget/blimp_document.h (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document.h:46: const int
render_widget_id_;
Should we rename this to document id in that case?

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document.h:53: BlimpDocumentManager*
manager_;
You can just comment about why this is here, used to send messages to
corresponding render widget instance on the engine for this document.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
File blimp/client/core/render_widget/blimp_document_manager.h (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager.h:25: // 2. The
compositor corresponding to the document initialized on the engine
"The document corresponding to the render widget initialized..."?

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager.h:28: // output surface
from this document to draw to the view. All events from the
You can remove the "Only..draw to the view" part here. Its no longer true.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager.h:97: // visible and has
the |window_|. It corresponds to the
Can remove the "and has the window" part. No longer true.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
File blimp/client/core/render_widget/blimp_document_manager_unittest.cc (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager_unittest.cc:66: explicit
BlimpDocumentManagerForTesting(
Do you still need the explicit?

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager_unittest.cc:105: void
SetUpCompositors() {
SetUpDocuments?

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager_unittest.cc:205:
delegate()->OnRenderWidgetInitialized(2);
While we are at it, may be we could verify that the layer returned by the
document manager holds the content layer for the current document?

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/test/rende...
File blimp/client/test/render_widget/mock_render_widget_feature.cc (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/test/rende...
blimp/client/test/render_widget/mock_render_widget_feature.cc:11:
MockRenderWidgetFeature::~MockRenderWidgetFeature() = default;
nit: new line.

[xingliu, 2016-10-04 18:27:34]

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

[commit-bot: I haz the power, 2016-10-04 18:27:58]

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

[xingliu, 2016-10-04 18:33:50]

Fixes based on review.

Rename render_widget_id to document_id.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/DEPS
File blimp/client/DEPS (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/DEPS#newco...
blimp/client/DEPS:16: "+third_party/WebKit/public/platform/WebInputEvent.h",
On 2016/10/03 20:04:58, Khushal wrote:
> Why did we need to change this?

Someone moved the sources of Webkit.

Probably this crbug: crbug.com/625684

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/feedb...
File blimp/client/core/feedback/BUILD.gn (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/feedb...
blimp/client/core/feedback/BUILD.gn:21: "//skia",
On 2016/10/03 20:04:59, Khushal wrote:
> Is skia needed here?

Removed, I think I got gn check failed before rebase, weird.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
File blimp/client/core/render_widget/blimp_document.cc (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document.cc:28: compositor_.reset();
On 2016/10/03 20:04:59, Khushal wrote:
> nit: Don't need to reset.

Done.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
File blimp/client/core/render_widget/blimp_document.h (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document.h:46: const int
render_widget_id_;
On 2016/10/03 20:04:59, Khushal wrote:
> Should we rename this to document id in that case?

Rename most of it, except the functions parameters in
RenderWidgetFeature::RenderWidgetFeatureDelegate.

Also added some comment of matching between document_id and render_widget_id.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document.h:53: BlimpDocumentManager*
manager_;
On 2016/10/03 20:04:59, Khushal wrote:
> You can just comment about why this is here, used to send messages to
> corresponding render widget instance on the engine for this document.

Done.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
File blimp/client/core/render_widget/blimp_document_manager.h (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager.h:25: // 2. The
compositor corresponding to the document initialized on the engine
On 2016/10/03 20:04:59, Khushal wrote:
> "The document corresponding to the render widget initialized..."?

Done.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager.h:28: // output surface
from this document to draw to the view. All events from the
On 2016/10/03 20:04:59, Khushal wrote:
> You can remove the "Only..draw to the view" part here. Its no longer true.

Done.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager.h:97: // visible and has
the |window_|. It corresponds to the
On 2016/10/03 20:04:59, Khushal wrote:
> Can remove the "and has the window" part. No longer true.

Done.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
File blimp/client/core/render_widget/blimp_document_manager_unittest.cc (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager_unittest.cc:66: explicit
BlimpDocumentManagerForTesting(
On 2016/10/03 20:04:59, Khushal wrote:
> Do you still need the explicit?

Done.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager_unittest.cc:105: void
SetUpCompositors() {
On 2016/10/03 20:04:59, Khushal wrote:
> SetUpDocuments?

Done.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document_manager_unittest.cc:205:
delegate()->OnRenderWidgetInitialized(2);
On 2016/10/03 20:04:59, Khushal wrote:
> While we are at it, may be we could verify that the layer returned by the
> document manager holds the content layer for the current document?

Done. Makes sense.

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/test/rende...
File blimp/client/test/render_widget/mock_render_widget_feature.cc (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/test/rende...
blimp/client/test/render_widget/mock_render_widget_feature.cc:11:
MockRenderWidgetFeature::~MockRenderWidgetFeature() = default;
On 2016/10/03 20:04:59, Khushal wrote:
> nit: new line.

Done.

[Khushal, 2016-10-04 18:49:54]

lgtm. Thanks!

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
File blimp/client/core/render_widget/blimp_document.h (right):

https://codereview.chromium.org/2382733007/diff/40001/blimp/client/core/rende...
blimp/client/core/render_widget/blimp_document.h:46: const int
render_widget_id_;
On 2016/10/04 18:33:49, xingliu wrote:
> On 2016/10/03 20:04:59, Khushal wrote:
> > Should we rename this to document id in that case?
> 
> Rename most of it, except the functions parameters in
> RenderWidgetFeature::RenderWidgetFeatureDelegate.
> 
> Also added some comment of matching between document_id and render_widget_id.

Sounds good. The RenderWidgetFeature should die once Helium steps in.

https://codereview.chromium.org/2382733007/diff/100001/blimp/client/core/rend...
File blimp/client/core/render_widget/blimp_document_manager_unittest.cc (right):

https://codereview.chromium.org/2382733007/diff/100001/blimp/client/core/rend...
blimp/client/core/render_widget/blimp_document_manager_unittest.cc:198:
delegate()->OnRenderWidgetInitialized(1);
Add a
EXPECT_EQ(document_manager_->layer(), mock_compositor1_->layer()->parent()) here
too.

and EXPECT_EQ(document_manager_->layer()->children().size(), 1), since that
should be the only layer attached (this when the 2nd one is initialized also).

[commit-bot: I haz the power, 2016-10-04 19:27:21]

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

[commit-bot: I haz the power, 2016-10-04 19:27:22]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[xingliu, 2016-10-04 20:17:49]

Thanks for the feedback, added some more test checks on compositor layers.

https://codereview.chromium.org/2382733007/diff/100001/blimp/client/core/rend...
File blimp/client/core/render_widget/blimp_document_manager_unittest.cc (right):

https://codereview.chromium.org/2382733007/diff/100001/blimp/client/core/rend...
blimp/client/core/render_widget/blimp_document_manager_unittest.cc:198:
delegate()->OnRenderWidgetInitialized(1);
On 2016/10/04 18:49:54, Khushal wrote:
> Add a
> EXPECT_EQ(document_manager_->layer(), mock_compositor1_->layer()->parent())
here
> too.
> 
> and EXPECT_EQ(document_manager_->layer()->children().size(), 1), since that
> should be the only layer attached (this when the 2nd one is initialized also).

Done, also add a check after documents are deleted.

[xingliu, 2016-10-04 20:17:59]

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

[commit-bot: I haz the power, 2016-10-04 20:18:32]

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

[xingliu, 2016-10-04 20:35:10]

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

[commit-bot: I haz the power, 2016-10-04 20:35:38]

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-10-04 22:29:02]

https://codereview.chromium.org/2382733007/diff/120001/blimp/client/core/rend...
File blimp/client/core/render_widget/blimp_document_manager.h (right):

https://codereview.chromium.org/2382733007/diff/120001/blimp/client/core/rend...
blimp/client/core/render_widget/blimp_document_manager.h:21: // The
BlimpDocumentManager
Suggest:
The BlimpDocumentManager is responsible for:
1. Managing ...
2. The compositor ...

https://codereview.chromium.org/2382733007/diff/120001/blimp/client/test/rend...
File blimp/client/test/render_widget/mock_render_widget_feature.h (right):

https://codereview.chromium.org/2382733007/diff/120001/blimp/client/test/rend...
blimp/client/test/render_widget/mock_render_widget_feature.h:18: class
MockRenderWidgetFeature : public RenderWidgetFeature {
Should this live near RenderWidgetFeature?

[commit-bot: I haz the power, 2016-10-04 22:43:09]

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

[commit-bot: I haz the power, 2016-10-04 22:43:09]

Dry run: This issue passed the CQ dry run.

[xingliu, 2016-10-05 00:37:30]

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

[commit-bot: I haz the power, 2016-10-05 00:37:59]

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

[xingliu, 2016-10-05 00:40:08]

Thanks for the feedback, moved mock_render_widget_feature to core/render_widget.

https://codereview.chromium.org/2382733007/diff/120001/blimp/client/core/rend...
File blimp/client/core/render_widget/blimp_document_manager.h (right):

https://codereview.chromium.org/2382733007/diff/120001/blimp/client/core/rend...
blimp/client/core/render_widget/blimp_document_manager.h:21: // The
BlimpDocumentManager
On 2016/10/04 22:29:02, David Trainor wrote:
> Suggest:
> The BlimpDocumentManager is responsible for:
> 1. Managing ...
> 2. The compositor ...

Done.

https://codereview.chromium.org/2382733007/diff/120001/blimp/client/test/rend...
File blimp/client/test/render_widget/mock_render_widget_feature.h (right):

https://codereview.chromium.org/2382733007/diff/120001/blimp/client/test/rend...
blimp/client/test/render_widget/mock_render_widget_feature.h:18: class
MockRenderWidgetFeature : public RenderWidgetFeature {
On 2016/10/04 22:29:02, David Trainor wrote:
> Should this live near RenderWidgetFeature?

Makes sense, seems only two core unit tests depend on this class.

[commit-bot: I haz the power, 2016-10-05 01:37:09]

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

[commit-bot: I haz the power, 2016-10-05 01:37:10]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2016-10-05 16:29:28]

lgtm!

[xingliu, 2016-10-05 20:52:22]

The CQ bit was checked by xingliu@chromium.org

[xingliu, 2016-10-05 20:52:22]

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/2382733007/#ps180001
(title: "Clean up build file.")

[commit-bot: I haz the power, 2016-10-05 20:53:02]

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-10-05 21:05:16]

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

[commit-bot: I haz the power, 2016-10-05 21:05:17]

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...)

[xingliu, 2016-10-05 21:56:16]

xingliu@chromium.org changed reviewers:
+ pdr@chromium.org

[xingliu, 2016-10-05 21:56:17]

@pdr, please help review blimp/client/DEPS.

[pdr., 2016-10-05 22:33:34]

pdr@chromium.org changed reviewers:
+ kouhei@chromium.org
- pdr@chromium.org

[pdr., 2016-10-05 22:33:54]

On 2016/10/05 at 21:56:17, xingliu wrote:
> @pdr, please help review blimp/client/DEPS.

I'm sorry, I'm not a particularly good reviewer for blimp. I've added Kouhei as
a reviewer.

[xingliu, 2016-10-05 23:12:00]

On 2016/10/05 22:33:54, pdr. wrote:
> On 2016/10/05 at 21:56:17, xingliu wrote:
> > @pdr, please help review blimp/client/DEPS.
> 
> I'm sorry, I'm not a particularly good reviewer for blimp. I've added Kouhei
as
> a reviewer.

Thanks, we include third_party/WebKit/public/platform/WebInputEvent.h in the
DEPS file, that's the only part left for review I think. :)

Don't need to review all the files, but only the DEPS file.

[kouhei (in TOK), 2016-10-05 23:16:29]

https://codereview.chromium.org/2382733007/diff/180001/blimp/client/DEPS
File blimp/client/DEPS (right):

https://codereview.chromium.org/2382733007/diff/180001/blimp/client/DEPS#newc...
blimp/client/DEPS:16: "+third_party/WebKit/public/platform/WebInputEvent.h",
This lgtm. However I'm not sure how this relates to the other changes.
Maybe worth splitting the CL for this change?

Please add ref for crbug.com/625684 in CL desc.

[xingliu, 2016-10-05 23:31:10]

Description was changed from

==========
Add BlimpDocument, pull out functions in BlimpCompositor.

Previously BlimpCompositor is heavy weight class that also include
render_widget_id and input manager, and we have the following ownership:

BlimpContentImpl -> BlimpCompositorManager -> BlimpCompositor.


This CL:
1. Rename BlimpCompositorManager to BlimpDocumentManager.
2. Add BlimpDocument, which owns a BlimpCompositor. It matches the idea
of helium document. So more feature with document scope can goes in
this layer.
3. Unit test for new class, and also rename old unit tests.

The ownership chains now become the following:

BlimpContentImpl->BlimpDocumentManager->BlimpDocument->BlimpCompositor.


BUG=644326
==========

to

==========
Add BlimpDocument, pull out functions in BlimpCompositor.

Previously BlimpCompositor is heavy weight class that also include
render_widget_id and input manager, and we have the following ownership:

BlimpContentImpl -> BlimpCompositorManager -> BlimpCompositor.


This CL:
1. Rename BlimpCompositorManager to BlimpDocumentManager.
2. Add BlimpDocument, which owns a BlimpCompositor. It matches the idea
of helium document. So more feature with document scope can goes in
this layer.
3. Unit test for new class, and also rename old unit tests.

The ownership chains now become the following:

BlimpContentImpl->BlimpDocumentManager->BlimpDocument->BlimpCompositor.

BUG=644326


Also change the blimp/client/DEPS file for path change of WebInputEvent.h.
BUG=625684
==========

[xingliu, 2016-10-05 23:31:46]

Description was changed from

==========
Add BlimpDocument, pull out functions in BlimpCompositor.

Previously BlimpCompositor is heavy weight class that also include
render_widget_id and input manager, and we have the following ownership:

BlimpContentImpl -> BlimpCompositorManager -> BlimpCompositor.


This CL:
1. Rename BlimpCompositorManager to BlimpDocumentManager.
2. Add BlimpDocument, which owns a BlimpCompositor. It matches the idea
of helium document. So more feature with document scope can goes in
this layer.
3. Unit test for new class, and also rename old unit tests.

The ownership chains now become the following:

BlimpContentImpl->BlimpDocumentManager->BlimpDocument->BlimpCompositor.

BUG=644326


Also change the blimp/client/DEPS file for path change of WebInputEvent.h.
BUG=625684
==========

to

==========
Add BlimpDocument, pull out functions in BlimpCompositor.

Previously BlimpCompositor is heavy weight class that also include
render_widget_id and input manager, and we have the following ownership:

BlimpContentImpl -> BlimpCompositorManager -> BlimpCompositor.


This CL:
1. Rename BlimpCompositorManager to BlimpDocumentManager.
2. Add BlimpDocument, which owns a BlimpCompositor. It matches the idea
of helium document. So more feature with document scope can goes in
this layer.
3. Unit test for new class, and also rename old unit tests.

The ownership chains now become the following:

BlimpContentImpl->BlimpDocumentManager->BlimpDocument->BlimpCompositor.

BUG=644326

Also change the blimp/client/DEPS file for path change of WebInputEvent.h.
BUG=625684
==========

[xingliu, 2016-10-05 23:41:09]

If everything looks fine, then I'll prepare to put it into CQ.

https://codereview.chromium.org/2382733007/diff/180001/blimp/client/DEPS
File blimp/client/DEPS (right):

https://codereview.chromium.org/2382733007/diff/180001/blimp/client/DEPS#newc...
blimp/client/DEPS:16: "+third_party/WebKit/public/platform/WebInputEvent.h",
On 2016/10/05 23:16:29, kouhei wrote:
> This lgtm. However I'm not sure how this relates to the other changes.
> Maybe worth splitting the CL for this change?
> 
> Please add ref for crbug.com/625684 in CL desc.

Added description to link to bug 625684.

Only the include is changed, because of gn check error or compiling error during
development.

[xingliu, 2016-10-06 01:57:29]

The CQ bit was checked by xingliu@chromium.org

[commit-bot: I haz the power, 2016-10-06 01:58:06]

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-10-06 02:06:31]

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

[commit-bot: I haz the power, 2016-10-06 02:06:33]

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...)

[xingliu, 2016-10-06 02:11:34]

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

[commit-bot: I haz the power, 2016-10-06 02:12:06]

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-10-06 03:02:23]

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

[commit-bot: I haz the power, 2016-10-06 03:02:28]

Dry run: This issue passed the CQ dry run.

[xingliu, 2016-10-06 17:14:02]

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

[commit-bot: I haz the power, 2016-10-06 17:14:37]

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-10-06 19:09:40]

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

[commit-bot: I haz the power, 2016-10-06 19:09:41]

Dry run: This issue passed the CQ dry run.

[xingliu, 2016-10-06 19:21:27]

Description was changed from

==========
Add BlimpDocument, pull out functions in BlimpCompositor.

Previously BlimpCompositor is heavy weight class that also include
render_widget_id and input manager, and we have the following ownership:

BlimpContentImpl -> BlimpCompositorManager -> BlimpCompositor.


This CL:
1. Rename BlimpCompositorManager to BlimpDocumentManager.
2. Add BlimpDocument, which owns a BlimpCompositor. It matches the idea
of helium document. So more feature with document scope can goes in
this layer.
3. Unit test for new class, and also rename old unit tests.

The ownership chains now become the following:

BlimpContentImpl->BlimpDocumentManager->BlimpDocument->BlimpCompositor.

BUG=644326

Also change the blimp/client/DEPS file for path change of WebInputEvent.h.
BUG=625684
==========

to

==========
Add BlimpDocument, pull out functions in BlimpCompositor.

Previously BlimpCompositor is heavy weight class that also include
render_widget_id and input manager, and we have the following ownership:

BlimpContentImpl -> BlimpCompositorManager -> BlimpCompositor.


This CL:
1. Rename BlimpCompositorManager to BlimpDocumentManager.
2. Add BlimpDocument, which owns a BlimpCompositor. It matches the idea
of helium document. So more feature with document scope can goes in
this layer.
3. Unit test for new class, and also rename old unit tests.

The ownership chains now become the following:

BlimpContentImpl->BlimpDocumentManager->BlimpDocument->BlimpCompositor.

BUG=644326
==========

[xingliu, 2016-10-06 19:23:01]

On 2016/10/05 23:16:29, kouhei wrote:
> https://codereview.chromium.org/2382733007/diff/180001/blimp/client/DEPS
> File blimp/client/DEPS (right):
> 
>
https://codereview.chromium.org/2382733007/diff/180001/blimp/client/DEPS#newc...
> blimp/client/DEPS:16: "+third_party/WebKit/public/platform/WebInputEvent.h",
> This lgtm. However I'm not sure how this relates to the other changes.
> Maybe worth splitting the CL for this change?
> 
> Please add ref for crbug.com/625684 in CL desc.

The include change related to WebInputEvent.h in this CL has been removed, also
removed the link
to crbug.com/625684 in the description.

[xingliu, 2016-10-06 19:23:14]

The CQ bit was checked by xingliu@chromium.org

[xingliu, 2016-10-06 19:23:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, kouhei@chromium.org, khushalsagar@chromium.org
Link to the patchset: https://codereview.chromium.org/2382733007/#ps200001
(title: "Remove Webkit DEPS modification.")

[commit-bot: I haz the power, 2016-10-06 19:23:51]

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-10-06 19:34:34]

Description was changed from

==========
Add BlimpDocument, pull out functions in BlimpCompositor.

Previously BlimpCompositor is heavy weight class that also include
render_widget_id and input manager, and we have the following ownership:

BlimpContentImpl -> BlimpCompositorManager -> BlimpCompositor.


This CL:
1. Rename BlimpCompositorManager to BlimpDocumentManager.
2. Add BlimpDocument, which owns a BlimpCompositor. It matches the idea
of helium document. So more feature with document scope can goes in
this layer.
3. Unit test for new class, and also rename old unit tests.

The ownership chains now become the following:

BlimpContentImpl->BlimpDocumentManager->BlimpDocument->BlimpCompositor.

BUG=644326
==========

to

==========
Add BlimpDocument, pull out functions in BlimpCompositor.

Previously BlimpCompositor is heavy weight class that also include
render_widget_id and input manager, and we have the following ownership:

BlimpContentImpl -> BlimpCompositorManager -> BlimpCompositor.


This CL:
1. Rename BlimpCompositorManager to BlimpDocumentManager.
2. Add BlimpDocument, which owns a BlimpCompositor. It matches the idea
of helium document. So more feature with document scope can goes in
this layer.
3. Unit test for new class, and also rename old unit tests.

The ownership chains now become the following:

BlimpContentImpl->BlimpDocumentManager->BlimpDocument->BlimpCompositor.

BUG=644326
==========

[commit-bot: I haz the power, 2016-10-06 19:34:36]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-10-06 19:39:17]

Description was changed from

==========
Add BlimpDocument, pull out functions in BlimpCompositor.

Previously BlimpCompositor is heavy weight class that also include
render_widget_id and input manager, and we have the following ownership:

BlimpContentImpl -> BlimpCompositorManager -> BlimpCompositor.


This CL:
1. Rename BlimpCompositorManager to BlimpDocumentManager.
2. Add BlimpDocument, which owns a BlimpCompositor. It matches the idea
of helium document. So more feature with document scope can goes in
this layer.
3. Unit test for new class, and also rename old unit tests.

The ownership chains now become the following:

BlimpContentImpl->BlimpDocumentManager->BlimpDocument->BlimpCompositor.

BUG=644326
==========

to

==========
Add BlimpDocument, pull out functions in BlimpCompositor.

Previously BlimpCompositor is heavy weight class that also include
render_widget_id and input manager, and we have the following ownership:

BlimpContentImpl -> BlimpCompositorManager -> BlimpCompositor.

This CL:
1. Rename BlimpCompositorManager to BlimpDocumentManager.
2. Add BlimpDocument, which owns a BlimpCompositor. It matches the idea
of helium document. So more feature with document scope can goes in
this layer.
3. Unit test for new class, and also rename old unit tests.

The ownership chains now become the following:

BlimpContentImpl->BlimpDocumentManager->BlimpDocument->BlimpCompositor.

BUG=644326

Committed: https://crrev.com/d78db80d1a9953c01fe68f94adee0f4460cb2caf
Cr-Commit-Position: refs/heads/master@{#423629}
==========

[commit-bot: I haz the power, 2016-10-06 19:39:19]

Patchset 11 (id:??) landed as
https://crrev.com/d78db80d1a9953c01fe68f94adee0f4460cb2caf
Cr-Commit-Position: refs/heads/master@{#423629}