cc: Change ClipNode::applies_local_clip to a clip_type enum (issue 2437923002 by ajuma@chromium.org)

aj...@chromium.org

unread,

Oct 21, 2016, 6:48:25 PM10/21/16

to weil...@chromium.org, jayd...@chromium.org, chromium...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, dongseo...@intel.com, pdr+graphi...@chromium.org, jbr...@chromium.org, ju...@chromium.org, caba...@adobe.com, fma...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, ajuma...@chromium.org, danakj...@chromium.org, rob....@samsung.com

Reviewers: weiliangc, jaydasika
CL: https://codereview.chromium.org/2437923002/

Description:
cc: Change ClipNode::applies_local_clip to a clip_type enum

This change is in preparation for adding "expanding" clip nodes,
which will involve adding a new ClipType.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
BUG=653631

Affected files (+79, -32 lines):
M cc/proto/cc_conversions.h
M cc/proto/cc_conversions.cc
M cc/proto/property_tree.proto
M cc/trees/clip_node.h
M cc/trees/clip_node.cc
M cc/trees/draw_property_utils.cc
M cc/trees/layer_tree_host_common_unittest.cc
M cc/trees/layer_tree_impl.cc
M cc/trees/property_tree_builder.cc
M cc/trees/property_tree_unittest.cc
M third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
M third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp

jayd...@chromium.org

unread,

Oct 21, 2016, 7:20:59 PM10/21/16

to ajuma...@chromium.org, weil...@chromium.org, chromium...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, dongseo...@intel.com, pdr+graphi...@chromium.org, jbr...@chromium.org, ju...@chromium.org, caba...@adobe.com, fma...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, ajuma...@chromium.org, danakj...@chromium.org, rob....@samsung.com

lgtm

https://codereview.chromium.org/2437923002/diff/1/cc/trees/property_tree_builder.cc
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/2437923002/diff/1/cc/trees/property_tree_builder.cc#newcode372
cc/trees/property_tree_builder.cc:372: DCHECK(parent->clip_type !=
ClipNode::ClipType::APPLIES_LOCAL_CLIP);
Nit : You can use DCHECK_NE here.

https://codereview.chromium.org/2437923002/

aj...@chromium.org

unread,

Oct 21, 2016, 7:35:30 PM10/21/16

to weil...@chromium.org, jayd...@chromium.org, p...@chromium.org, chromium...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, dongseo...@intel.com, pdr+graphi...@chromium.org, jbr...@chromium.org, ju...@chromium.org, caba...@adobe.com, fma...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, ajuma...@chromium.org, danakj...@chromium.org, rob....@samsung.com

+pdr for Source/platform/graphics

https://codereview.chromium.org/2437923002/diff/1/cc/trees/property_tree_builder.cc
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/2437923002/diff/1/cc/trees/property_tree_builder.cc#newcode372
cc/trees/property_tree_builder.cc:372: DCHECK(parent->clip_type !=
ClipNode::ClipType::APPLIES_LOCAL_CLIP);

On 2016/10/21 23:20:58, jaydasika wrote:
> Nit : You can use DCHECK_NE here.

Done.

https://codereview.chromium.org/2437923002/

p...@chromium.org

unread,

Oct 21, 2016, 7:38:53 PM10/21/16

to ajuma...@chromium.org, weil...@chromium.org, jayd...@chromium.org, chromium...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, dongseo...@intel.com, pdr+graphi...@chromium.org, jbr...@chromium.org, ju...@chromium.org, caba...@adobe.com, fma...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, ajuma...@chromium.org, danakj...@chromium.org, rob....@samsung.com

LGTM

https://codereview.chromium.org/2437923002/

commit-bot@chromium.org via chromiumcodereview-hr.appspot.com

unread,

Oct 21, 2016, 7:50:52 PM10/21/16

