Use OwnPtr for FillLayer::m_next
Created attachment 120525 [details] Patch
Comment on attachment 120525 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120525&action=review r- because of a logic mistake I spotted in imagesAreLoaded(). > Source/WebCore/rendering/style/FillLayer.cpp:80 > FillLayer::~FillLayer() > { > - delete m_next; > } Wouldn’t the compiler generate this destructor if you omitted it (and didn’t declare it)? > Source/WebCore/rendering/style/FillLayer.cpp:255 > - return false; > + for (const FillLayer* layer = this; layer; layer = layer->m_next.get()) { > + if (layer->m_image && !layer->m_image->isLoaded()) > + return true; Should return false here.
Comment on attachment 120525 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120525&action=review >> Source/WebCore/rendering/style/FillLayer.cpp:80 >> } > > Wouldn’t the compiler generate this destructor if you omitted it (and didn’t declare it)? It would, but further, it would probably be inlined each place a FillLayer was destroyed. But I’ll do as you imply I should. >> Source/WebCore/rendering/style/FillLayer.cpp:255 >> + return true; > > Should return false here. Oops!
Created attachment 120529 [details] Patch
Comment on attachment 120529 [details] Patch Rejecting attachment 120529 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ss/CSSStyleApplyProperty.cpp.rej patching file Source/WebCore/rendering/style/FillLayer.cpp Hunk #2 succeeded at 40 with fuzz 1 (offset 15 lines). Hunk #3 succeeded at 65 (offset 15 lines). Hunk #4 succeeded at 90 (offset 15 lines). Hunk #5 succeeded at 239 (offset 15 lines). patching file Source/WebCore/rendering/style/FillLayer.h Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Dan Bernst..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/12798232
Guess I’ll have to rebase this myself. Not in front of a computer with a source tree at the moment.
This hasn't been landed yet.
Created attachment 229219 [details] Patch
Attachment 229219 [details] did not pass style-queue: ERROR: Source/WebCore/rendering/style/FillLayer.cpp:76: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r167208: <http://trac.webkit.org/changeset/167208>