Remove duplicate HashMap traversal and unneeded reference count churn in DataObjectGtk::forClipboard.
Created attachment 274088 [details] Patch
Created attachment 274090 [details] Patch
Comment on attachment 274090 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=274090&action=review > Source/WebCore/platform/gtk/DataObjectGtk.cpp:163 > + auto it = objectMap.find(clipboard); > > - if (!objectMap.contains(clipboard)) { > - Ref<DataObjectGtk> dataObject = DataObjectGtk::create(); > - objectMap.set(clipboard, dataObject.copyRef()); > - return dataObject.ptr(); > + if (it == objectMap.end()) { > + auto dataObject = DataObjectGtk::create(); > + DataObjectGtk* dataObjectPtr = dataObject.ptr(); > + objectMap.set(clipboard, WTFMove(dataObject)); > + return dataObjectPtr; > } > > - HashMap<GtkClipboard*, RefPtr<DataObjectGtk> >::iterator it = objectMap.find(clipboard); > return it->value.get(); > } Or even better you could use HashMap::add and check the result. Something like this: auto addResult = objectMap.add(clipboard, nullptr); if (addResult.isNewEntry) addResult.iterator->value = DataObjectGtk::create(); return addResult.iterator->value.get();
(In reply to comment #3) > Comment on attachment 274090 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=274090&action=review > > > Source/WebCore/platform/gtk/DataObjectGtk.cpp:163 > > + auto it = objectMap.find(clipboard); > > > > - if (!objectMap.contains(clipboard)) { > > - Ref<DataObjectGtk> dataObject = DataObjectGtk::create(); > > - objectMap.set(clipboard, dataObject.copyRef()); > > - return dataObject.ptr(); > > + if (it == objectMap.end()) { > > + auto dataObject = DataObjectGtk::create(); > > + DataObjectGtk* dataObjectPtr = dataObject.ptr(); > > + objectMap.set(clipboard, WTFMove(dataObject)); > > + return dataObjectPtr; > > } > > > > - HashMap<GtkClipboard*, RefPtr<DataObjectGtk> >::iterator it = objectMap.find(clipboard); > > return it->value.get(); > > } > > Or even better you could use HashMap::add and check the result. Something > like this: > > auto addResult = objectMap.add(clipboard, nullptr); > if (addResult.isNewEntry) > addResult.iterator->value = DataObjectGtk::create(); > return addResult.iterator->value.get(); Indeed it is. Thank you for your comment. I will change this as you wrote.
Created attachment 274091 [details] Patch
Comment on attachment 274091 [details] Patch Clearing flags on attachment: 274091 Committed r198209: <http://trac.webkit.org/changeset/198209>
All reviewed patches have been landed. Closing bug.