setOrientTo{Auto|Angle} should reflect on orient attribute

third_party/WebKit/Source/core/svg/SVGMarkerElement.cpp

Index: third_party/WebKit/Source/core/svg/SVGMarkerElement.cpp
diff --git a/third_party/WebKit/Source/core/svg/SVGMarkerElement.cpp b/third_party/WebKit/Source/core/svg/SVGMarkerElement.cpp
index fe0e9a4b7ec3bf3978e09526ae4d09cbeb96b015..7730e90f9f67c61b59134eaba08605f463731466 100644
--- a/third_party/WebKit/Source/core/svg/SVGMarkerElement.cpp
+++ b/third_party/WebKit/Source/core/svg/SVGMarkerElement.cpp
@@ -116,6 +116,9 @@ void SVGMarkerElement::childrenChanged(const ChildrenChange& change)
 
 void SVGMarkerElement::setOrientToAuto()
 {
+    if (Attribute* orientAttribute = ensureUniqueElementData().attributes().find(SVGNames::orientAttr))

[fs, 2016/02/19 10:33:13]

This will not do the right thing if this method is called when an 'orient'
attribute has not been specified (in which case the value of the attribute
should still be updated.) It also looks bit hacky TBH.

[Shanmuga Pandi, 2016/02/19 11:30:29]

Yes. you are right.. I got it..
In case 'orient' attrribute is not specified before, then we can call
attributes().append(SVGNames::orientAttr, value)..

If you think it looks bit hacky, then can we simply call setAttribute() as Rob
suggested.

[fs, 2016/02/19 12:38:35]

Yes, calling setAttribute would be preferable to manipulating the attribute
collection manually.

[Shanmuga Pandi, 2016/02/19 14:46:44]

Acknowledged.

[Shanmuga Pandi, 2016/02/22 06:45:07]

Done.

+        orientAttribute->setValue("auto");

[rwlbuis, 2016/02/17 22:38:02]

Have you considered using setAttribute?

[fs, 2016/02/19 10:33:13]

Using setAttribute would likely work fine in these two methods although I
believe it'll miss the bigger issue. For instance consider this (rough
equivalent) script:

markerElement.orientType.baseVal = 1;
markerElement.getAttribute('orient'); // returns?

So the wider issue of attribute synchronization should probably be considered
here as well.

[Shanmuga Pandi, 2016/02/19 11:30:29]

Hmm.. 
The part here "markerElement.orientType.baseVal = 1;" will be handled by
SVGAngleTearOff::setValue() and adding setAttribute/equivalent from here, again
its more of hacky.

[fs, 2016/02/19 12:38:35]

My point is that doing that will show the same issue as calling this method
(because the sequence of operations is almost exactly the same), and
setAttribute should definitely not be added there since that would defeat the
attribute synchronization lazyness. It will not go through SVGAngleTearOff,
since orientType is an SVGAnimatedEnumeration (and hence it's baseVal is an
SVGEnumeration.)

[Shanmuga Pandi, 2016/02/19 14:46:44]

Yes. Copy-Paste problem... It will go through SVGAnimatedEnumeration.

+
     m_orientAngle->baseValue()->orientType()->setEnumValue(SVGMarkerOrientAuto);
     invalidateSVGAttributes();
     svgAttributeBaseValChanged(SVGNames::orientAttr);
@@ -125,6 +128,9 @@ void SVGMarkerElement::setOrientToAngle(PassRefPtrWillBeRawPtr<SVGAngleTearOff>
 {
     ASSERT(angle);
     RefPtrWillBeRawPtr<SVGAngle> target = angle->target();
+    if (Attribute* orientAttribute = ensureUniqueElementData().attributes().find(SVGNames::orientAttr))
+        orientAttribute->setValue(AtomicString(target->valueAsString()));
+
     m_orientAngle->baseValue()->newValueSpecifiedUnits(target->unitType(), target->valueInSpecifiedUnits());
     invalidateSVGAttributes();
     svgAttributeBaseValChanged(SVGNames::orientAttr);