[WIN] Make WebHistory compile without USE(CF)
Created attachment 212799 [details] Patch
Created attachment 212800 [details] Patch
Attachment 212800 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/win/COMVariantSetter.h', u'Source/WebKit/win/ChangeLog', u'Source/WebKit/win/WebHistory.cpp', u'Source/WebKit/win/WebHistory.h']" exit_code: 1 Source/WebKit/win/COMVariantSetter.h:125: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 212800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=212800&action=review > Source/WebKit/win/WebHistory.cpp:88 > -static COMPtr<CFDictionaryPropertyBag> createUserInfoFromArray(BSTR notificationStr, CFArrayRef arrayItem) > +static COMPtr<IPropertyBag> createUserInfoFromArray(BSTR notificationStr, IWebHistoryItem** data, size_t size) > { > +#if USE(CF) > + RetainPtr<CFArrayRef> arrayItem = adoptCF(CFArrayCreate(kCFAllocatorDefault, (const void**)data, size, &MarshallingHelpers::kIUnknownArrayCallBacks)); > + > RetainPtr<CFMutableDictionaryRef> dictionary = adoptCF( > CFDictionaryCreateMutable(0, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks)); > > RetainPtr<CFStringRef> key = adoptCF(MarshallingHelpers::BSTRToCFStringRef(notificationStr)); > - CFDictionaryAddValue(dictionary.get(), key.get(), arrayItem); > + CFDictionaryAddValue(dictionary.get(), key.get(), arrayItem.get()); > > COMPtr<CFDictionaryPropertyBag> result = CFDictionaryPropertyBag::createInstance(); > result->setDictionary(dictionary.get()); > - return result; > + return COMPtr<IPropertyBag>(AdoptCOM, result.leakRef()); > +#else > + Vector<COMPtr<IWebHistoryItem>, 1> arrayItem; > + arrayItem.reserveInitialCapacity(size); > + for (size_t i = 0; i < size; ++i) > + arrayItem[i] = data[i]; > + > + HashMap<String, Vector<COMPtr<IWebHistoryItem>>> dictionary; > + String key(notificationStr, ::SysStringLen(notificationStr)); > + dictionary.set(key, std::move(arrayItem)); > + return COMPtr<IPropertyBag>(AdoptCOM, COMPropertyBag<Vector<COMPtr<IWebHistoryItem>>>::adopt(dictionary)); > +#endif > } Does not make sense to have both of these. Why keep the CF version?
(In reply to comment #4) > Does not make sense to have both of these. Why keep the CF version? Maybe someone wants to query the CFDictionaryPropertyBag interface. I don't know what the Safari (or other windows application) code does to access the values.
Comment on attachment 212800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=212800&action=review Looks good to me. Please correct the bracket placement and we can land this. >> Source/WebKit/win/COMVariantSetter.h:125 >> +{ > > This { should be at the end of the previous line [whitespace/braces] [4] Please correct this bracket position. >> Source/WebKit/win/WebHistory.cpp:88 >> } > > Does not make sense to have both of these. Why keep the CF version? We need the CF versions to maintain compatibility with other software.
Created attachment 215658 [details] Patch
Comment on attachment 215658 [details] Patch r=me
Comment on attachment 215658 [details] Patch Clearing flags on attachment: 215658 Committed r158370: <http://trac.webkit.org/changeset/158370>
All reviewed patches have been landed. Closing bug.