Implementation of brand-color

Source/core/dom/Document.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) Research In Motion Limited 2010-2011. All rights reserved.
@@ skipping 33 matching lines @@
 #include "core/animation/AnimationTimeline.h"
 #include "core/animation/DocumentAnimations.h"
 #include "core/css/CSSFontSelector.h"
 #include "core/css/CSSStyleDeclaration.h"
 #include "core/css/CSSStyleSheet.h"
 #include "core/css/MediaQueryMatcher.h"
 #include "core/css/StylePropertySet.h"
 #include "core/css/StyleSheetContents.h"
 #include "core/css/StyleSheetList.h"
 #include "core/css/invalidation/StyleInvalidator.h"
+#include "core/css/parser/CSSPropertyParser.h"
 #include "core/css/resolver/FontBuilder.h"
 #include "core/css/resolver/StyleResolver.h"
 #include "core/css/resolver/StyleResolverStats.h"
 #include "core/dom/AddConsoleMessageTask.h"
 #include "core/dom/Attr.h"
 #include "core/dom/CDATASection.h"
 #include "core/dom/Comment.h"
 #include "core/dom/ContextFeatures.h"
 #include "core/dom/DOMImplementation.h"
 #include "core/dom/DocumentFragment.h"
@@ skipping 4666 matching lines @@
 
     if (firstTouchIcon.m_iconType != InvalidIcon)
         iconURLs.append(firstTouchIcon);
     if (firstTouchPrecomposedIcon.m_iconType != InvalidIcon)
         iconURLs.append(firstTouchPrecomposedIcon);
     for (int i = secondaryIcons.size() - 1; i >= 0; --i)
         iconURLs.append(secondaryIcons[i]);
     return iconURLs;
 }
 
+Color Document::brandColor() const
+{
+    if (!RuntimeEnabledFeatures::brandColorEnabled())
+        return Color();
+
+    for (HTMLMetaElement* metaElement = head() ? Traversal<HTMLMetaElement>::firstChild(*head()) : 0; metaElement; metaElement = Traversal<HTMLMetaElement>::nextSibling(*metaElement)) {

[esprehn, 2014/06/19 10:37:04]

You call this method from inside HTMLMetaElement::process() which already knows
which element you're looking for. Crawling the head again is just wasted work.

I don't think you want to expose a property on document like this. Instead you
should plumb the color down through the dispatchDidChangeBrandColor(color)
notification and HTMLMetaElement should contain this processing. Document should
have no knowledge of this feature.

[michaelbai, 2014/06/19 16:42:51]

In the case that there are multiple brand-color meta tags, I just want the last
one, even others are changed later.

On 2014/06/19 10:37:04, esprehn wrote:

[esprehn, 2014/06/19 18:02:20]

That's not what your code does. Every time a <meta> is inserted you notify the
embedder of it, then the embedder calls this function and it'll keep finding the
first one that was inserted, not the last one.

Note also that the parser can yield arbitrarily, so you could get one <meta>,
the embedder tells the browser process which does something with the color, then
the parser could resume and another <meta> would be inserted. Script can do that
too by just appendChild'ing meta elements inside setTimeouts.

Finally crawling the head() looking for <meta> is not correct. They can show up
anywhere in the page, and you will even notify the embedder when they do.

+        RGBA32 rgb;
+        if (equalIgnoringCase(metaElement->name(), "brand-color") && CSSPropertyParser::fastParseColor(rgb, metaElement->content().string().stripWhiteSpace(), true))

[esprehn, 2014/06/19 10:37:04]

CSSPropertyParser::fastParseColor is a private API of the parser, you shouldn't
be using it here. You end up allowing rgb() and rgba() syntax in addition to hex
and keywords, but you disallow hsl() and hsla() colors and other things we
didn't fast path. 

You want to be using BisonCSSParser::parseColor.

[michaelbai, 2014/06/19 16:42:51]

We only support color name, rgba(), rgb() or "#" syntax in brand-color.


On 2014/06/19 10:37:04, esprehn wrote:

[esprehn, 2014/06/19 18:02:21]

Why only support an arbitrary subset of the css color spec? That doesn't make
sense. It's a one line change to follow the CSS color spec here, and the
embedded doesn't have to know anything about it.

+            return Color(rgb);
+    }
+    return Color();
+}
+
 HTMLLinkElement* Document::linkManifest() const
 {
     HTMLHeadElement* head = this->head();
     if (!head)
         return 0;
 
     // The first link element with a manifest rel must be used. Others are ignored.
     for (HTMLLinkElement* linkElement = Traversal<HTMLLinkElement>::firstChild(*head); linkElement; linkElement = Traversal<HTMLLinkElement>::nextSibling(*linkElement)) {
         if (!linkElement->relAttribute().isManifest())
             continue;
@@ skipping 1085 matching lines @@
     visitor->trace(m_compositorPendingAnimations);
     visitor->trace(m_contextDocument);
     visitor->registerWeakMembers<Document, &Document::clearWeakMembers>(this);
     DocumentSupplementable::trace(visitor);
     TreeScope::trace(visitor);
     ContainerNode::trace(visitor);
     ExecutionContext::trace(visitor);
 }
 
 } // namespace WebCore