Otherwise, a lot of logging in WebCore needs to go through the DOMWindow which is unnecessary.
Created attachment 191760 [details] Patch
Comment on attachment 191760 [details] Patch Attachment 191760 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17045262
Comment on attachment 191760 [details] Patch Attachment 191760 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17086053
Comment on attachment 191760 [details] Patch Attachment 191760 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/16991311
Comment on attachment 191760 [details] Patch Attachment 191760 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17082223
Comment on attachment 191760 [details] Patch Attachment 191760 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17073145 New failing tests: fast/xsl/xslt-mismatched-tags-in-xslt.xml fast/xsl/xslt-missing-namespace-in-xslt.xml
Created attachment 191786 [details] Patch
Comment on attachment 191786 [details] Patch Attachment 191786 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/16991356
Created attachment 193152 [details] Patch
Comment on attachment 193152 [details] Patch Attachment 193152 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17189511
Comment on attachment 193152 [details] Patch Attachment 193152 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17129205
Comment on attachment 193152 [details] Patch Attachment 193152 [details] did not pass win-ews (win): Output: http://webkit-commit-queue.appspot.com/results/17041605
Comment on attachment 193152 [details] Patch Attachment 193152 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/16996290 New failing tests: fast/xsl/xslt-mismatched-tags-in-xslt.xml fast/xsl/xslt-missing-namespace-in-xslt.xml
Comment on attachment 193152 [details] Patch Attachment 193152 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17126439
Comment on attachment 193152 [details] Patch Attachment 193152 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17186187
Comment on attachment 193152 [details] Patch Attachment 193152 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/16997832
Comment on attachment 193152 [details] Patch Attachment 193152 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17176167 New failing tests: fast/xsl/xslt-mismatched-tags-in-xslt.xml fast/xsl/xslt-missing-namespace-in-xslt.xml
Created attachment 193259 [details] Patch
Comment on attachment 193259 [details] Patch Attachment 193259 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17215140
Comment on attachment 193259 [details] Patch Attachment 193259 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17116304
Comment on attachment 193259 [details] Patch Attachment 193259 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17210250
Comment on attachment 193259 [details] Patch Attachment 193259 [details] did not pass win-ews (win): Output: http://webkit-commit-queue.appspot.com/results/17055662
Created attachment 193261 [details] Patch
Comment on attachment 193261 [details] Patch Attachment 193261 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17060887 New failing tests: fast/xsl/xslt-mismatched-tags-in-xslt.xml fast/xsl/xslt-missing-namespace-in-xslt.xml
Created attachment 193275 [details] Patch
Comment on attachment 193275 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193275&action=review > Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp:82 > + if (console) Nit: I believe the accepted WebKit style here would be to inline the assignment. That is `if (PageConsole* console = m_globalObject->impl()->pageConsole())`. > Source/WebCore/dom/Document.cpp:4819 > + if (p) Nit: Inline the assignment, and prefer complete words for variable names: 'page' rather than 'p'. > Source/WebCore/dom/Document.cpp:4831 > + if (p) Nit: Same as above. > Source/WebCore/page/DOMWindow.h:236 > Console* console() const; Do we still need access to the Console from DOMWindow, or will PageConsole fill the external requirements? It would be nice if external callers only had one option; otherwise they'll likely make the wrong choice. If possible, I'd suggest removing DOMWindow::console, or making it private.
Comment on attachment 193275 [details] Patch Attachment 193275 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17060918
Comment on attachment 193275 [details] Patch Attachment 193275 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17145279 New failing tests: fast/xsl/xslt-mismatched-tags-in-xslt.xml fast/xsl/xslt-missing-namespace-in-xslt.xml
Created attachment 193285 [details] Patch
Comment on attachment 193285 [details] Patch Attachment 193285 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17207241 New failing tests: fast/xsl/xslt-mismatched-tags-in-xslt.xml fast/xsl/xslt-missing-namespace-in-xslt.xml
Comment on attachment 193285 [details] Patch Attachment 193285 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17187261
Comment on attachment 193285 [details] Patch Attachment 193285 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17165234
Comment on attachment 193285 [details] Patch Attachment 193285 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17156719
Created attachment 193432 [details] Patch
Created attachment 193448 [details] Patch
Comment on attachment 193448 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193448&action=review > Source/WebCore/ChangeLog:10 > + No new tests (OOPS!). Please remove this line, otherwise commit queue won't let you commit the patch. > Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp:-80 > - // FIXME: Pass actual line number and source URL. Please keep FIXME here. > Source/WebCore/dom/Document.cpp:127 > +#include "PageConsole.h" Do we still need to include Console.h here? > Source/WebCore/dom/Document.cpp:4848 > + Page* p = page(); Page* page = ... > Source/WebCore/dom/Document.cpp:4861 > + if (p) Ditto > Source/WebCore/page/Console.h:81 > static void mute(); Should be removed. > Source/WebCore/page/Console.h:82 > static void unmute(); Ditto > Source/WebCore/page/Console.h:-99 > - inline Page* page() const; Why did you remove this? > Source/WebCore/xml/XSLStyleSheetLibxslt.cpp:27 > #include "DOMWindow.h" Could be removed now. > Source/WebCore/xml/XSLTProcessorLibxslt.cpp:30 > #include "DOMWindow.h" Could be removed now.
Created attachment 193743 [details] Patch
Comment on attachment 193743 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193743&action=review > Source/WebCore/WebCore.vcxproj/WebCore.vcxproj:13641 > \ No newline at end of file I am not sure if newline is expected here. Please revert this change. > Source/WebCore/dom/Document.cpp:4847 > + if (page()) if (Page* page = page()) page->console()->addMessage(source, level, message, requestIdentifier, this); > Source/WebCore/dom/Document.cpp:4858 > + if (page()) Ditto > Source/WebCore/xml/XSLStyleSheetLibxslt.cpp:27 > #include "DOMWindow.h" I still think you don't need to include DOMWindow.h here. > Source/WebCore/xml/XSLTProcessorLibxslt.cpp:30 > #include "DOMWindow.h" Ditto
Created attachment 193750 [details] Patch
Created attachment 193791 [details] Patch
Comment on attachment 193743 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193743&action=review >> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj:13641 >> \ No newline at end of file > > I am not sure if newline is expected here. Please revert this change. done >> Source/WebCore/dom/Document.cpp:4847 >> + if (page()) > > if (Page* page = page()) > page->console()->addMessage(source, level, message, requestIdentifier, this); It is not valid C++ code. 'page' on the both sides of the expressions refer to the same variable. It is a pointer and the () operation is not applicable to it. >> Source/WebCore/xml/XSLStyleSheetLibxslt.cpp:27 >> #include "DOMWindow.h" > > I still think you don't need to include DOMWindow.h here. done >> Source/WebCore/xml/XSLTProcessorLibxslt.cpp:30 >> #include "DOMWindow.h" > > Ditto done
Created attachment 193804 [details] Patch
Created attachment 193833 [details] Patch
Comment on attachment 193833 [details] Patch Clearing flags on attachment: 193833 Committed r146208: <http://trac.webkit.org/changeset/146208>
All reviewed patches have been landed. Closing bug.
Comment on attachment 193743 [details] Patch Clearing obsolete r?