"non-generated" because this change will not touch the idl.
Created attachment 52342 [details] Proposed fix.
Attachment 52342 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/1568160
Created attachment 52344 [details] Proposed fix. Fixes the qt build problem (and any platform on which dashboard support isn't enabled).
Comment on attachment 52344 [details] Proposed fix. > +RenderBox* CanvasSurface::domRenderBox() const > +RenderStyle* CanvasSurface::domComputedStyle() Why do these two function names start with the prefix, DOM? Would the names be ambiguous or confusing without it? I suggest removing it. > +CanvasRenderingContext2D::CanvasRenderingContext2D(HTMLCanvasElement* canvas, bool inCompatMode, bool usesDashbardCompatibilityMode) I don't think there's enough context here for the names inCompatMode and m_inCompatMode. In WebCore::Document it's already a pretty weak name, but here it's even weaker. I think usesCompatParseModeForCSS would be better. You might be able to think of something even better. > + // Silence unused warnings when dashboard support isn't enabled. > + ((void) usesDashbardCompatibilityMode); This should be: #if !ENABLE(DASHBOARD_SUPPORT) ASSERT_UNUSED(usesDashboardCompatibilityMode, !usesDashboardCompatibilityMode); #endif The "cast to void" unused parameter idiom should never be used directly, only through either the UNUSED_PARAM or ASSERT_UNUSED macro. review- primarily because of the cast to void
Created attachment 52443 [details] Proposed fix.
Created attachment 52444 [details] Proposed fix.
Comment on attachment 52443 [details] Proposed fix. You forgot to set review? on this. Or was it intentional? This is only a diff from what you previously posted. Is that to help me see what you did in response to my suggestions? > + RenderStyle* computedStyle() { return HTMLElement::computedStyle(); } There's an extra space here after the "{" character. > - CSSParser parser(!m_inCompatMode); // Use the parse mode of the canvas' document when parsing CSS. > + CSSParser parser(!m_usesCSSCompatibilityParseMode); Heh, now that I see this I wish we had reversed the sense so it was strict, but that would be inconvenient for the caller, so I retract that suggestion.
I see now. Heh, jumped the gun.
Comment on attachment 52444 [details] Proposed fix. > + RenderStyle* computedStyle() { return HTMLElement::computedStyle(); } Extra space after the "{".
(In reply to comment #7) > (From update of attachment 52443 [details]) > You forgot to set review? on this. Or was it intentional? This is only a diff > from what you previously posted. Is that to help me see what you did in > response to my suggestions? No, I goofed when I told git the revision number to do the diff from. Sorry about that (but the change is completely there). (I had a minor error in the ChangeLog that I caught last time when I removed the r?.) I'll fix the space issue and commit. Thx!
Committed as http://trac.webkit.org/changeset/57020
http://trac.webkit.org/changeset/57020 might have broken Chromium Win Release, Chromium Mac Release, and Chromium Linux Release