New issue
Advanced search Search tips

Issue 671440 link

Starred by 2 users
Status: Fixed
Owner:
danakj@chromium.org
Closed: Mar 2017
Cc:
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 671433



Sign in to add a comment

Make cc::DisplayItem non-virtual

Project Member Reported by danakj@chromium.org, Dec 6 2016 
To make it possible to memcpy a DisplayItemList, DisplayItems should not be virtual as these are not valid across processes.  To fix this, it would be better if cc::DisplayItem behaved like SkLiteDL.  Instead of treating each object in the list as a virtual object, it should instead have a type enum and use static casting to some final class.
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 7 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af5878e1056010e6d73329ddbd5a55917da87bb7

commit af5878e1056010e6d73329ddbd5a55917da87bb7
Author: danakj <danakj@chromium.org>
Date: Wed Dec 07 04:54:26 2016

cc: Devirtualize DisplayItem::ExternalMemoryUsage().

Make it a local member on each (non-ending) display item type, and
introduce a DisplayItem type field to tell which function to call.

R=vmpstr@chromium.org
BUG= 671440 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2561523002
Cr-Commit-Position: refs/heads/master@{#436865}

[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/clip_display_item.cc
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/clip_display_item.h
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/clip_path_display_item.cc
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/clip_path_display_item.h
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/compositing_display_item.cc
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/compositing_display_item.h
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/display_item.cc
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/display_item.h
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/display_item_list.cc
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/drawing_display_item.cc
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/drawing_display_item.h
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/filter_display_item.cc
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/filter_display_item.h
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/float_clip_display_item.cc
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/float_clip_display_item.h
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/transform_display_item.cc
[modify] https://crrev.com/af5878e1056010e6d73329ddbd5a55917da87bb7/cc/playback/transform_display_item.h
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 15 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a63104e9dda2b9a1e985505150b0dd370248c399

commit a63104e9dda2b9a1e985505150b0dd370248c399
Author: danakj <danakj@chromium.org>
Date: Wed Mar 15 00:12:35 2017

cc: Remove AsValue virtual from DisplayItem.

Have DisplayItemList iterate and turn each item into tracing output
directly. This makes accessors as needed on DisplayItems.

This removes base::trace_event from any public APIs on DisplayItem
and DisplayItemList.

R=enne@chromium.org
BUG= 671440 

Review-Url: https://codereview.chromium.org/2747273002
Cr-Commit-Position: refs/heads/master@{#456906}

[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/BUILD.gn
[delete] https://crrev.com/3fae4d36245600ce148e4c827271c34b38a85b9b/cc/debug/traced_display_item_list.cc
[delete] https://crrev.com/3fae4d36245600ce148e4c827271c34b38a85b9b/cc/debug/traced_display_item_list.h
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/clip_display_item.cc
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/clip_display_item.h
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/clip_path_display_item.cc
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/clip_path_display_item.h
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/compositing_display_item.cc
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/compositing_display_item.h
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/display_item.h
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/display_item_list.cc
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/display_item_list.h
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/display_item_list_unittest.cc
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/drawing_display_item.cc
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/drawing_display_item.h
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/filter_display_item.cc
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/filter_display_item.h
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/float_clip_display_item.cc
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/float_clip_display_item.h
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/raster_source.cc
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/transform_display_item.cc
[modify] https://crrev.com/a63104e9dda2b9a1e985505150b0dd370248c399/cc/playback/transform_display_item.h
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 16 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2

commit 0920d170ce0af366a0d1c386e63a2a2ab88a8dd2
Author: danakj <danakj@chromium.org>
Date: Thu Mar 16 15:16:56 2017

cc: Move DisplayItem::Raster up to DisplayItemList

This removes all cases of virtual from DisplayItem, making them
into just data types. Makes the fields public like DrawQuads, but
also const.

R=enne@chromium.org
BUG= 671440 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2751833007
Cr-Commit-Position: refs/heads/master@{#457439}

[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/clip_display_item.cc
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/clip_display_item.h
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/clip_path_display_item.cc
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/clip_path_display_item.h
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/compositing_display_item.cc
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/compositing_display_item.h
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/display_item.h
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/display_item_list.cc
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/display_item_list.h
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/drawing_display_item.cc
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/drawing_display_item.h
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/filter_display_item.cc
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/filter_display_item.h
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/float_clip_display_item.cc
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/float_clip_display_item.h
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/transform_display_item.cc
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/cc/playback/transform_display_item.h
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/ui/compositor/paint_cache.cc
[modify] https://crrev.com/0920d170ce0af366a0d1c386e63a2a2ab88a8dd2/ui/compositor/paint_cache.h

Comment 4 by danakj@chromium.org, Mar 16 2017

Owner: danakj@chromium.org
Status: Fixed (was: Available)
►

Sign in to add a comment