Both CoordinatedTileClient and TiledBackingStore::Client are implemented by CoordinatedGraphicsLayer. So, I suggest to simplify them by removing CoordinatedTileClient.
Created attachment 232168 [details] Patch
Your patch replaces CoordinatedTileClient with CoordinatedTiledBackingStoreClient inherited from TiledBackingStoreClient. But we don't have a CoordinatedTiledBackingStore class. I'm not sure this is a proper approach.
(In reply to comment #2) > Your patch replaces CoordinatedTileClient with CoordinatedTiledBackingStoreClient inherited from TiledBackingStoreClient. > But we don't have a CoordinatedTiledBackingStore class. > I'm not sure this is a proper approach. Right. I can't find similar cases in WebKit. Although there are some examples which extends Client (TrackPrivateBaseClient and InbandTextTrackPrivateClient), they also have instances. I can make CoordinatedTiledBackingStore, but I think that it looks bit weird. In my understanding, CoordinatedGraphicsLayer uses the extension of TiledBackingStore system, especially Tile and callbacks of TiledBackingStore. By the way, TiledBackingStore is now only used for Coordinated Graphics system. So, another suggestion is merging these extension into TiledBackingStore.
(In reply to comment #3) > (In reply to comment #2) > > Your patch replaces CoordinatedTileClient with CoordinatedTiledBackingStoreClient inherited from TiledBackingStoreClient. > > But we don't have a CoordinatedTiledBackingStore class. > > I'm not sure this is a proper approach. > > Right. > I can't find similar cases in WebKit. > Although there are some examples which extends Client (TrackPrivateBaseClient and InbandTextTrackPrivateClient), they also have instances. > > I can make CoordinatedTiledBackingStore, but I think that it looks bit weird. > > In my understanding, CoordinatedGraphicsLayer uses the extension of TiledBackingStore system, especially Tile and callbacks of TiledBackingStore. > > > By the way, TiledBackingStore is now only used for Coordinated Graphics system. > > So, another suggestion is merging these extension into TiledBackingStore. Although I removed at Bug 133341, DocumentThreadableLoaderClient inherit ThreadableLoaderClient like CoordinatedTiledBackingStoreClient. (It was dead code since chromium port is out) Another suggestion, which merges CoordinatedTiledBackingStoreClient to TiledBackingStoreClient, looks not simple because Atlas and CoordinatedSurface and CoordinatedTile::Client are tightly coupled. In my analyzing, it requires the changes of callback structure so I want to do it in another bug.
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > Your patch replaces CoordinatedTileClient with CoordinatedTiledBackingStoreClient inherited from TiledBackingStoreClient. > > > But we don't have a CoordinatedTiledBackingStore class. > > > I'm not sure this is a proper approach. > > > > Right. > > I can't find similar cases in WebKit. > > Although there are some examples which extends Client (TrackPrivateBaseClient and InbandTextTrackPrivateClient), they also have instances. > > > > I can make CoordinatedTiledBackingStore, but I think that it looks bit weird. > > > > In my understanding, CoordinatedGraphicsLayer uses the extension of TiledBackingStore system, especially Tile and callbacks of TiledBackingStore. > > > > > > By the way, TiledBackingStore is now only used for Coordinated Graphics system. > > > > So, another suggestion is merging these extension into TiledBackingStore. > > Although I removed at Bug 133341, DocumentThreadableLoaderClient inherit ThreadableLoaderClient like CoordinatedTiledBackingStoreClient. > (It was dead code since chromium port is out) > > Another suggestion, which merges CoordinatedTiledBackingStoreClient to TiledBackingStoreClient, looks not simple because Atlas and CoordinatedSurface and CoordinatedTile::Client are tightly coupled. > > In my analyzing, it requires the changes of callback structure so I want to do it in another bug. To extend TiledBackingStoreClient looks better than to add CoordinatedTiledBackingStoreClient. And I think the rename of interfaces in CoordinatedTileClient is also needed when merging them into TiledBackingStoreClient.
yoon, could you take a loot at this?
Ryuan, is this patch still valid for EFL port ?
Created attachment 247389 [details] Patch
(In reply to comment #7) > Ryuan, is this patch still valid for EFL port ? Sure, this is just a kind of clean up. I rebased. I think that TiledBackingStore is now just a part of Coordinated Graphics. So, I think that we can simplify them more.
Created attachment 253704 [details] Patch
Comment on attachment 253704 [details] Patch Clearing flags on attachment: 253704 Committed r184876: <http://trac.webkit.org/changeset/184876>
All reviewed patches have been landed. Closing bug.