Bug 136654

Summary: Clean up some ENABLE(PICTURE_SIZES) stuff, don't change function signatures inside #ifdefs
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, gyuyoung.kim, koivisto, simon.fraser, yoav
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 136942    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Simon Fraser (smfr)
Reported 2014-09-08 18:27:04 PDT
Clean up some ENABLE(PICTURE_SIZES) stuff, don't change function signatures inside #ifdefs
Attachments
Patch (11.49 KB, patch)
2014-09-08 18:28 PDT, Simon Fraser (smfr)
no flags
Patch (12.92 KB, patch)
2014-09-09 15:13 PDT, Simon Fraser (smfr)
no flags
Simon Fraser (smfr)
Comment 1 2014-09-08 18:28:36 PDT
Darin Adler
Comment 2 2014-09-08 19:41:51 PDT
Comment on attachment 237828 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237828&action=review > Source/WebCore/html/parser/HTMLDocumentParser.cpp:329 > + m_preloadScanner->scan(m_preloader.get(), document()->baseElementURL(), document()->renderView(), document()->frame()); Seems bizarre to pass both a RenderView* and a Frame*. Why not just pass a Document& instead? It seems to me that SourceSizeList::parseSizesAttribute should also just take a Document&. And the same for every level in between.
Simon Fraser (smfr)
Comment 3 2014-09-08 23:01:21 PDT
Yes, I thought the same thing. I'll do that.
Simon Fraser (smfr)
Comment 4 2014-09-09 15:13:34 PDT
Darin Adler
Comment 5 2014-09-10 08:28:40 PDT
Comment on attachment 237862 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237862&action=review > Source/WebCore/html/parser/HTMLPreloadScanner.cpp:102 > for (HTMLToken::AttributeList::const_iterator iter = attributes.begin(); iter != attributes.end(); ++iter) { Opportunity for new-style for loop. > Source/WebCore/html/parser/HTMLPreloadScanner.cpp:112 > + sourceSize = SourceSizeList::parseSizesAttribute(m_sizesAttribute, document.renderView(), document.frame()); I suggest pushing down Document& even further, and changing the interface to SourceSizeList to take Document&. > Source/WebCore/html/parser/HTMLPreloadScanner.h:50 > + void scan(const HTMLToken&, PreloadRequestStream& requests, Document&); I don’t think the argument name “requests” adds anything here. > Source/WebCore/html/parser/HTMLPreloadScanner.h:130 > + void scan(HTMLResourcePreloader*, Document&); Should be HTMLResourcePreloader&.
WebKit Commit Bot
Comment 6 2014-09-10 08:54:47 PDT
Comment on attachment 237862 [details] Patch Clearing flags on attachment: 237862 Committed r173464: <http://trac.webkit.org/changeset/173464>
WebKit Commit Bot
Comment 7 2014-09-10 08:54:51 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.