Setting HTMLMediaElement.controller does not properly remove the 'mediagroup' content attribute

Source/core/html/HTMLMediaElement.cpp

 /*
  * Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 3705 matching lines @@
 
 AudioSourceProvider* HTMLMediaElement::audioSourceProvider()
 {
     if (m_player)
         return m_player->audioSourceProvider();
 
     return 0;
 }
 #endif
 
-const String& HTMLMediaElement::mediaGroup() const
+const AtomicString& HTMLMediaElement::mediaGroup() const
 {
-    return m_mediaGroup;
+    return fastGetAttribute(HTMLNames::mediagroupAttr);
 }
 
-void HTMLMediaElement::setMediaGroup(const String& group)
+void HTMLMediaElement::setMediaGroup(const AtomicString& group)
 {
-    if (m_mediaGroup == group)
-        return;
-    m_mediaGroup = group;
-
     // When a media element is created with a mediagroup attribute, and when a media element's mediagroup
     // attribute is set, changed, or removed, the user agent must run the following steps:
     // 1. Let m [this] be the media element in question.
     // 2. Let m have no current media controller, if it currently has one.
-    setController(0);
+    setControllerInternal(0);
 
     // 3. If m's mediagroup attribute is being removed, then abort these steps.
     if (group.isNull() || group.isEmpty())
         return;
 
     // 4. If there is another media element whose Document is the same as m's Document (even if one or both
     // of these elements are not actually in the Document),
     HashSet<HTMLMediaElement*> elements = documentToElementSetMap().get(&document());
     for (HashSet<HTMLMediaElement*>::iterator i = elements.begin(); i != elements.end(); ++i) {
         if (*i == this)
             continue;
 
         // and which also has a mediagroup attribute, and whose mediagroup attribute has the same value as
         // the new value of m's mediagroup attribute,
         if ((*i)->mediaGroup() == group) {
             //  then let controller be that media element's current media controller.
-            setController((*i)->controller());
+            setControllerInternal((*i)->controller());
             return;
         }
     }
 
     // Otherwise, let controller be a newly created MediaController.
-    setController(MediaController::create(Node::executionContext()));
+    setControllerInternal(MediaController::create(Node::executionContext()));
 }
 
 MediaController* HTMLMediaElement::controller() const
 {
     return m_mediaController.get();
 }
 
 void HTMLMediaElement::setController(PassRefPtr<MediaController> controller)
 {
+    // 4.8.10.11.2 Media controllers: controller attribute.
+    // On setting, it must first remove the element's mediagroup attribute, if any,
+    removeAttribute(HTMLNames::mediagroupAttr);

[philipj_slow, 2013/11/08 19:58:04]

Is the HTMLNames:: prefix needed? The vast majority of places in this file don't
include it, crossoriginAttr being the exception.

[Inactive, 2013/11/08 20:12:06]

Done.

+    // and then set the current media controller to the given value.
+    setControllerInternal(controller);
+}
+
+void HTMLMediaElement::setControllerInternal(PassRefPtr<MediaController> controller)
+{
     if (m_mediaController)
         m_mediaController->removeMediaElement(this);
 
     m_mediaController = controller;
 
     if (m_mediaController)
         m_mediaController->addMediaElement(this);
 
     if (hasMediaControls())
         mediaControls()->setMediaController(m_mediaController ? m_mediaController.get() : static_cast<MediaControllerInterface*>(this));
@@ skipping 87 matching lines @@
 {
     scheduleLayerUpdate();
 }
 
 bool HTMLMediaElement::isInteractiveContent() const
 {
     return fastHasAttribute(controlsAttr);
 }
 
 }