Bug 121893 – CTTE: More Document references please

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

CTTE: More Document references please

[Sam Weinig]

Created attachment 212537 [details]
Patch

[WebKit Commit Bot]

Attachment 212537 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/css/CSSCursorImageValue.cpp', u'Source/WebCore/css/CSSCursorImageValue.h', u'Source/WebCore/css/CSSImageSetValue.cpp', u'Source/WebCore/css/CSSImageSetValue.h', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParser.h', u'Source/WebCore/css/CSSParserMode.h', u'Source/WebCore/css/CSSStyleSheet.cpp', u'Source/WebCore/css/CSSStyleSheet.h', u'Source/WebCore/css/SVGCSSStyleSelector.cpp', u'Source/WebCore/css/StyleResolver.cpp', u'Source/WebCore/dom/Document.cpp', u'Source/WebCore/dom/DocumentStyleSheetCollection.cpp', u'Source/WebCore/dom/DocumentStyleSheetCollection.h', u'Source/WebCore/dom/Element.cpp', u'Source/WebCore/dom/Element.h', u'Source/WebCore/dom/InlineStyleSheetOwner.cpp', u'Source/WebCore/dom/InlineStyleSheetOwner.h', u'Source/WebCore/dom/Node.cpp', u'Source/WebCore/dom/ProcessingInstruction.cpp', u'Source/WebCore/dom/SelectorQuery.cpp', u'Source/WebCore/dom/SelectorQuery.h', u'Source/WebCore/editing/CompositeEditCommand.cpp', u'Source/WebCore/editing/CompositeEditCommand.h', u'Source/WebCore/html/HTMLBodyElement.cpp', u'Source/WebCore/html/HTMLDocument.h', u'Source/WebCore/html/HTMLLinkElement.cpp', u'Source/WebCore/html/HTMLMediaElement.cpp', u'Source/WebCore/html/HTMLPlugInImageElement.cpp', u'Source/WebCore/html/HTMLStyleElement.cpp', u'Source/WebCore/html/HTMLStyleElement.h', u'Source/WebCore/html/MediaDocument.h', u'Source/WebCore/inspector/InspectorStyleSheet.cpp', u'Source/WebCore/loader/FormSubmission.cpp', u'Source/WebCore/platform/network/FormDataBuilder.cpp', u'Source/WebCore/platform/network/FormDataBuilder.h', u'Source/WebCore/rendering/RenderFullScreen.cpp', u'Source/WebCore/rendering/RenderFullScreen.h', u'Source/WebCore/rendering/svg/RenderSVGResourceContainer.h', u'Source/WebCore/rendering/svg/RenderSVGTextPath.cpp', u'Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp', u'Source/WebCore/rendering/svg/SVGResources.cpp', u'Source/WebCore/style/StyleResolveTree.cpp', u'Source/WebCore/svg/SVGAltGlyphElement.cpp', u'Source/WebCore/svg/SVGFEImageElement.cpp', u'Source/WebCore/svg/SVGGlyphRefElement.cpp', u'Source/WebCore/svg/SVGLinearGradientElement.cpp', u'Source/WebCore/svg/SVGMPathElement.cpp', u'Source/WebCore/svg/SVGPatternElement.cpp', u'Source/WebCore/svg/SVGRadialGradientElement.cpp', u'Source/WebCore/svg/SVGStyleElement.cpp', u'Source/WebCore/svg/SVGStyleElement.h', u'Source/WebCore/svg/SVGTRefElement.cpp', u'Source/WebCore/svg/SVGTextPathElement.cpp', u'Source/WebCore/svg/SVGURIReference.cpp', u'Source/WebCore/svg/SVGURIReference.h', u'Source/WebCore/svg/SVGUseElement.cpp', u'Source/WebCore/svg/animation/SVGSMILElement.cpp', u'Source/WebCore/svg/graphics/filters/SVGFEImage.cpp', u'Source/WebCore/svg/graphics/filters/SVGFEImage.h', u'Source/WebCore/testing/Internals.cpp', u'Source/WebCore/testing/Internals.h', u'Source/WebCore/testing/Internals.idl']" exit_code: 1
Source/WebCore/ChangeLog:8:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/WebCore/dom/DocumentStyleSheetCollection.h:53:  Missing spaces around >>  [whitespace/operators] [3]
Source/WebCore/dom/DocumentStyleSheetCollection.h:55:  Missing spaces around >>  [whitespace/operators] [3]
Source/WebCore/dom/DocumentStyleSheetCollection.h:58:  Missing spaces around >>  [whitespace/operators] [3]
Source/WebCore/dom/DocumentStyleSheetCollection.h:59:  Missing spaces around >>  [whitespace/operators] [3]
Source/WebCore/dom/DocumentStyleSheetCollection.h:60:  Missing spaces around >>  [whitespace/operators] [3]
Source/WebCore/dom/DocumentStyleSheetCollection.h:61:  Missing spaces around >>  [whitespace/operators] [3]
Total errors found: 7 in 65 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 212537 [details]
Patch