to ajuma...@chromium.org, weil...@chromium.org, jayd...@chromium.org, p...@chromium.org, commi...@chromium.org, chromium...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, dongseo...@intel.com, pdr+graphi...@chromium.org, jbr...@chromium.org, ju...@chromium.org, caba...@adobe.com, fma...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, ajuma...@chromium.org, danakj...@chromium.org, rob....@samsung.com

CQ is trying da patch. Follow status at

https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2437923002/40001

https://codereview.chromium.org/2437923002/

commit-bot@chromium.org via chromiumcodereview-hr.appspot.com

unread,

Oct 21, 2016, 11:38:03 PM10/21/16

to ajuma...@chromium.org, weil...@chromium.org, jayd...@chromium.org, p...@chromium.org, commi...@chromium.org, chromium...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, dongseo...@intel.com, pdr+graphi...@chromium.org, jbr...@chromium.org, ju...@chromium.org, caba...@adobe.com, fma...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, ajuma...@chromium.org, danakj...@chromium.org, rob....@samsung.com

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_rel_ng/builds/321819)

https://codereview.chromium.org/2437923002/

commit-bot@chromium.org via chromiumcodereview-hr.appspot.com

unread,

Oct 24, 2016, 9:04:01 AM10/24/16

to ajuma...@chromium.org, weil...@chromium.org, jayd...@chromium.org, p...@chromium.org, commi...@chromium.org, chromium...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, dongseo...@intel.com, pdr+graphi...@chromium.org, jbr...@chromium.org, ju...@chromium.org, caba...@adobe.com, fma...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, ajuma...@chromium.org, danakj...@chromium.org, rob....@samsung.com

commit-bot@chromium.org via chromiumcodereview-hr.appspot.com

unread,

Oct 24, 2016, 11:03:44 AM10/24/16

to ajuma...@chromium.org, weil...@chromium.org, jayd...@chromium.org, p...@chromium.org, commi...@chromium.org, chromium...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, dongseo...@intel.com, pdr+graphi...@chromium.org, jbr...@chromium.org, ju...@chromium.org, caba...@adobe.com, fma...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, ajuma...@chromium.org, danakj...@chromium.org, rob....@samsung.com

Try jobs failed on following builders:

linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/301329)

https://codereview.chromium.org/2437923002/

commit-bot@chromium.org via chromiumcodereview-hr.appspot.com

unread,

Oct 24, 2016, 11:35:55 AM10/24/16

to ajuma...@chromium.org, weil...@chromium.org, jayd...@chromium.org, p...@chromium.org, commi...@chromium.org, chromium...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, dongseo...@intel.com, pdr+graphi...@chromium.org, jbr...@chromium.org, ju...@chromium.org, caba...@adobe.com, fma...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, ajuma...@chromium.org, danakj...@chromium.org, rob....@samsung.com

commit-bot@chromium.org via chromiumcodereview-hr.appspot.com

unread,

Oct 24, 2016, 12:26:08 PM10/24/16

to ajuma...@chromium.org, weil...@chromium.org, jayd...@chromium.org, p...@chromium.org, commi...@chromium.org, chromium...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, dongseo...@intel.com, pdr+graphi...@chromium.org, jbr...@chromium.org, ju...@chromium.org, caba...@adobe.com, fma...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, ajuma...@chromium.org, danakj...@chromium.org, rob....@samsung.com

Committed patchset #3 (id:40001)

https://codereview.chromium.org/2437923002/

commit-bot@chromium.org via chromiumcodereview-hr.appspot.com

unread,

Oct 24, 2016, 12:49:59 PM10/24/16

to ajuma...@chromium.org, weil...@chromium.org, jayd...@chromium.org, p...@chromium.org, commi...@chromium.org, chromium...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, dongseo...@intel.com, pdr+graphi...@chromium.org, jbr...@chromium.org, ju...@chromium.org, caba...@adobe.com, fma...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, ajuma...@chromium.org, danakj...@chromium.org, rob....@samsung.com

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

https://codereview.chromium.org/2437923002/

Reply all

Reply to author

Forward