cc: Chromify LayerTreeHostCommon

cc: Chromify LayerTreeHostCommon

NOTRY=true
R=danakj@chromium.org
BUG=none


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=189830

[danakj, 2013-03-20 17:27:14]

LGTM

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:39: TRACE_EVENT0("cc",
"layer_tree_host_common::sortLayers");
SortLayers

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:178: static inline bool
TransformToParentIsKnown(LayerImpl*) { return true; }
need var name

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:185: static inline bool
TransformToScreenIsKnown(LayerImpl*) { return true; }
var name

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:300: "LayerTreeHostCommon::requireSurface
flattening");
RequireSurface (technically SubtreeShouldRenderToSeparateSurface)

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:310: TRACE_EVENT_INSTANT0("cc",
"LayerTreeHostCommon::requireSurface clipping");
RequireSurface

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:325: TRACE_EVENT_INSTANT0("cc",
"LayerTreeHostCommon::requireSurface opacity");
RequireSurface

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:342: //     scroll_delta is actually applied.
indent this to line up with the --

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:482: layer->replica_layer() ?
layer->replica_layer()->mask_layer() : 0;
NULL

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:864: Scale(1.0 / layer->contents_scale_x(),
1.0 / layer->contents_scale_y());
.Scale( on the previous line?

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:872:
PreconcatTransform(layer_draw_properties.target_space_transform);
can PreconcatTransform( go on the prev line?

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:881: subtree_can_use_lcd_text &&
(accumulated_draw_opacity == 1.0) &&
1.f

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:916: combined_transform.Scale(1 /
render_surface_sublayer_scale.x(),
1.0

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:917: 1 / render_surface_sublayer_scale.y());
1.0

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:940: layer_draw_properties.opacity = 1;
1.f

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:983: // FIXME:  make this smarter for the
SkImageFilter case (check for
TODO

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1063: } else
{}

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1127:
!layer->render_surface()->layer_list().size()) {
!size() -> empty() >_>

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1133: // targetSurface space)
period

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1142: // space)
period

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1231: 1 / render_surface_sublayer_scale.x(),
1.0

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1232: 1 / render_surface_sublayer_scale.y());
1.0

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1404: // If projectPoint could not project to
a valid value, then we assume that
ProjectPoint()

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1434: // If projectPoint could not project to
a valid value, then we assume that
ProjectPoint()

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1448: // clipRects that are active.
clip_rects

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1457: // Note that drawableContentRects are
actually in targetSurface space, so
drawable_content_rects

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1457: // Note that drawableContentRects are
actually in targetSurface space, so
target_surface

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1567: ;
rm this

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
File cc/trees/layer_tree_host_common.h (right):

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.h:68: static Layer* get_child_as_raw_ptr(
These aren't class accessors, so I think they should be CapitializedStyle

[enne (OOO), 2013-03-20 20:22:07]

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:178: static inline bool
TransformToParentIsKnown(LayerImpl*) { return true; }
On 2013/03/20 17:27:14, danakj wrote:
> need var name

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:185: static inline bool
TransformToScreenIsKnown(LayerImpl*) { return true; }
On 2013/03/20 17:27:14, danakj wrote:
> var name

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:300: "LayerTreeHostCommon::requireSurface
flattening");
On 2013/03/20 17:27:14, danakj wrote:
> RequireSurface (technically SubtreeShouldRenderToSeparateSurface)

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:310: TRACE_EVENT_INSTANT0("cc",
"LayerTreeHostCommon::requireSurface clipping");
On 2013/03/20 17:27:14, danakj wrote:
> RequireSurface

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:310: TRACE_EVENT_INSTANT0("cc",
"LayerTreeHostCommon::requireSurface clipping");
On 2013/03/20 17:27:14, danakj wrote:
> RequireSurface

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:325: TRACE_EVENT_INSTANT0("cc",
"LayerTreeHostCommon::requireSurface opacity");
On 2013/03/20 17:27:14, danakj wrote:
> RequireSurface

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:342: //     scroll_delta is actually applied.
On 2013/03/20 17:27:14, danakj wrote:
> indent this to line up with the --

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:482: layer->replica_layer() ?
layer->replica_layer()->mask_layer() : 0;
On 2013/03/20 17:27:14, danakj wrote:
> NULL

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:864: Scale(1.0 / layer->contents_scale_x(),
1.0 / layer->contents_scale_y());
On 2013/03/20 17:27:14, danakj wrote:
> .Scale( on the previous line?

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:872:
PreconcatTransform(layer_draw_properties.target_space_transform);
On 2013/03/20 17:27:14, danakj wrote:
> can PreconcatTransform( go on the prev line?

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:872:
PreconcatTransform(layer_draw_properties.target_space_transform);
On 2013/03/20 17:27:14, danakj wrote:
> can PreconcatTransform( go on the prev line?

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:881: subtree_can_use_lcd_text &&
(accumulated_draw_opacity == 1.0) &&
On 2013/03/20 17:27:14, danakj wrote:
> 1.f

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:881: subtree_can_use_lcd_text &&
(accumulated_draw_opacity == 1.0) &&
On 2013/03/20 17:27:14, danakj wrote:
> 1.f

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:916: combined_transform.Scale(1 /
render_surface_sublayer_scale.x(),
On 2013/03/20 17:27:14, danakj wrote:
> 1.0

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:917: 1 / render_surface_sublayer_scale.y());
On 2013/03/20 17:27:14, danakj wrote:
> 1.0

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:940: layer_draw_properties.opacity = 1;
On 2013/03/20 17:27:14, danakj wrote:
> 1.f

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:983: // FIXME:  make this smarter for the
SkImageFilter case (check for
On 2013/03/20 17:27:14, danakj wrote:
> TODO

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1063: } else
On 2013/03/20 17:27:14, danakj wrote:
> {}

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1127:
!layer->render_surface()->layer_list().size()) {
On 2013/03/20 17:27:14, danakj wrote:
> !size() -> empty() >_>

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1127:
!layer->render_surface()->layer_list().size()) {
On 2013/03/20 17:27:14, danakj wrote:
> !size() -> empty() >_>

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1133: // targetSurface space)
On 2013/03/20 17:27:14, danakj wrote:
> period

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1142: // space)
On 2013/03/20 17:27:14, danakj wrote:
> period

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1231: 1 / render_surface_sublayer_scale.x(),
On 2013/03/20 17:27:14, danakj wrote:
> 1.0

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1232: 1 / render_surface_sublayer_scale.y());
On 2013/03/20 17:27:14, danakj wrote:
> 1.0

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1404: // If projectPoint could not project to
a valid value, then we assume that
On 2013/03/20 17:27:14, danakj wrote:
> ProjectPoint()

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1434: // If projectPoint could not project to
a valid value, then we assume that
On 2013/03/20 17:27:14, danakj wrote:
> ProjectPoint()

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1448: // clipRects that are active.
On 2013/03/20 17:27:14, danakj wrote:
> clip_rects

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1457: // Note that drawableContentRects are
actually in targetSurface space, so
On 2013/03/20 17:27:14, danakj wrote:
> drawable_content_rects

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1457: // Note that drawableContentRects are
actually in targetSurface space, so
On 2013/03/20 17:27:14, danakj wrote:
> target_surface

Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.cc:1567: ;
On 2013/03/20 17:27:14, danakj wrote:
> rm this

Weird.  I removed another of these elsewhere but missed this one.  Done.

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
File cc/trees/layer_tree_host_common.h (right):

https://codereview.chromium.org/12760011/diff/1/cc/trees/layer_tree_host_comm...
cc/trees/layer_tree_host_common.h:68: static Layer* get_child_as_raw_ptr(
On 2013/03/20 17:27:14, danakj wrote:
> These aren't class accessors, so I think they should be CapitializedStyle

Negatory.  hacker_style is for cheap functions, not only for accessors.  Isn't
that the argument for why we should use that style?

[commit-bot: I haz the power, 2013-03-20 20:25:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/12760011/6001

[commit-bot: I haz the power, 2013-03-20 20:25:27]

Failed to apply patch for cc/layers/layer_iterator.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/layers/layer_iterator.h
  Hunk #1 FAILED at 203.
  1 out of 1 hunk FAILED -- saving rejects to file
cc/layers/layer_iterator.h.rej

Patch:       cc/layers/layer_iterator.h
Index: cc/layers/layer_iterator.h
diff --git a/cc/layers/layer_iterator.h b/cc/layers/layer_iterator.h
index
2bdd5aebfde58bd3e4874fbb694f14002b4de5e1..9cf0d6f90dc2bc35dbb913f3d22afed1326a9d50
100644
--- a/cc/layers/layer_iterator.h
+++ b/cc/layers/layer_iterator.h
@@ -203,7 +203,7 @@ class LayerIterator {
   }
 
   inline bool current_layer_represents_contributing_render_surface() const {
-    return LayerTreeHostCommon::renderSurfaceContributesToTarget<LayerType>(
+    return LayerTreeHostCommon::RenderSurfaceContributesToTarget<LayerType>(
         current_layer(), target_render_surface_layer()->id());
   }
   inline bool current_layer_represents_target_render_surface() const {

[commit-bot: I haz the power, 2013-03-20 21:41:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/12760011/6001

[commit-bot: I haz the power, 2013-03-21 03:40:01]

Retried try job too often on win_rel for step(s) net_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-03-21 04:11:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/12760011/6001

[commit-bot: I haz the power, 2013-03-21 04:11:38]

Failed to apply patch for cc/trees/layer_tree_host.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/trees/layer_tree_host.cc
  Hunk #2 FAILED at 678.
  1 out of 3 hunks FAILED -- saving rejects to file
cc/trees/layer_tree_host.cc.rej

Patch:       cc/trees/layer_tree_host.cc
Index: cc/trees/layer_tree_host.cc
diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc
index
43e225a9917dde7df8308b464573c015e9604fb5..e19690a0cfd534659a5b88c2093fbe760739b824
100644
--- a/cc/trees/layer_tree_host.cc
+++ b/cc/trees/layer_tree_host.cc
@@ -288,7 +288,7 @@ void
LayerTreeHost::FinishCommitOnImplThread(LayerTreeHostImpl* host_impl) {
   needs_full_tree_sync_ = false;
 
   if (root_layer_ && hud_layer_) {
-    LayerImpl* hud_impl = LayerTreeHostCommon::findLayerInSubtree(
+    LayerImpl* hud_impl = LayerTreeHostCommon::FindLayerInSubtree(
         sync_tree->root_layer(), hud_layer_->id());
     sync_tree->set_hud_layer(static_cast<HeadsUpDisplayLayerImpl*>(hud_impl));
   } else {
@@ -678,14 +678,14 @@ void LayerTreeHost::UpdateLayers(Layer* root_layer,
     UpdateHudLayer();
 
     TRACE_EVENT0("cc", "LayerTreeHost::UpdateLayers::calcDrawProps");
-    LayerTreeHostCommon::calculateDrawProperties(
+    LayerTreeHostCommon::CalculateDrawProperties(
         root_layer,
         device_viewport_size(),
         device_scale_factor_,
         page_scale_factor_,
         GetRendererCapabilities().max_texture_size,
         settings_.canUseLCDText,
-        update_list);
+        &update_list);
   }
 
   // Reset partial texture update requests.
@@ -881,18 +881,18 @@ void LayerTreeHost::ApplyScrollAndScale(const
ScrollAndScaleSet& info) {
 
   for (size_t i = 0; i < info.scrolls.size(); ++i) {
     Layer* layer =
-        LayerTreeHostCommon::findLayerInSubtree(root_layer_.get(),
-                                                info.scrolls[i].layerId);
+        LayerTreeHostCommon::FindLayerInSubtree(root_layer_.get(),
+                                                info.scrolls[i].layer_id);
     if (!layer)
       continue;
     if (layer == root_scroll_layer)
-      root_scroll_delta += info.scrolls[i].scrollDelta;
+      root_scroll_delta += info.scrolls[i].scroll_delta;
     else
       layer->SetScrollOffset(layer->scroll_offset() +
-                             info.scrolls[i].scrollDelta);
+                             info.scrolls[i].scroll_delta);
   }
-  if (!root_scroll_delta.IsZero() || info.pageScaleDelta != 1.f)
-    client_->ApplyScrollAndScale(root_scroll_delta, info.pageScaleDelta);
+  if (!root_scroll_delta.IsZero() || info.page_scale_delta != 1.f)
+    client_->ApplyScrollAndScale(root_scroll_delta, info.page_scale_delta);
 }
 
 void LayerTreeHost::SetImplTransform(const gfx::Transform& transform) {

[commit-bot: I haz the power, 2013-03-21 17:14:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/12760011/26001

[commit-bot: I haz the power, 2013-03-21 17:15:03]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

HTTP Error 500: 
<html><head>
<meta http-equiv="content-type" content="text/html;charset=utf-8">
<title>500 Server Error</title>
</head>
<body text=#000000 bgcolor=#ffffff>
<h1>Error: Server Error</h1>
<h2>The server encountered an error and could not complete your request.<p>If
the problem persists, please <A
HREF="http://code.google.com/appengine/community.html">report</A> your problem
and mention this error message and the query that caused it.</h2>
<h2></h2>
</body></html>

[commit-bot: I haz the power, 2013-03-21 17:28:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/12760011/26001

[commit-bot: I haz the power, 2013-03-21 17:33:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/12760011/26001

[commit-bot: I haz the power, 2013-03-22 03:23:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/12760011/26001

[commit-bot: I haz the power, 2013-03-22 16:26:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/12760011/26001

[commit-bot: I haz the power, 2013-03-22 16:26:45]

Change committed as 189830