Attachment 212537 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/2190108

[Geoffrey Garen]

Comment on attachment 212537 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=212537&action=review

Creating library C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\lib32\WebKit.lib and object C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\lib32\WebKit.exp
     1>WebKit.exp : error LNK2001: unresolved external symbol "public: __thiscall WebCore::CSSParserContext::CSSParserContext(class WebCore::Document *,class WebCore::KURL const &,class WTF::String const &)" (??0CSSParserContext@WebCore@@QAE@PAVDocument@1@ABVKURL@1@ABVString@WTF@@@Z)

> Source/WebCore/testing/Internals.cpp:1652
> -void Internals::insertAuthorCSS(Document* document, const String& css) const
> +void Internals::insertAuthorCSS(Document* document, const String& css, ExceptionCode& ec) const
>  {
> -    RefPtr<StyleSheetContents> parsedSheet = StyleSheetContents::create(document);
> +    if (!document) {
> +        ec = INVALID_ACCESS_ERR;
> +        return;
> +    }
> +
> +    RefPtr<StyleSheetContents> parsedSheet = StyleSheetContents::create(*document);
>      parsedSheet->setIsUserStyleSheet(false);
>      parsedSheet->parseString(css);
>      document->styleSheetCollection()->addAuthorSheet(parsedSheet);
>  }
>  
> -void Internals::insertUserCSS(Document* document, const String& css) const
> +void Internals::insertUserCSS(Document* document, const String& css, ExceptionCode& ec) const
>  {
> -    RefPtr<StyleSheetContents> parsedSheet = StyleSheetContents::create(document);
> +    if (!document) {
> +        ec = INVALID_ACCESS_ERR;
> +        return;
> +    }
> +
> +    RefPtr<StyleSheetContents> parsedSheet = StyleSheetContents::create(*document);

When can these be NULL? Do we have a test for this DOM exception? (At a glance, it seems like the old code didn't throw a DOM exception.)

[Sam Weinig]

(In reply to comment #4)
> (From update of attachment 212537 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=212537&action=review
> 
> Creating library C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\lib32\WebKit.lib and object C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\lib32\WebKit.exp
>      1>WebKit.exp : error LNK2001: unresolved external symbol "public: __thiscall WebCore::CSSParserContext::CSSParserContext(class WebCore::Document *,class WebCore::KURL const &,class WTF::String const &)" (??0CSSParserContext@WebCore@@QAE@PAVDocument@1@ABVKURL@1@ABVString@WTF@@@Z)
> 
> > Source/WebCore/testing/Internals.cpp:1652
> > -void Internals::insertAuthorCSS(Document* document, const String& css) const
> > +void Internals::insertAuthorCSS(Document* document, const String& css, ExceptionCode& ec) const
> >  {
> > -    RefPtr<StyleSheetContents> parsedSheet = StyleSheetContents::create(document);
> > +    if (!document) {
> > +        ec = INVALID_ACCESS_ERR;
> > +        return;
> > +    }
> > +
> > +    RefPtr<StyleSheetContents> parsedSheet = StyleSheetContents::create(*document);
> >      parsedSheet->setIsUserStyleSheet(false);
> >      parsedSheet->parseString(css);
> >      document->styleSheetCollection()->addAuthorSheet(parsedSheet);
> >  }
> >  
> > -void Internals::insertUserCSS(Document* document, const String& css) const
> > +void Internals::insertUserCSS(Document* document, const String& css, ExceptionCode& ec) const
> >  {
> > -    RefPtr<StyleSheetContents> parsedSheet = StyleSheetContents::create(document);
> > +    if (!document) {
> > +        ec = INVALID_ACCESS_ERR;
> > +        return;
> > +    }
> > +
> > +    RefPtr<StyleSheetContents> parsedSheet = StyleSheetContents::create(*document);
> 
> When can these be NULL? Do we have a test for this DOM exception? (At a glance, it seems like the old code didn't throw a DOM exception.)

This can be null if in a layout test you do window.internals.insertAuthorCSS(null).  All the other Internals functions that take a document also check for null, so this makes it consistent.  I don't see a good reason to add a test for the function we use only for testing :(.

[Sam Weinig]

Committed r156408: <http://trac.webkit.org/changeset/156408>