Created attachment 271112 [details] Patch

Created attachment 271122 [details] Patch

Comment on attachment 271122 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271122&action=review > Source/WebCore/css/CSSFontFace.cpp:72 > + // if (!success) { Throw a SyntaxError } This can be done by taking an extra ExceptionCode& argument and setting it to SYNTAX_ERR

Generally I would prefer we took the same approach here as with most of the rest of the CSSOM and separated the internal type (StyleRule, StyleSheetContents,...) and the external CSSOM type (CSSStyleRule, CSSStyleSheet,...). This approach would involve renaming the current CSSFontFace to StyleFontFace or similar and adding a new CSSFontFace type (which calls into StyleFontFace) to provide the public API. Keeping the external and the internal cleanly separated allows more flexibility in defining the internal structures and often leads to cleaner code.

(In reply to comment #4) > Generally I would prefer we took the same approach here as with most of the > rest of the CSSOM and separated the internal type (StyleRule, > StyleSheetContents,...) and the external CSSOM type (CSSStyleRule, > CSSStyleSheet,...). This approach would involve renaming the current > CSSFontFace to StyleFontFace or similar and adding a new CSSFontFace type > (which calls into StyleFontFace) to provide the public API. > > Keeping the external and the internal cleanly separated allows more > flexibility in defining the internal structures and often leads to cleaner > code. This patch is in preparation for implementing a JavaScript API, not a CSSOM API. However, I do agree with you about the distinction between internal and external APIs.

Created attachment 271211 [details] Patch

Committed r196510: <http://trac.webkit.org/changeset/196510>

Comment on attachment 271211 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271211&action=review > Source/WebCore/css/CSSFontFace.h:106 > + CSSFontFace(bool isLocalFallback); Should mark this explicit.