Rename confusing child_id to unique_id_win

content/browser/accessibility/browser_accessibility_manager.cc

 // Copyright (c) 2012 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/accessibility/browser_accessibility_manager.h"
 
 #include "base/logging.h"
 #include "content/browser/accessibility/browser_accessibility.h"
 #include "content/browser/accessibility/browser_accessibility_state_impl.h"
 #include "content/common/accessibility_messages.h"
 
 namespace content {
 
 BrowserAccessibility* BrowserAccessibilityFactory::Create() {
   return BrowserAccessibility::Create();
 }
 
-// Start child IDs at -1 and decrement each time, because clients use
-// child IDs of 1, 2, 3, ... to access the children of an object by
-// index, so we use negative IDs to clearly distinguish between indices
-// and unique IDs.
-// static
-int32 BrowserAccessibilityManager::next_child_id_ = -1;
-
 #if !defined(OS_MACOSX) && \
     !defined(OS_WIN) && \
     !defined(TOOLKIT_GTK)
 // We have subclassess of BrowserAccessibilityManager on Mac, Linux/GTK,
 // and Win. For any other platform, instantiate the base class.
 // static
 BrowserAccessibilityManager* BrowserAccessibilityManager::Create(
     const AccessibilityNodeData& src,
     BrowserAccessibilityDelegate* delegate,
     BrowserAccessibilityFactory* factory) {
@@ skipping 11 matching lines @@
       focus_(NULL),
       osk_state_(OSK_ALLOWED) {
   std::vector<AccessibilityNodeData> nodes;
   nodes.push_back(src);
   if (!UpdateNodes(nodes))
     return;
   if (!focus_)
     SetFocus(root_, false);
 }
 
-// static
-int32 BrowserAccessibilityManager::GetNextChildID() {
-  // Get the next child ID, and wrap around when we get near the end
-  // of a 32-bit integer range. It's okay to wrap around; we just want
-  // to avoid it as long as possible because clients may cache the ID of
-  // an object for a while to determine if they've seen it before.
-  next_child_id_--;
-  if (next_child_id_ == -2000000000)
-    next_child_id_ = -1;
-
-  return next_child_id_;
-}
-
 BrowserAccessibilityManager::~BrowserAccessibilityManager() {
   if (root_)
     root_->Destroy();
 }
 
 BrowserAccessibility* BrowserAccessibilityManager::GetRoot() {
   return root_;
 }
 
-BrowserAccessibility* BrowserAccessibilityManager::GetFromChildID(
-    int32 child_id) {
-  base::hash_map<int32, BrowserAccessibility*>::iterator iter =
-      child_id_map_.find(child_id);
-  if (iter != child_id_map_.end()) {
-    return iter->second;
-  } else {
-    return NULL;
-  }
-}
-
 BrowserAccessibility* BrowserAccessibilityManager::GetFromRendererID(
     int32 renderer_id) {
-  base::hash_map<int32, int32>::iterator iter =
-      renderer_id_to_child_id_map_.find(renderer_id);
-  if (iter == renderer_id_to_child_id_map_.end())
-    return NULL;
-
-  int32 child_id = iter->second;
-  return GetFromChildID(child_id);
+  base::hash_map<int32, BrowserAccessibility*>::iterator iter =
+      renderer_id_map_.find(renderer_id);
+  if (iter != renderer_id_map_.end())
+    return iter->second;
+  return NULL;
 }
 
 void BrowserAccessibilityManager::GotFocus(bool touch_event_context) {
   if (!touch_event_context)
     osk_state_ = OSK_DISALLOWED_BECAUSE_TAB_JUST_APPEARED;
 
   if (!focus_)
     return;
 
   NotifyAccessibilityEvent(AccessibilityNotificationFocusChanged, focus_);
 }
 
 void BrowserAccessibilityManager::WasHidden() {
   osk_state_ = OSK_DISALLOWED_BECAUSE_TAB_HIDDEN;
 }
 
 void BrowserAccessibilityManager::GotMouseDown() {
   osk_state_ = OSK_ALLOWED_WITHIN_FOCUSED_OBJECT;
   NotifyAccessibilityEvent(AccessibilityNotificationFocusChanged, focus_);
 }
 
 bool BrowserAccessibilityManager::IsOSKAllowed(const gfx::Rect& bounds) {
   if (!delegate_ || !delegate_->HasFocus())
     return false;
 
   gfx::Point touch_point = delegate_->GetLastTouchEventLocation();
   return bounds.Contains(touch_point);
 }
 
-void BrowserAccessibilityManager::Remove(BrowserAccessibility* node) {
+void BrowserAccessibilityManager::RemoveNode(BrowserAccessibility* node) {
   if (node == focus_)
     SetFocus(root_, false);
-  int child_id = node->child_id();
   int renderer_id = node->renderer_id();
-  child_id_map_.erase(child_id);
-  DCHECK(renderer_id_to_child_id_map_[renderer_id] == child_id);
-  // Make sure we don't overwrite a newer entry (see UpdateNode for a possible
-  // corner case).
-  if (renderer_id_to_child_id_map_[renderer_id] == child_id)
-    renderer_id_to_child_id_map_.erase(renderer_id);
+  renderer_id_map_.erase(renderer_id);
 }
 
 void BrowserAccessibilityManager::OnAccessibilityNotifications(
     const std::vector<AccessibilityHostMsg_NotificationParams>& params) {
   for (uint32 index = 0; index < params.size(); index++) {
     const AccessibilityHostMsg_NotificationParams& param = params[index];
 
     // Update nodes that changed.
     if (!UpdateNodes(param.nodes))
       return;
@@ skipping 139 matching lines @@
       CHECK(false);
   }
 
   return success;
 }
 
 BrowserAccessibility* BrowserAccessibilityManager::CreateNode(
     BrowserAccessibility* parent,
     int32 renderer_id,
     int32 index_in_parent) {
-  BrowserAccessibility* instance = factory_->Create();
-  int32 child_id = GetNextChildID();
-  instance->InitializeTreeStructure(
-      this, parent, child_id, renderer_id, index_in_parent);
-  child_id_map_[child_id] = instance;
-  renderer_id_to_child_id_map_[renderer_id] = child_id;
-  return instance;
+  BrowserAccessibility* node = factory_->Create();
+  node->InitializeTreeStructure(
+      this, parent, renderer_id, index_in_parent);
+  InitializeNode(node);
+  return node;
+}
+
+void BrowserAccessibilityManager::InitializeNode(BrowserAccessibility* node) {

[aboxhall, 2013/04/09 09:28:29]

This seems like a bit of a confusing name for this method, since it doesn't
actually do anything to the node (and it's called right after
node->InitializeTreeStructure). Perhaps it should be named something like
AddNodeToMap or RegisterNode?

[dmazzoni, 2013/04/09 19:44:04]

Good idea, done.

+  renderer_id_map_[node->renderer_id()] = node;
 }
 
 bool BrowserAccessibilityManager::UpdateNode(const AccessibilityNodeData& src) {
   // This method updates one node in the tree based on serialized data
   // received from the renderer.
 
   // Create a set of child ids in |src| for fast lookup. If a duplicate id is
   // found, exit now with a fatal error before changing anything else.
   std::set<int32> new_child_ids;
   for (size_t i = 0; i < src.child_ids.size(); ++i) {
@@ skipping 75 matching lines @@
   }
 
   // Keep track of what node is focused.
   if ((src.state >> AccessibilityNodeData::STATE_FOCUSED) & 1)
     SetFocus(instance, false);
 
   return success;
 }
 
 }  // namespace content