Remove live ranges from Document.h, AlternativeTextController.h, DictionaryLookup.h, and WebPage.h
Created attachment 403825 [details] Patch
Created attachment 403986 [details] Patch
OK, looks like this new patch is on a patch to be green on all of EWS.
Comment on attachment 403986 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403986&action=review > Source/WebCore/dom/Document.h:1587 > enum ConstructionFlags { Synthesized = 1, NonRenderedPlaceholder = 1 << 1 }; > - Document(Frame*, const URL&, unsigned = DefaultDocumentClass, unsigned constructionFlags = 0); > + Document(Frame*, const URL&, DocumentClassFlags = DefaultDocumentClass, unsigned constructionFlags = 0); Probably want to convert both DocumentClassFlags and ConstructionFlags to use OptionSet at some point. > Source/WebCore/editing/AlternativeTextController.cpp:593 > // Clone the range, since the caller of may keep a refernece to the original range and modify it. > - SpellingCorrectionCommand::create(range.cloneRange(), alternative)->apply(); > + SpellingCorrectionCommand::create(createLiveRange(range), alternative)->apply(); Comment is not quite right anymore. > Source/WebCore/editing/mac/DictionaryLookup.h:60 > - WEBCORE_EXPORT static std::tuple<RefPtr<Range>, NSDictionary *> rangeForSelection(const VisibleSelection&); > - WEBCORE_EXPORT static std::tuple<RefPtr<Range>, NSDictionary *> rangeAtHitTestResult(const HitTestResult&); > + WEBCORE_EXPORT static Optional<std::tuple<SimpleRange, NSDictionary *>> rangeForSelection(const VisibleSelection&); > + WEBCORE_EXPORT static Optional<std::tuple<SimpleRange, NSDictionary *>> rangeAtHitTestResult(const HitTestResult&); > WEBCORE_EXPORT static std::tuple<NSString *, NSDictionary *> stringForPDFSelection(PDFSelection *); I think these would be easier to understand if they returned used a struct for the return value (or Optional<NewStruct> as the case may be) rather than tuple. Not new, but this is the type of std::pair/std::tuple usage I think we should avoid, where you can't tell what the values are for straight from the function name. (/me is scared to blame the file and find out I added the std::tuple use).
Comment on attachment 403986 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403986&action=review >> Source/WebCore/dom/Document.h:1587 >> + Document(Frame*, const URL&, DocumentClassFlags = DefaultDocumentClass, unsigned constructionFlags = 0); > > Probably want to convert both DocumentClassFlags and ConstructionFlags to use OptionSet at some point. Yes! >> Source/WebCore/editing/AlternativeTextController.cpp:593 >> + SpellingCorrectionCommand::create(createLiveRange(range), alternative)->apply(); > > Comment is not quite right anymore. Will fix. >> Source/WebCore/editing/mac/DictionaryLookup.h:60 >> WEBCORE_EXPORT static std::tuple<NSString *, NSDictionary *> stringForPDFSelection(PDFSelection *); > > I think these would be easier to understand if they returned used a struct for the return value (or Optional<NewStruct> as the case may be) rather than tuple. Not new, but this is the type of std::pair/std::tuple usage I think we should avoid, where you can't tell what the values are for straight from the function name. (/me is scared to blame the file and find out I added the std::tuple use). OK, I’ll consider doing that to improve this in the future. I agree that a struct would be nicer. Don’t want to do it right now.
Committed r264247: <https://trac.webkit.org/changeset/264247>
<rdar://problem/65372579>
Follow up fix for Catalyst in https://trac.webkit.org/changeset/264317/webkit