Use HashMap<..., OwnPtr<> > for Chromium compositor tilemap
Created attachment 118514 [details] Patch
Could we do the OwnPtr.h change separately? I'd like that bit to get a broader look from people who probably don't care much about chromium compositor internals.
(In reply to comment #2) > Could we do the OwnPtr.h change separately? I'd like that bit to get a broader look from people who probably don't care much about chromium compositor internals. Sure, no problem.
Created attachment 118520 [details] Patch
Anybody interested in reviewing this patch?
Comment on attachment 118520 [details] Patch Nice! R=me
Comment on attachment 118520 [details] Patch Clearing flags on attachment: 118520 Committed r102726: <http://trac.webkit.org/changeset/102726>
All reviewed patches have been landed. Closing bug.
It appears that this patch caused a Chromium Mac build failure: http://build.webkit.org/builders/Chromium%20Mac%20Release%20%28Tests%29/builds/10806
Error run through c++filt: Undefined symbols: "__ZN3WTF6OwnPtrIN7WebCore17CCLayerTilingData4TileEEC1ERKS4_", referenced from: __ZN3WTF9HashTableISt4pairIiiES1_IS2_NS_6OwnPtrIN7WebCore17CCLayerTilingData4TileEEEENS_18PairFirstExtractorIS8_EENS_8PairHashIiiEENS_14PairHashTraitsINS5_16TileMapKeyTraitsENS_10HashTraitsIS7_EEEESE_E6rehashEi in libwebcore_platform.a(CCLayerTilingData.o) __ZN3WTF9HashTableISt4pairIiiES1_IS2_NS_6OwnPtrIN7WebCore17CCLayerTilingData4TileEEEENS_18PairFirstExtractorIS8_EENS_8PairHashIiiEENS_14PairHashTraitsINS5_16TileMapKeyTraitsENS_10HashTraitsIS7_EEEESE_E3addINS_17HashMapTranslatorISH_SC_EES2_NS_10PassOwnPtrIS6_EEEES1_INS_17HashTableIteratorIS2_S8_SA_SC_SH_SE_EEbERKT0_RKT1_ in libwebcore_platform.a(CCLayerTilingData.o) ld: symbol(s) not found clang: error: linker command failed with exit code 1 (use -v to see invocation)Undefined symbols: "WTF::OwnPtr<WebCore::CCLayerTilingData::Tile>::OwnPtr(WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> const&)", referenced from: WTF::HashTable<std::pair<int, int>, std::pair<std::pair<int, int>, WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> >, WTF::PairFirstExtractor<std::pair<std::pair<int, int>, WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> > >, WTF::PairHash<int, int>, WTF::PairHashTraits<WebCore::CCLayerTilingData::TileMapKeyTraits, WTF::HashTraits<WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> > >, WebCore::CCLayerTilingData::TileMapKeyTraits>::rehash(int) in libwebcore_platform.a(CCLayerTilingData.o) std::pair<WTF::HashTableIterator<std::pair<int, int>, std::pair<std::pair<int, int>, WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> >, WTF::PairFirstExtractor<std::pair<std::pair<int, int>, WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> > >, WTF::PairHash<int, int>, WTF::PairHashTraits<WebCore::CCLayerTilingData::TileMapKeyTraits, WTF::HashTraits<WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> > >, WebCore::CCLayerTilingData::TileMapKeyTraits>, bool> WTF::HashTable<std::pair<int, int>, std::pair<std::pair<int, int>, WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> >, WTF::PairFirstExtractor<std::pair<std::pair<int, int>, WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> > >, WTF::PairHash<int, int>, WTF::PairHashTraits<WebCore::CCLayerTilingData::TileMapKeyTraits, WTF::HashTraits<WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> > >, WebCore::CCLayerTilingData::TileMapKeyTraits>::add<WTF::HashMapTranslator<WTF::PairHashTraits<WebCore::CCLayerTilingData::TileMapKeyTraits, WTF::HashTraits<WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> > >, WTF::PairHash<int, int> >, std::pair<int, int>, WTF::PassOwnPtr<WebCore::CCLayerTilingData::Tile> >(std::pair<int, int> const&, WTF::PassOwnPtr<WebCore::CCLayerTilingData::Tile> const&) in libwebcore_platform.a(CCLayerTilingData.o) ld: symbol(s) not found I'm not sure what that means.
Undefined symbols: "__ZN3WTF6OwnPtrIN7WebCore17CCLayerTilingData4TileEEC1ERKS4_", referenced from: __ZNSt4pairIS_IiiEN3WTF6OwnPtrIN7WebCore17CCLayerTilingData4TileEEEEC2ERKS7_ in libwebcore_platform.a(CCLayerTilingData.o) ld: symbol(s) not found clang: error: linker command failed with exit code 1 (use -v to see invocation) and rniwa-macpro:webkit3 rniwa$ c++filt __ZN3WTF6OwnPtrIN7WebCore17CCLayerTilingData4TileEEC1ERKS4_ WTF::OwnPtr<WebCore::CCLayerTilingData::Tile>::OwnPtr(WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> const&) rniwa-macpro:webkit3 rniwa$ c++filt __ZNSt4pairIS_IiiEN3WTF6OwnPtrIN7WebCore17CCLayerTilingData4TileEEEEC2ERKS7_ std::pair<std::pair<int, int>, WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> >::pair(std::pair<std::pair<int, int>, WTF::OwnPtr<WebCore::CCLayerTilingData::Tile> > const&) So... something is screwed up here. We shouldn't be calling the copy constructor of OwnPtr. I haven't been following HashMap's OwnPtr support enough to tell what we need to do here.
Going to roll this patch out for now, the fix does not appear obvious.
Reverted r102726 for reason: Does not compile on clang Committed r102738: <http://trac.webkit.org/changeset/102738>
(In reply to comment #13) > Reverted r102726 for reason: > > Does not compile on clang > > Committed r102738: <http://trac.webkit.org/changeset/102738> rniwa: Thanks for rolling this out. Sorry for the hassle! thakis: This looks a lot like the issue in https://bugs.webkit.org/show_bug.cgi?id=73711#c24, where Clang takes umbrage with the declared-but-not-defined OwnPtr copy constructor that gets optimized out elsewhere.
jamesr said he'd try removing that weird declared-for-an-ancient-gcc-but-not-defined-anywhere constructor. Has that happend?
No, and I don't have a particular timetable for trying that.
enne: Then I'd try doing that first.
Created attachment 125977 [details] Rebased to ToT
Comment on attachment 125977 [details] Rebased to ToT Attachment 125977 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11453218
(In reply to comment #19) > (From update of attachment 125977 [details]) > Attachment 125977 [details] did not pass gtk-ews (gtk): > Output: http://queues.webkit.org/results/11453218 Whoops, didn't mean to include the patch from bug 78071 as well.
Comment on attachment 125977 [details] Rebased to ToT Attachment 125977 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11449243
Comment on attachment 125977 [details] Rebased to ToT Attachment 125977 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11449249
Created attachment 126555 [details] Rebased to ToT
Comment on attachment 126555 [details] Rebased to ToT View in context: https://bugs.webkit.org/attachment.cgi?id=126555&action=review R=me > Source/WebCore/platform/graphics/chromium/cc/CCTiledLayerImpl.cpp:50 > + static PassOwnPtr<DrawableTile> create() { return adoptPtr(new DrawableTile()); } nit: i normally omit the () from the c'tor call here. not a strong preference
Committed in r107645.