Add style information to the snapshot node

content/browser/web_contents/web_contents_android.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/web_contents/web_contents_android.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/command_line.h"
@@ skipping 42 matching lines @@
 }
 
 ScopedJavaLocalRef<jobject> WalkAXTreeDepthFirst(JNIEnv* env,
       BrowserAccessibilityAndroid* node) {
 
   ScopedJavaLocalRef<jstring> j_text =
       ConvertUTF16ToJavaString(env, node->GetText());
   ScopedJavaLocalRef<jstring> j_class =
       ConvertUTF8ToJavaString(env, node->GetClassName());
   const gfx::Rect& location = node->GetLocation();
+  // The style attributes exists and valid if size attribute exists. Otherwise,
+  // they are not. Use a negative size information to indicate the existence
+  // of style information.
+  float size = -1.0;
+  int color = 0;
+  int bgcolor = 0;
+  int text_style = 0;
+  if (node->HasFloatAttribute(ui::AX_ATTR_FONT_SIZE)) {
+    color = node->GetIntAttribute(ui::AX_ATTR_COLOR);

[Ted C, 2015/05/29 17:04:29]

can you add a DCHECK above each one of these using Has<XXX>Attribute?

[dmazzoni, 2015/05/29 17:06:01]

No, we deliberately filter out zero values and empty strings from attributes so
if you want the value of the attribute including possibly zero, this is the
right way to do it.

[Ted C, 2015/05/29 17:10:37]

Huh?

So HasFloatAttribute returns false if the value is 0?  I'm more concerned that
all these attributes are associated with size, which seems odd/bad/incorrect to
me.

I'd rather have some means of asserting that is true and that if size is
specified
then so is everything else.  So if there is no way to do it currently, it seems
like something we should add and a TODO should be here referencing the bug for
it.

[sgurun-gerrit only, 2015/05/29 17:23:19]

I will add a todo and a crbug and we can discuss how to do it better there.
maybe a better way to encapsulate this information is having something like
hasStyleInformation hiding all the internal details.

+    bgcolor = node->GetIntAttribute(ui::AX_ATTR_BACKGROUND_COLOR);
+    size =  node->GetFloatAttribute(ui::AX_ATTR_FONT_SIZE);
+    text_style = node->GetIntAttribute(ui::AX_ATTR_TEXT_STYLE);
+  }
+
   ScopedJavaLocalRef<jobject> j_node =
       Java_WebContentsImpl_createAccessibilitySnapshotNode(env,
           location.x(), location.y(), node->GetScrollX(),
           node->GetScrollY(), location.width(), location.height(),
-          j_text.obj(), j_class.obj());
+          j_text.obj(), color, bgcolor, size, text_style, j_class.obj());
 
   for(uint32 i = 0; i < node->PlatformChildCount(); i++) {
     BrowserAccessibilityAndroid* child =
         static_cast<BrowserAccessibilityAndroid*>(
             node->PlatformGetChild(i));
     Java_WebContentsImpl_addAccessibilityNodeAsChild(env,
         j_node.obj(), WalkAXTreeDepthFirst(env, child).obj());
   }
   return j_node;
 }
@@ skipping 488 matching lines @@
   ScopedJavaGlobalRef<jobject> j_callback;
   j_callback.Reset(env, callback);
   WebContentsImpl::AXTreeSnapshotCallback snapshot_callback =
       base::Bind(&AXTreeSnapshotCallback, j_callback);
 
   static_cast<WebContentsImpl*>(web_contents_)->RequestAXTreeSnapshot(
       snapshot_callback);
 }
 
 }  // namespace content