Tile is only used in TiledBackingStore and we don't need to use RefPtr for it since r185140
Created attachment 254550 [details] Patch
Comment on attachment 254550 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254550&action=review > Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp:251 > + m_tiles.set(coordinate, std::make_unique<Tile>(*this, coordinate)); Should probably be add, not set. > Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp:366 > unsigned removeCount = tilesToRemove.size(); > for (unsigned n = 0; n < removeCount; ++n) Should use modern for loop. > Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp:385 > unsigned removeCount = toRemove.size(); > for (unsigned n = 0; n < removeCount; ++n) Should use modern for loop. > Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.h:91 > + typedef HashMap<Tile::Coordinate, std::unique_ptr<Tile>> TileMap; Not even sure it’s helpful to allocate these on the heap. Maybe just Tile here instead of std::unique_ptr<Tile> some day.
Committed r185388: <http://trac.webkit.org/changeset/185388>
Comment on attachment 254550 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254550&action=review Thanks, I landed after followed your comments except Tile allocation. clearing flags. I tried but it was not available. >> Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp:251 >> + m_tiles.set(coordinate, std::make_unique<Tile>(*this, coordinate)); > > Should probably be add, not set. Thanks. I fixed. >> Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp:366 >> for (unsigned n = 0; n < removeCount; ++n) > > Should use modern for loop. Done. >> Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp:385 >> for (unsigned n = 0; n < removeCount; ++n) > > Should use modern for loop. Done. >> Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.h:91 >> + typedef HashMap<Tile::Coordinate, std::unique_ptr<Tile>> TileMap; > > Not even sure it’s helpful to allocate these on the heap. Maybe just Tile here instead of std::unique_ptr<Tile> some day. Maybe, but now Tile is not simple stucture to keep the HashMap as instance. Anyway, I will consider it in different bug not to allocate.