Browser accessibility improvements so that screen readers can access more

complicated webpages without problems.

First, WebAccessibility now works around a "multiple inheritance problem"
in WebCore::AccessibilityObject where the same node appears as a child of
multiple parents. For example, a table cell appears as a child of both a
row and a column. This is solved by having each WebAccessibility parent
check whether the child lists itself as an ancestor. If not, it notes the
child's id only in a separate vector, so each child appears fully only once.

Second, BrowserAccessibility now has internal reference counting, which allows
BrowserAccessibilityManager to update any subtree while maximally reusing
as many objects as possible. This fixes many screen reader interaction
problems!

All of this new functionality is tested with new cross-platform tests.

BUG=67192
BUG=67620
TEST=Adds new unit tests and a browser test.

Review URL: http://codereview.chromium.org/6625042

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@77316 0039d316-1c4b-4281-b951-d872f2087c98

14 files changed, 831 insertions, 175 deletions

diff --git a/chrome/browser/accessibility/accessibility_win_browsertest.cc b/chrome/browser/accessibility/accessibility_win_browsertest.cc
index 8c205e1..223f5f5 100644
--- a/chrome/browser/accessibility/accessibility_win_browsertest.cc
+++ b/chrome/browser/accessibility/accessibility_win_browsertest.cc
@@ -563,10 +563,13 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
       NotificationType::RENDER_VIEW_HOST_ACCESSIBILITY_TREE_UPDATED);
 
   // Check the browser's copy of the renderer accessibility tree.
+
+  AccessibleChecker text_field_div_checker(L"", L"div", L"");
   AccessibleChecker text_field_checker(L"", ROLE_SYSTEM_TEXT, L"old value");
   text_field_checker.SetExpectedState(STATE_SYSTEM_FOCUSABLE);
   AccessibleChecker body_checker(L"", L"body", L"");
   AccessibleChecker document_checker(L"", ROLE_SYSTEM_DOCUMENT, L"");
+  text_field_checker.AppendExpectedChild(&text_field_div_checker);
   body_checker.AppendExpectedChild(&text_field_checker);
   document_checker.AppendExpectedChild(&body_checker);
   document_checker.CheckAccessible(GetRendererAccessible());
diff --git a/chrome/browser/accessibility/browser_accessibility.cc b/chrome/browser/accessibility/browser_accessibility.cc
index 42cc878..290ff43 100644
--- a/chrome/browser/accessibility/browser_accessibility.cc
+++ b/chrome/browser/accessibility/browser_accessibility.cc
@@ -8,14 +8,25 @@
 #include "base/string_number_conversions.h"
 #include "chrome/browser/accessibility/browser_accessibility_manager.h"
 
+#if defined(OS_LINUX)
+// There's no OS-specific implementation of BrowserAccessibilityManager
+// on Linux, so just instantiate the base class.
+// static
+BrowserAccessibility* BrowserAccessibility::Create() {
+  return new BrowserAccessibility();
+}
+#endif
+
 BrowserAccessibility::BrowserAccessibility()
     : manager_(NULL),
       parent_(NULL),
       child_id_(0),
       index_in_parent_(0),
       renderer_id_(0),
+      ref_count_(1),
       role_(0),
-      state_(0) {
+      state_(0),
+      instance_active_(false) {
 }
 
 BrowserAccessibility::~BrowserAccessibility() {
@@ -48,29 +59,33 @@ void BrowserAccessibility::Initialize(
   location_ = src.location;
   role_ = src.role;
   state_ = src.state;
+  indirect_child_ids_ = src.indirect_child_ids;
 
   Initialize();
 }
 
-void BrowserAccessibility::ReleaseTree() {
-  // Now we can safely call InactivateTree on our children and remove
-  // references to them, so that as much of the tree as possible will be
-  // destroyed now - however, nodes that still have references to them
-  // might stick around a while until all clients have released them.
-  for (std::vector<BrowserAccessibility*>::iterator iter =
-           children_.begin();
-       iter != children_.end(); ++iter) {
-    (*iter)->ReleaseTree();
-    (*iter)->ReleaseReference();
-  }
-  children_.clear();
-  manager_->Remove(child_id_);
+void BrowserAccessibility::Initialize() {
+  instance_active_ = true;
 }
 
 void BrowserAccessibility::AddChild(BrowserAccessibility* child) {
   children_.push_back(child);
 }
 
+void BrowserAccessibility::DetachTree(
+    std::vector<BrowserAccessibility*>* nodes) {
+  nodes->push_back(this);
+  for (size_t i = 0; i < children_.size(); i++)
+    children_[i]->DetachTree(nodes);
+  children_.clear();
+}
+
+void BrowserAccessibility::UpdateParent(BrowserAccessibility* parent,
+                                        int index_in_parent) {
+  parent_ = parent;
+  index_in_parent_ = index_in_parent;
+}
+
 bool BrowserAccessibility::IsDescendantOf(
     BrowserAccessibility* ancestor) {
   if (this == ancestor) {
@@ -145,6 +160,30 @@ BrowserAccessibility* BrowserAccessibility::BrowserAccessibilityForPoint(
   return this;
 }
 
+void BrowserAccessibility::InternalAddReference() {
+  ref_count_++;
+}
+
+void BrowserAccessibility::InternalReleaseReference(bool recursive) {
+  DCHECK_GT(ref_count_, 0);
+
+  if (recursive || ref_count_ == 1) {
+    for (std::vector<BrowserAccessibility*>::iterator iter = children_.begin();
+         iter != children_.end();
+         ++iter) {
+      (*iter)->InternalReleaseReference(true);
+    }
+  }
+
+  ref_count_--;
+  if (ref_count_ == 0) {
+    instance_active_ = false;
+    children_.clear();
+    manager_->Remove(child_id_);
+    NativeReleaseReference();
+  }
+}
+
 bool BrowserAccessibility::HasAttribute(
     WebAccessibility::Attribute attribute) {
   return (attributes_.find(attribute) != attributes_.end());
diff --git a/chrome/browser/accessibility/browser_accessibility.h b/chrome/browser/accessibility/browser_accessibility.h
index 62b5966..d356b56 100644
--- a/chrome/browser/accessibility/browser_accessibility.h
+++ b/chrome/browser/accessibility/browser_accessibility.h
@@ -46,21 +46,15 @@ class BrowserAccessibility {
   // Perform platform specific initialization. This can be called multiple times
   // during the lifetime of this instance after the members of this base object
   // have been reset with new values from the renderer process.
-  virtual void Initialize() = 0;
-
-  // Remove references to all children and delete them if possible.
-  virtual void ReleaseTree();
-
-  // Release a reference to this node. This may be a no-op on platforms other
-  // than windows.
-  virtual void ReleaseReference() = 0;
+  virtual void Initialize();
 
   // Replace a child object. Used when updating the accessibility tree.
   virtual void ReplaceChild(
       BrowserAccessibility* old_acc,
       BrowserAccessibility* new_acc);
 
-  // Initialize this object
+  // Initialize this object, reading attributes from |src|. Does not
+  // recurse into children of |src| and build the whole subtree.
   void Initialize(BrowserAccessibilityManager* manager,
                   BrowserAccessibility* parent,
                   int32 child_id,
@@ -70,6 +64,13 @@ class BrowserAccessibility {
   // Add a child of this object.
   void AddChild(BrowserAccessibility* child);
 
+  // Detach all descendants of this subtree and push all of the node pointers,
+  // including this node, onto the end of |nodes|.
+  void DetachTree(std::vector<BrowserAccessibility*>* nodes);
+
+  // Update the parent and index in parent if this node has been moved.
+  void UpdateParent(BrowserAccessibility* parent, int index_in_parent);
+
   // Return true if this object is equal to or a descendant of |ancestor|.
   bool IsDescendantOf(BrowserAccessibility* ancestor);
 
@@ -96,7 +97,43 @@ class BrowserAccessibility {
   // Returns the deepest descendant that contains the specified point.
   BrowserAccessibility* BrowserAccessibilityForPoint(const gfx::Point& point);
 
+  //
+  // Reference counting
+  //
+  // Each object has an internal reference count and many platform
+  // implementations may also use native reference counting.
+  //
+  // The internal reference counting is used because sometimes
+  // multiple references to the same object exist temporarily during
+  // an update. When the internal reference count reaches zero,
+  // NativeReleaseReference is called.
+  //
+  // Native reference counting is used on some platforms because the
+  // operating system may hold onto a reference to a BrowserAccessibility
+  // object even after we're through with it. On these platforms, when
+  // the internal reference count reaches zero, instance_active is set
+  // to zero, and all queries on this object should return failure.
+  // The object isn't actually deleted until the operating system releases
+  // all of its references.
+  //
+
+  // Increment this node's internal reference count.
+  virtual void InternalAddReference();
+
+  // Decrement this node's internal reference count. If the reference count
+  // reaches zero, call NativeReleaseReference().
+  virtual void InternalReleaseReference(bool recursive);
+
+  // Subclasses should override this to support platform reference counting.
+  virtual void NativeAddReference() { }
+
+  // Subclasses should override this to support platform reference counting.
+  virtual void NativeReleaseReference() { delete this; }
+
+  //
   // Accessors
+  //
+
   const std::map<int32, string16>& attributes() const { return attributes_; }
   int32 child_id() const { return child_id_; }
   const std::vector<BrowserAccessibility*>& children() const {
@@ -114,6 +151,8 @@ class BrowserAccessibility {
   const string16& role_name() const { return role_name_; }
   int32 state() const { return state_; }
   const string16& value() const { return value_; }
+  bool instance_active() const { return instance_active_; }
+  int32 ref_count() const { return ref_count_; }
 
 #if defined(OS_MACOSX) && __OBJC__
   BrowserAccessibilityCocoa* toBrowserAccessibilityCocoa();
@@ -158,6 +197,9 @@ class BrowserAccessibility {
   // The children of this object.
   std::vector<BrowserAccessibility*> children_;
 
+  // The number of internal references to this object.
+  int32 ref_count_;
+
   // Accessibility metadata from the renderer
   string16 name_;
   string16 value_;
@@ -167,6 +209,14 @@ class BrowserAccessibility {
   int32 state_;
   string16 role_name_;
   WebKit::WebRect location_;
+  std::vector<int32> indirect_child_ids_;
+
+  // BrowserAccessibility objects are reference-counted on some platforms.
+  // When we're done with this object and it's removed from our accessibility
+  // tree, a client may still be holding onto a pointer to this object, so
+  // we mark it as inactive so that calls to any of this object's methods
+  // immediately return failure.
+  bool instance_active_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserAccessibility);
diff --git a/chrome/browser/accessibility/browser_accessibility_mac.h b/chrome/browser/accessibility/browser_accessibility_mac.h
index ff1eedd..a586bb3 100644
--- a/chrome/browser/accessibility/browser_accessibility_mac.h
+++ b/chrome/browser/accessibility/browser_accessibility_mac.h
@@ -19,7 +19,7 @@ class BrowserAccessibilityMac : public BrowserAccessibility {
  public:
   // Implementation of BrowserAccessibility.
   virtual void Initialize();
-  virtual void ReleaseReference();
+  virtual void NativeReleaseReference();
 
   // Overrides from BrowserAccessibility.
   // Used to know when to update the cocoa children.
diff --git a/chrome/browser/accessibility/browser_accessibility_mac.mm b/chrome/browser/accessibility/browser_accessibility_mac.mm
index e8698a0..97f016a 100644
--- a/chrome/browser/accessibility/browser_accessibility_mac.mm
+++ b/chrome/browser/accessibility/browser_accessibility_mac.mm
@@ -22,6 +22,8 @@ BrowserAccessibilityMac::BrowserAccessibilityMac()
 }
 
 void BrowserAccessibilityMac::Initialize() {
+  BrowserAccessibility::Initialize();
+
   if (browser_accessibility_cocoa_)
     return;
 
@@ -31,7 +33,7 @@ void BrowserAccessibilityMac::Initialize() {
       delegate:(RenderWidgetHostViewCocoa*)manager_->GetParentView()];
 }
 
-void BrowserAccessibilityMac::ReleaseReference() {
+void BrowserAccessibilityMac::NativeReleaseReference() {
   if (browser_accessibility_cocoa_) {
     BrowserAccessibilityCocoa* temp = browser_accessibility_cocoa_;
     browser_accessibility_cocoa_ = nil;
diff --git a/chrome/browser/accessibility/browser_accessibility_manager.cc b/chrome/browser/accessibility/browser_accessibility_manager.cc
index 0f80e4f..f2c1bbc 100644
--- a/chrome/browser/accessibility/browser_accessibility_manager.cc
+++ b/chrome/browser/accessibility/browser_accessibility_manager.cc
@@ -10,6 +10,10 @@
 
 using webkit_glue::WebAccessibility;
 
+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
@@ -17,18 +21,32 @@ using webkit_glue::WebAccessibility;
 // static
 int32 BrowserAccessibilityManager::next_child_id_ = -1;
 
+#if defined(OS_LINUX)
+// There's no OS-specific implementation of BrowserAccessibilityManager
+// on Linux, so just instantiate the base class.
+// static
+BrowserAccessibilityManager* BrowserAccessibilityManager::Create(
+    gfx::NativeView parent_view,
+    const WebAccessibility& src,
+    BrowserAccessibilityDelegate* delegate,
+    BrowserAccessibilityFactory* factory) {
+  return new BrowserAccessibilityManager(
+      parent_view, src, delegate, factory);
+}
+#endif
+
 BrowserAccessibilityManager::BrowserAccessibilityManager(
     gfx::NativeView parent_view,
-      const WebAccessibility& src,
-      BrowserAccessibilityDelegate* delegate,
-      BrowserAccessibilityFactory* factory)
+    const WebAccessibility& src,
+    BrowserAccessibilityDelegate* delegate,
+    BrowserAccessibilityFactory* factory)
     : parent_view_(parent_view),
       delegate_(delegate),
       factory_(factory),
       focus_(NULL) {
-  root_ = CreateAccessibilityTree(NULL, GetNextChildID(), src, 0);
+  root_ = CreateAccessibilityTree(NULL, src, 0);
   if (!focus_)
-    focus_ = root_;
+    SetFocus(root_, false);
 }
 
 // static
@@ -46,11 +64,11 @@ int32 BrowserAccessibilityManager::GetNextChildID() {
 
 BrowserAccessibilityManager::~BrowserAccessibilityManager() {
   // Clients could still hold references to some nodes of the tree, so
-  // calling Inactivate will make sure that as many nodes as possible are
-  // released now, and remaining nodes are marked as inactive so that
-  // calls to any methods on them will return E_FAIL;
-  root_->ReleaseTree();
-  root_->ReleaseReference();
+  // calling InternalReleaseReference will make sure that as many nodes
+  // as possible are released now, and remaining nodes are marked as
+  // inactive so that calls to any methods on them will fail gracefully.
+  focus_->InternalReleaseReference(false);
+  root_->InternalReleaseReference(true);
 }
 
 BrowserAccessibility* BrowserAccessibilityManager::GetRoot() {
@@ -139,27 +157,26 @@ void BrowserAccessibilityManager::OnAccessibilityObjectFocusChange(
   if (!new_browser_acc)
     return;
 
-  focus_ = new_browser_acc;
-  if (delegate_ && delegate_->HasFocus())
+  SetFocus(new_browser_acc, false);
+  if (delegate_ && delegate_->HasFocus()) {
     GotFocus();
-  // Mac currently does not have a BrowserAccessibilityDelegate.
-  else if (!delegate_) {
+  } else if (!delegate_) {
+    // Mac currently does not have a BrowserAccessibilityDelegate.
     NotifyAccessibilityEvent(
         ViewHostMsg_AccessibilityNotification_Params::
-            NOTIFICATION_TYPE_FOCUS_CHANGED,
+        NOTIFICATION_TYPE_FOCUS_CHANGED,
         focus_);
   }
 }
 
 void BrowserAccessibilityManager::OnAccessibilityObjectLoadComplete(
   const WebAccessibility& acc_obj) {
-  root_->ReleaseTree();
-  root_->ReleaseReference();
-  focus_ = NULL;
+  SetFocus(NULL, false);
+  root_->InternalReleaseReference(true);
 
-  root_ = CreateAccessibilityTree(NULL, GetNextChildID(), acc_obj, 0);
+  root_ = CreateAccessibilityTree(NULL, acc_obj, 0);
   if (!focus_)
-    focus_ = root_;
+    SetFocus(root_, false);
 
   NotifyAccessibilityEvent(
       ViewHostMsg_AccessibilityNotification_Params::
@@ -217,9 +234,15 @@ BrowserAccessibility* BrowserAccessibilityManager::GetFocus(
 }
 
 void BrowserAccessibilityManager::SetFocus(
-    const BrowserAccessibility& node) {
-  if (delegate_)
-    delegate_->SetAccessibilityFocus(node.renderer_id());
+    BrowserAccessibility* node, bool notify) {
+  if (focus_)
+    focus_->InternalReleaseReference(false);
+  focus_ = node;
+  if (focus_)
+    focus_->InternalAddReference();
+
+  if (notify && node && delegate_)
+    delegate_->SetAccessibilityFocus(node->renderer_id());
 }
 
 void BrowserAccessibilityManager::DoDefaultAction(
@@ -234,105 +257,91 @@ gfx::Rect BrowserAccessibilityManager::GetViewBounds() {
   return gfx::Rect();
 }
 
-bool BrowserAccessibilityManager::CanModifyTreeInPlace(
-    BrowserAccessibility* current_root,
-    const WebAccessibility& new_root) {
-  if (current_root->renderer_id() != new_root.id)
-    return false;
-  if (current_root->GetChildCount() != new_root.children.size())
-    return false;
-  for (unsigned int i = 0; i < current_root->GetChildCount(); i++) {
-    if (!CanModifyTreeInPlace(current_root->GetChild(i),
-                              new_root.children[i])) {
-      return false;
-    }
-  }
-  return true;
-}
-
-void BrowserAccessibilityManager::ModifyTreeInPlace(
-      BrowserAccessibility* current_root,
-      const WebAccessibility& new_root) {
-  DCHECK_EQ(current_root->renderer_id(), new_root.id);
-  DCHECK_EQ(current_root->GetChildCount(), new_root.children.size());
-  for (unsigned int i = 0; i < current_root->GetChildCount(); i++)
-    ModifyTreeInPlace(current_root->GetChild(i), new_root.children[i]);
-  current_root->Initialize(
-      this,
-      current_root->GetParent(),
-      current_root->child_id(),
-      current_root->index_in_parent(),
-      new_root);
-}
-
 BrowserAccessibility* BrowserAccessibilityManager::UpdateNode(
-    const WebAccessibility& acc_obj,
+    const WebAccessibility& src,
     bool include_children) {
   base::hash_map<int32, int32>::iterator iter =
-      renderer_id_to_child_id_map_.find(acc_obj.id);
+      renderer_id_to_child_id_map_.find(src.id);
   if (iter == renderer_id_to_child_id_map_.end())
     return NULL;
 
   int32 child_id = iter->second;
-  BrowserAccessibility* old_browser_acc = GetFromChildID(child_id);
-  if (!old_browser_acc)
+  BrowserAccessibility* current = GetFromChildID(child_id);
+  if (!current)
     return NULL;
 
   if (!include_children) {
-    DCHECK_EQ(0U, acc_obj.children.size());
-    old_browser_acc->Initialize(
+    DCHECK_EQ(0U, src.children.size());
+    current->Initialize(
         this,
-        old_browser_acc->GetParent(),
-        old_browser_acc->child_id(),
-        old_browser_acc->index_in_parent(),
-        acc_obj);
-    return old_browser_acc;
+        current->GetParent(),
+        current->child_id(),
+        current->index_in_parent(),
+        src);
+    return current;
   }
 
-  if (CanModifyTreeInPlace(old_browser_acc, acc_obj)) {
-    ModifyTreeInPlace(old_browser_acc, acc_obj);
-    return old_browser_acc;
-  }
+  // Detach all of the nodes in the old tree and get a single flat vector
+  // of all node pointers.
+  std::vector<BrowserAccessibility*> old_tree_nodes;
+  current->DetachTree(&old_tree_nodes);
 
-  BrowserAccessibility* new_browser_acc = CreateAccessibilityTree(
-      old_browser_acc->GetParent(),
-      child_id,
-      acc_obj,
-      old_browser_acc->index_in_parent());
+  // Build a new tree, reusing old nodes if possible. Each node that's
+  // reused will have its reference count incremented by one.
+  current = CreateAccessibilityTree(NULL, src, -1);
 
-  if (old_browser_acc->GetParent()) {
-    old_browser_acc->GetParent()->ReplaceChild(
-        old_browser_acc,
-        new_browser_acc);
-  } else {
-    DCHECK_EQ(old_browser_acc, root_);
-    root_ = new_browser_acc;
-  }
-  if (focus_ && focus_->IsDescendantOf(old_browser_acc))
-    focus_ = root_;
+  // Decrement the reference count of all nodes in the old tree, which will
+  // delete any nodes no longer needed.
+  for (int i = 0; i < static_cast<int>(old_tree_nodes.size()); i++)
+    old_tree_nodes[i]->InternalReleaseReference(false);
 
-  old_browser_acc->ReleaseTree();
-  old_browser_acc->ReleaseReference();
-  child_id_map_[child_id] = new_browser_acc;
+  DCHECK(focus_);
+  if (!focus_->instance_active())
+    SetFocus(root_, false);
 
-  return new_browser_acc;
+  return current;
 }
 
 BrowserAccessibility* BrowserAccessibilityManager::CreateAccessibilityTree(
     BrowserAccessibility* parent,
-    int child_id,
     const WebAccessibility& src,
     int index_in_parent) {
-  BrowserAccessibility* instance = factory_->Create();
+  BrowserAccessibility* instance = NULL;
+  int32 child_id = 0;
+  base::hash_map<int32, int32>::iterator iter =
+      renderer_id_to_child_id_map_.find(src.id);
+
+  // If a BrowserAccessibility instance for this ID already exists, add a
+  // new reference to it and retrieve its children vector.
+  if (iter != renderer_id_to_child_id_map_.end()) {
+    child_id = iter->second;
+    instance = GetFromChildID(child_id);
+  }
+
+  // If the node has changed roles, don't reuse a BrowserAccessibility
+  // object, that could confuse a screen reader.
+  if (instance && instance->role() != src.role)
+    instance = NULL;
+
+  if (instance) {
+    // If we're reusing a node, update its parent and increment its
+    // reference count.
+    instance->UpdateParent(parent, index_in_parent);
+    instance->InternalAddReference();
+  } else {
+    // Otherwise, create a new instance.
+    instance = factory_->Create();
+    child_id = GetNextChildID();
+  }
 
   instance->Initialize(this, parent, child_id, index_in_parent, src);
   child_id_map_[child_id] = instance;
   renderer_id_to_child_id_map_[src.id] = child_id;
   if ((src.state >> WebAccessibility::STATE_FOCUSED) & 1)
-    focus_ = instance;
+    SetFocus(instance, false);
   for (int i = 0; i < static_cast<int>(src.children.size()); ++i) {
     BrowserAccessibility* child = CreateAccessibilityTree(
-        instance, GetNextChildID(), src.children[i], i);
+        instance, src.children[i], i);
     instance->AddChild(child);
   }
 
diff --git a/chrome/browser/accessibility/browser_accessibility_manager.h b/chrome/browser/accessibility/browser_accessibility_manager.h
index f9ce989..14d1649 100644
--- a/chrome/browser/accessibility/browser_accessibility_manager.h
+++ b/chrome/browser/accessibility/browser_accessibility_manager.h
@@ -60,7 +60,7 @@ class BrowserAccessibilityManager {
   // header here.
   virtual void NotifyAccessibilityEvent(
       int type,
-      BrowserAccessibility* node) = 0;
+      BrowserAccessibility* node) { }
 
   // Returns the next unique child id.
   static int32 GetNextChildID();
@@ -79,8 +79,10 @@ class BrowserAccessibilityManager {
   // view got focused.
   void GotFocus();
 
-  // Tell the renderer to set focus to this node.
-  void SetFocus(const BrowserAccessibility& node);
+  // Update the focused node to |node|, which may be null.
+  // If |notify| is true, send a message to the renderer to set focus
+  // to this node.
+  void SetFocus(BrowserAccessibility* node, bool notify);
 
   // Tell the renderer to do the default action for this node.
   void DoDefaultAction(const BrowserAccessibility& node);
@@ -91,7 +93,7 @@ class BrowserAccessibilityManager {
   // Called when the renderer process has notified us of about tree changes.
   // Send a notification to MSAA clients of the change.
   void OnAccessibilityNotifications(
-    const std::vector<ViewHostMsg_AccessibilityNotification_Params>& params);
+      const std::vector<ViewHostMsg_AccessibilityNotification_Params>& params);
 
   gfx::NativeView GetParentView();
 
@@ -124,33 +126,19 @@ class BrowserAccessibilityManager {
   void OnAccessibilityObjectTextChange(
       const WebAccessibility& acc_obj);
 
-  // Recursively compare the IDs of our subtree to a new subtree received
-  // from the renderer and return true if their IDs match exactly.
-  bool CanModifyTreeInPlace(
-      BrowserAccessibility* current_root,
-      const WebAccessibility& new_root);
-
-  // Recursively modify a subtree (by reinitializing) to match a new
-  // subtree received from the renderer process. Should only be called
-  // if CanModifyTreeInPlace returned true.
-  void ModifyTreeInPlace(
-      BrowserAccessibility* current_root,
-      const WebAccessibility& new_root);
-
   // Update an accessibility node with an updated WebAccessibility node
   // received from the renderer process. When |include_children| is true
   // the node's children will also be updated, otherwise only the node
   // itself is updated. Returns the updated node or NULL if no node was
   // updated.
   BrowserAccessibility* UpdateNode(
-      const WebAccessibility& acc_obj,
+      const WebAccessibility& src,
       bool include_children);
 
   // Recursively build a tree of BrowserAccessibility objects from
   // the WebAccessibility tree received from the renderer process.
   BrowserAccessibility* CreateAccessibilityTree(
       BrowserAccessibility* parent,
-      int child_id,
       const WebAccessibility& src,
       int index_in_parent);
 
diff --git a/chrome/browser/accessibility/browser_accessibility_manager_mac.mm b/chrome/browser/accessibility/browser_accessibility_manager_mac.mm
index 03a08dd..3aebc6e 100644
--- a/chrome/browser/accessibility/browser_accessibility_manager_mac.mm
+++ b/chrome/browser/accessibility/browser_accessibility_manager_mac.mm
@@ -18,10 +18,6 @@ BrowserAccessibilityManager* BrowserAccessibilityManager::Create(
       parent_view, src, delegate, factory);
 }
 
-BrowserAccessibility* BrowserAccessibilityFactory::Create() {
-  return BrowserAccessibility::Create();
-}
-
 BrowserAccessibilityManagerMac::BrowserAccessibilityManagerMac(
     gfx::NativeView parent_window,
     const webkit_glue::WebAccessibility& src,
diff --git a/chrome/browser/accessibility/browser_accessibility_manager_unittest.cc b/chrome/browser/accessibility/browser_accessibility_manager_unittest.cc
new file mode 100644
index 0000000..0ac6e5e
--- /dev/null
+++ b/chrome/browser/accessibility/browser_accessibility_manager_unittest.cc
@@ -0,0 +1,549 @@
+// Copyright (c) 2010 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 "base/string16.h"
+#include "base/utf_string_conversions.h"
+#include "chrome/browser/accessibility/browser_accessibility.h"
+#include "chrome/browser/accessibility/browser_accessibility_manager.h"
+#include "chrome/common/render_messages_params.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/glue/webaccessibility.h"
+
+using webkit_glue::WebAccessibility;
+
+namespace {
+
+// Subclass of BrowserAccessibility that counts the number of instances.
+class CountedBrowserAccessibility : public BrowserAccessibility {
+ public:
+  CountedBrowserAccessibility() {
+    global_obj_count_++;
+    native_ref_count_ = 1;
+  }
+  virtual ~CountedBrowserAccessibility() {
+    global_obj_count_--;
+  }
+
+  virtual void NativeAddReference() OVERRIDE {
+    native_ref_count_++;
+  }
+
+  virtual void NativeReleaseReference() OVERRIDE {
+    native_ref_count_--;
+    if (native_ref_count_ == 0)
+      delete this;
+  }
+
+  int native_ref_count_;
+  static int global_obj_count_;
+};
+
+int CountedBrowserAccessibility::global_obj_count_ = 0;
+
+// Factory that creates a CountedBrowserAccessibility.
+class CountedBrowserAccessibilityFactory
+    : public BrowserAccessibilityFactory {
+ public:
+  virtual ~CountedBrowserAccessibilityFactory() {}
+  virtual BrowserAccessibility* Create() {
+    return new CountedBrowserAccessibility();
+  }
+};
+
+}  // anonymous namespace
+
+TEST(BrowserAccessibilityManagerTest, TestNoLeaks) {
+  // Create WebAccessibility objects for a simple document tree,
+  // representing the accessibility information used to initialize
+  // BrowserAccessibilityManager.
+  WebAccessibility button;
+  button.id = 2;
+  button.name = UTF8ToUTF16("Button");
+  button.role = WebAccessibility::ROLE_BUTTON;
+  button.state = 0;
+
+  WebAccessibility checkbox;
+  checkbox.id = 3;
+  checkbox.name = UTF8ToUTF16("Checkbox");
+  checkbox.role = WebAccessibility::ROLE_CHECKBOX;
+  checkbox.state = 0;
+
+  WebAccessibility root;
+  root.id = 1;
+  root.name = UTF8ToUTF16("Document");
+  root.role = WebAccessibility::ROLE_DOCUMENT;
+  root.state = 0;
+  root.children.push_back(button);
+  root.children.push_back(checkbox);
+
+  // Construct a BrowserAccessibilityManager with this WebAccessibility tree
+  // and a factory for an instance-counting BrowserAccessibility, and ensure
+  // that exactly 3 instances were created. Note that the manager takes
+  // ownership of the factory.
+  CountedBrowserAccessibility::global_obj_count_ = 0;
+  BrowserAccessibilityManager* manager =
+      BrowserAccessibilityManager::Create(
+          NULL,
+          root,
+          NULL,
+          new CountedBrowserAccessibilityFactory());
+
+  ASSERT_EQ(3, CountedBrowserAccessibility::global_obj_count_);
+
+  // Delete the manager and test that all 3 instances are deleted.
+  delete manager;
+  ASSERT_EQ(0, CountedBrowserAccessibility::global_obj_count_);
+
+  // Construct a manager again, and this time save references to two of
+  // the three nodes in the tree.
+  manager =
+      BrowserAccessibilityManager::Create(
+          NULL,
+          root,
+          NULL,
+          new CountedBrowserAccessibilityFactory());
+  ASSERT_EQ(3, CountedBrowserAccessibility::global_obj_count_);
+
+  CountedBrowserAccessibility* root_accessible =
+      static_cast<CountedBrowserAccessibility*>(manager->GetRoot());
+  root_accessible->NativeAddReference();
+  CountedBrowserAccessibility* child1_accessible =
+      static_cast<CountedBrowserAccessibility*>(root_accessible->GetChild(1));
+  child1_accessible->NativeAddReference();
+
+  // Now delete the manager, and only one of the three nodes in the tree
+  // should be released.
+  delete manager;
+  ASSERT_EQ(2, CountedBrowserAccessibility::global_obj_count_);
+
+  // Release each of our references and make sure that each one results in
+  // the instance being deleted as its reference count hits zero.
+  root_accessible->NativeReleaseReference();
+  ASSERT_EQ(1, CountedBrowserAccessibility::global_obj_count_);
+  child1_accessible->NativeReleaseReference();
+  ASSERT_EQ(0, CountedBrowserAccessibility::global_obj_count_);
+}
+
+TEST(BrowserAccessibilityManagerTest, TestReuseBrowserAccessibilityObjects) {
+  // Make sure that changes to a subtree reuse as many objects as possible.
+
+  // Tree 1:
+  //
+  // root
+  //   child1
+  //   child2
+  //   child3
+
+  WebAccessibility tree1_child1;
+  tree1_child1.id = 2;
+  tree1_child1.name = UTF8ToUTF16("Child1");
+  tree1_child1.role = WebAccessibility::ROLE_BUTTON;
+  tree1_child1.state = 0;
+
+  WebAccessibility tree1_child2;
+  tree1_child2.id = 3;
+  tree1_child2.name = UTF8ToUTF16("Child2");
+  tree1_child2.role = WebAccessibility::ROLE_BUTTON;
+  tree1_child2.state = 0;
+
+  WebAccessibility tree1_child3;
+  tree1_child3.id = 4;
+  tree1_child3.name = UTF8ToUTF16("Child3");
+  tree1_child3.role = WebAccessibility::ROLE_BUTTON;
+  tree1_child3.state = 0;
+
+  WebAccessibility tree1_root;
+  tree1_root.id = 1;
+  tree1_root.name = UTF8ToUTF16("Document");
+  tree1_root.role = WebAccessibility::ROLE_DOCUMENT;
+  tree1_root.state = 0;
+  tree1_root.children.push_back(tree1_child1);
+  tree1_root.children.push_back(tree1_child2);
+  tree1_root.children.push_back(tree1_child3);
+
+  // Tree 2:
+  //
+  // root
+  //   child0  <-- inserted
+  //   child1
+  //   child2
+  //           <-- child3 deleted
+
+  WebAccessibility tree2_child0;
+  tree2_child0.id = 5;
+  tree2_child0.name = UTF8ToUTF16("Child0");
+  tree2_child0.role = WebAccessibility::ROLE_BUTTON;
+  tree2_child0.state = 0;
+
+  WebAccessibility tree2_child1;
+  tree2_child1.id = 2;
+  tree2_child1.name = UTF8ToUTF16("Child1");
+  tree2_child1.role = WebAccessibility::ROLE_BUTTON;
+  tree2_child1.state = 0;
+
+  WebAccessibility tree2_child2;
+  tree2_child2.id = 3;
+  tree2_child2.name = UTF8ToUTF16("Child2");
+  tree2_child2.role = WebAccessibility::ROLE_BUTTON;
+  tree2_child2.state = 0;
+
+  WebAccessibility tree2_root;
+  tree2_root.id = 1;
+  tree2_root.name = UTF8ToUTF16("DocumentChanged");
+  tree2_root.role = WebAccessibility::ROLE_DOCUMENT;
+  tree2_root.state = 0;
+  tree2_root.children.push_back(tree2_child0);
+  tree2_root.children.push_back(tree2_child1);
+  tree2_root.children.push_back(tree2_child2);
+
+  // Construct a BrowserAccessibilityManager with tree1.
+  CountedBrowserAccessibility::global_obj_count_ = 0;
+  BrowserAccessibilityManager* manager =
+      BrowserAccessibilityManager::Create(
+          NULL,
+          tree1_root,
+          NULL,
+          new CountedBrowserAccessibilityFactory());
+  ASSERT_EQ(4, CountedBrowserAccessibility::global_obj_count_);
+
+  // Save references to all of the objects.
+  CountedBrowserAccessibility* root_accessible =
+      static_cast<CountedBrowserAccessibility*>(manager->GetRoot());
+  root_accessible->NativeAddReference();
+  CountedBrowserAccessibility* child1_accessible =
+      static_cast<CountedBrowserAccessibility*>(root_accessible->GetChild(0));
+  child1_accessible->NativeAddReference();
+  CountedBrowserAccessibility* child2_accessible =
+      static_cast<CountedBrowserAccessibility*>(root_accessible->GetChild(1));
+  child2_accessible->NativeAddReference();
+  CountedBrowserAccessibility* child3_accessible =
+      static_cast<CountedBrowserAccessibility*>(root_accessible->GetChild(2));
+  child3_accessible->NativeAddReference();
+
+  // Check the index in parent.
+  EXPECT_EQ(0, child1_accessible->index_in_parent());
+  EXPECT_EQ(1, child2_accessible->index_in_parent());
+  EXPECT_EQ(2, child3_accessible->index_in_parent());
+
+  // Process a notification containing the changed subtree.
+  std::vector<ViewHostMsg_AccessibilityNotification_Params> params;
+  params.push_back(ViewHostMsg_AccessibilityNotification_Params());
+  ViewHostMsg_AccessibilityNotification_Params* msg = &params[0];
+  msg->notification_type = ViewHostMsg_AccessibilityNotification_Params::
+      NOTIFICATION_TYPE_CHILDREN_CHANGED;
+  msg->acc_obj = tree2_root;
+  manager->OnAccessibilityNotifications(params);
+
+  // There should be 5 objects now: the 4 from the new tree, plus the
+  // reference to child3 we kept.
+  EXPECT_EQ(5, CountedBrowserAccessibility::global_obj_count_);
+
+  // Check that our references to the root, child1, and child2 are still valid,
+  // but that the reference to child3 is now invalid.
+  EXPECT_TRUE(root_accessible->instance_active());
+  EXPECT_TRUE(child1_accessible->instance_active());
+  EXPECT_TRUE(child2_accessible->instance_active());
+  EXPECT_FALSE(child3_accessible->instance_active());
+
+  // Check that the index in parent has been updated.
+  EXPECT_EQ(1, child1_accessible->index_in_parent());
+  EXPECT_EQ(2, child2_accessible->index_in_parent());
+
+  // Release our references. The object count should only decrease by 1
+  // for child3.
+  root_accessible->NativeReleaseReference();
+  child1_accessible->NativeReleaseReference();
+  child2_accessible->NativeReleaseReference();
+  child3_accessible->NativeReleaseReference();
+
+  EXPECT_EQ(4, CountedBrowserAccessibility::global_obj_count_);
+
+  // Delete the manager and make sure all memory is cleaned up.
+  delete manager;
+  ASSERT_EQ(0, CountedBrowserAccessibility::global_obj_count_);
+}
+
+TEST(BrowserAccessibilityManagerTest, TestReuseBrowserAccessibilityObjects2) {
+  // Similar to the test above, but with a more complicated tree.
+
+  // Tree 1:
+  //
+  // root
+  //   container
+  //     child1
+  //       grandchild1
+  //     child2
+  //       grandchild2
+  //     child3
+  //       grandchild3
+
+  WebAccessibility tree1_grandchild1;
+  tree1_grandchild1.id = 4;
+  tree1_grandchild1.name = UTF8ToUTF16("GrandChild1");
+  tree1_grandchild1.role = WebAccessibility::ROLE_BUTTON;
+  tree1_grandchild1.state = 0;
+
+  WebAccessibility tree1_child1;
+  tree1_child1.id = 3;
+  tree1_child1.name = UTF8ToUTF16("Child1");
+  tree1_child1.role = WebAccessibility::ROLE_BUTTON;
+  tree1_child1.state = 0;
+  tree1_child1.children.push_back(tree1_grandchild1);
+
+  WebAccessibility tree1_grandchild2;
+  tree1_grandchild2.id = 6;
+  tree1_grandchild2.name = UTF8ToUTF16("GrandChild1");
+  tree1_grandchild2.role = WebAccessibility::ROLE_BUTTON;
+  tree1_grandchild2.state = 0;
+
+  WebAccessibility tree1_child2;
+  tree1_child2.id = 5;
+  tree1_child2.name = UTF8ToUTF16("Child2");
+  tree1_child2.role = WebAccessibility::ROLE_BUTTON;
+  tree1_child2.state = 0;
+  tree1_child2.children.push_back(tree1_grandchild2);
+
+  WebAccessibility tree1_grandchild3;
+  tree1_grandchild3.id = 8;
+  tree1_grandchild3.name = UTF8ToUTF16("GrandChild3");
+  tree1_grandchild3.role = WebAccessibility::ROLE_BUTTON;
+  tree1_grandchild3.state = 0;
+
+  WebAccessibility tree1_child3;
+  tree1_child3.id = 7;
+  tree1_child3.name = UTF8ToUTF16("Child3");
+  tree1_child3.role = WebAccessibility::ROLE_BUTTON;
+  tree1_child3.state = 0;
+  tree1_child3.children.push_back(tree1_grandchild3);
+
+  WebAccessibility tree1_container;
+  tree1_container.id = 2;
+  tree1_container.name = UTF8ToUTF16("Container");
+  tree1_container.role = WebAccessibility::ROLE_GROUP;
+  tree1_container.state = 0;
+  tree1_container.children.push_back(tree1_child1);
+  tree1_container.children.push_back(tree1_child2);
+  tree1_container.children.push_back(tree1_child3);
+
+  WebAccessibility tree1_root;
+  tree1_root.id = 1;
+  tree1_root.name = UTF8ToUTF16("Document");
+  tree1_root.role = WebAccessibility::ROLE_DOCUMENT;
+  tree1_root.state = 0;
+  tree1_root.children.push_back(tree1_container);
+
+  // Tree 2:
+  //
+  // root
+  //   container
+  //     child0         <-- inserted
+  //       grandchild0  <--
+  //     child1
+  //       grandchild1
+  //     child2
+  //       grandchild2
+  //                    <-- child3 (and grandchild3) deleted
+
+  WebAccessibility tree2_grandchild0;
+  tree2_grandchild0.id = 9;
+  tree2_grandchild0.name = UTF8ToUTF16("GrandChild0");
+  tree2_grandchild0.role = WebAccessibility::ROLE_BUTTON;
+  tree2_grandchild0.state = 0;
+
+  WebAccessibility tree2_child0;
+  tree2_child0.id = 10;
+  tree2_child0.name = UTF8ToUTF16("Child0");
+  tree2_child0.role = WebAccessibility::ROLE_BUTTON;
+  tree2_child0.state = 0;
+  tree2_child0.children.push_back(tree2_grandchild0);
+
+  WebAccessibility tree2_grandchild1;
+  tree2_grandchild1.id = 4;
+  tree2_grandchild1.name = UTF8ToUTF16("GrandChild1");
+  tree2_grandchild1.role = WebAccessibility::ROLE_BUTTON;
+  tree2_grandchild1.state = 0;
+
+  WebAccessibility tree2_child1;
+  tree2_child1.id = 3;
+  tree2_child1.name = UTF8ToUTF16("Child1");
+  tree2_child1.role = WebAccessibility::ROLE_BUTTON;
+  tree2_child1.state = 0;
+  tree2_child1.children.push_back(tree2_grandchild1);
+
+  WebAccessibility tree2_grandchild2;
+  tree2_grandchild2.id = 6;
+  tree2_grandchild2.name = UTF8ToUTF16("GrandChild1");
+  tree2_grandchild2.role = WebAccessibility::ROLE_BUTTON;
+  tree2_grandchild2.state = 0;
+
+  WebAccessibility tree2_child2;
+  tree2_child2.id = 5;
+  tree2_child2.name = UTF8ToUTF16("Child2");
+  tree2_child2.role = WebAccessibility::ROLE_BUTTON;
+  tree2_child2.state = 0;
+  tree2_child2.children.push_back(tree2_grandchild2);
+
+  WebAccessibility tree2_container;
+  tree2_container.id = 2;
+  tree2_container.name = UTF8ToUTF16("Container");
+  tree2_container.role = WebAccessibility::ROLE_GROUP;
+  tree2_container.state = 0;
+  tree2_container.children.push_back(tree2_child0);
+  tree2_container.children.push_back(tree2_child1);
+  tree2_container.children.push_back(tree2_child2);
+
+  WebAccessibility tree2_root;
+  tree2_root.id = 1;
+  tree2_root.name = UTF8ToUTF16("Document");
+  tree2_root.role = WebAccessibility::ROLE_DOCUMENT;
+  tree2_root.state = 0;
+  tree2_root.children.push_back(tree2_container);
+
+  // Construct a BrowserAccessibilityManager with tree1.
+  CountedBrowserAccessibility::global_obj_count_ = 0;
+  BrowserAccessibilityManager* manager =
+      BrowserAccessibilityManager::Create(
+          NULL,
+          tree1_root,
+          NULL,
+          new CountedBrowserAccessibilityFactory());
+  ASSERT_EQ(8, CountedBrowserAccessibility::global_obj_count_);
+
+  // Save references to some objects.
+  CountedBrowserAccessibility* root_accessible =
+      static_cast<CountedBrowserAccessibility*>(manager->GetRoot());
+  root_accessible->NativeAddReference();
+  CountedBrowserAccessibility* container_accessible =
+      static_cast<CountedBrowserAccessibility*>(root_accessible->GetChild(0));
+  container_accessible->NativeAddReference();
+  CountedBrowserAccessibility* child2_accessible =
+      static_cast<CountedBrowserAccessibility*>(
+          container_accessible->GetChild(1));
+  child2_accessible->NativeAddReference();
+  CountedBrowserAccessibility* child3_accessible =
+      static_cast<CountedBrowserAccessibility*>(
+          container_accessible->GetChild(2));
+  child3_accessible->NativeAddReference();
+
+  // Check the index in parent.
+  EXPECT_EQ(1, child2_accessible->index_in_parent());
+  EXPECT_EQ(2, child3_accessible->index_in_parent());
+
+  // Process a notification containing the changed subtree rooted at
+  // the container.
+  std::vector<ViewHostMsg_AccessibilityNotification_Params> params;
+  params.push_back(ViewHostMsg_AccessibilityNotification_Params());
+  ViewHostMsg_AccessibilityNotification_Params* msg = &params[0];
+  msg->notification_type = ViewHostMsg_AccessibilityNotification_Params::
+      NOTIFICATION_TYPE_CHILDREN_CHANGED;
+  msg->acc_obj = tree2_container;
+  manager->OnAccessibilityNotifications(params);
+
+  // There should be 9 objects now: the 8 from the new tree, plus the
+  // reference to child3 we kept.
+  EXPECT_EQ(9, CountedBrowserAccessibility::global_obj_count_);
+
+  // Check that our references to the root and container and child2 are
+  // still valid, but that the reference to child3 is now invalid.
+  EXPECT_TRUE(root_accessible->instance_active());
+  EXPECT_TRUE(container_accessible->instance_active());
+  EXPECT_TRUE(child2_accessible->instance_active());
+  EXPECT_FALSE(child3_accessible->instance_active());
+
+  // Check that the index in parent has been updated.
+  EXPECT_EQ(2, child2_accessible->index_in_parent());
+
+  // Release our references. The object count should only decrease by 1
+  // for child3.
+  root_accessible->NativeReleaseReference();
+  container_accessible->NativeReleaseReference();
+  child2_accessible->NativeReleaseReference();
+  child3_accessible->NativeReleaseReference();
+
+  EXPECT_EQ(8, CountedBrowserAccessibility::global_obj_count_);
+
+  // Delete the manager and make sure all memory is cleaned up.
+  delete manager;
+  ASSERT_EQ(0, CountedBrowserAccessibility::global_obj_count_);
+}
+
+TEST(BrowserAccessibilityManagerTest, TestMoveChildUp) {
+  // Tree 1:
+  //
+  // 1
+  //   2
+  //   3
+  //     4
+
+  WebAccessibility tree1_4;
+  tree1_4.id = 4;
+  tree1_4.state = 0;
+
+  WebAccessibility tree1_3;
+  tree1_3.id = 3;
+  tree1_3.state = 0;
+  tree1_3.children.push_back(tree1_4);
+
+  WebAccessibility tree1_2;
+  tree1_2.id = 2;
+  tree1_2.state = 0;
+
+  WebAccessibility tree1_1;
+  tree1_1.id = 1;
+  tree1_1.state = 0;
+  tree1_1.children.push_back(tree1_2);
+  tree1_1.children.push_back(tree1_3);
+
+  // Tree 2:
+  //
+  // 1
+  //   4    <-- moves up a level and gains child
+  //     6  <-- new
+  //   5    <-- new
+
+  WebAccessibility tree2_6;
+  tree2_6.id = 6;
+  tree2_6.state = 0;
+
+  WebAccessibility tree2_5;
+  tree2_5.id = 5;
+  tree2_5.state = 0;
+
+  WebAccessibility tree2_4;
+  tree2_4.id = 4;
+  tree2_4.state = 0;
+  tree2_4.children.push_back(tree2_6);
+
+  WebAccessibility tree2_1;
+  tree2_1.id = 1;
+  tree2_1.state = 0;
+  tree2_1.children.push_back(tree2_4);
+  tree2_1.children.push_back(tree2_5);
+
+  // Construct a BrowserAccessibilityManager with tree1.
+  CountedBrowserAccessibility::global_obj_count_ = 0;
+  BrowserAccessibilityManager* manager =
+      BrowserAccessibilityManager::Create(
+          NULL,
+          tree1_1,
+          NULL,
+          new CountedBrowserAccessibilityFactory());
+  ASSERT_EQ(4, CountedBrowserAccessibility::global_obj_count_);
+
+  // Process a notification containing the changed subtree.
+  std::vector<ViewHostMsg_AccessibilityNotification_Params> params;
+  params.push_back(ViewHostMsg_AccessibilityNotification_Params());
+  ViewHostMsg_AccessibilityNotification_Params* msg = &params[0];
+  msg->notification_type = ViewHostMsg_AccessibilityNotification_Params::
+      NOTIFICATION_TYPE_CHILDREN_CHANGED;
+  msg->acc_obj = tree2_1;
+  manager->OnAccessibilityNotifications(params);
+
+  // There should be 4 objects now.
+  EXPECT_EQ(4, CountedBrowserAccessibility::global_obj_count_);
+
+  // Delete the manager and make sure all memory is cleaned up.
+  delete manager;
+  ASSERT_EQ(0, CountedBrowserAccessibility::global_obj_count_);
+}
diff --git a/chrome/browser/accessibility/browser_accessibility_manager_win.cc b/chrome/browser/accessibility/browser_accessibility_manager_win.cc
index 5720dd3..bb3105e 100644
--- a/chrome/browser/accessibility/browser_accessibility_manager_win.cc
+++ b/chrome/browser/accessibility/browser_accessibility_manager_win.cc
@@ -22,12 +22,6 @@ BrowserAccessibilityManager* BrowserAccessibilityManager::Create(
       factory);
 }
 
-// Defining this here instead of in the base implementation file to address link
-// errors on the linux shared library build. (linux shlib)
-BrowserAccessibility* BrowserAccessibilityFactory::Create() {
-  return BrowserAccessibility::Create();
-}
-
 BrowserAccessibilityManagerWin*
 BrowserAccessibilityManager::toBrowserAccessibilityManagerWin() {
   return static_cast<BrowserAccessibilityManagerWin*>(this);
@@ -39,6 +33,12 @@ BrowserAccessibilityManagerWin::BrowserAccessibilityManagerWin(
     BrowserAccessibilityDelegate* delegate,
     BrowserAccessibilityFactory* factory)
     : BrowserAccessibilityManager(parent_view, src, delegate, factory) {
+  // Allow NULL parent_view for unit testing.
+  if (parent_view == NULL) {
+    window_iaccessible_ = NULL;
+    return;
+  }
+
   HRESULT hr = ::CreateStdAccessibleObject(
       parent_view, OBJID_WINDOW, IID_IAccessible,
       reinterpret_cast<void **>(&window_iaccessible_));
diff --git a/chrome/browser/accessibility/browser_accessibility_win.cc b/chrome/browser/accessibility/browser_accessibility_win.cc
index 050cea0..5b17979 100644
--- a/chrome/browser/accessibility/browser_accessibility_win.cc
+++ b/chrome/browser/accessibility/browser_accessibility_win.cc
@@ -34,15 +34,13 @@ BrowserAccessibilityWin* BrowserAccessibility::toBrowserAccessibilityWin() {
 }
 
 BrowserAccessibilityWin::BrowserAccessibilityWin()
-    : instance_active_(false),
-      ia_role_(0),
+    : ia_role_(0),
       ia_state_(0),
       ia2_role_(0),
       ia2_state_(0) {
 }
 
 BrowserAccessibilityWin::~BrowserAccessibilityWin() {
-  ReleaseTree();
 }
 
 //
@@ -391,7 +389,7 @@ STDMETHODIMP BrowserAccessibilityWin::accSelect(
     return E_FAIL;
 
   if (flags_sel & SELFLAG_TAKEFOCUS) {
-    manager_->SetFocus(*this);
+    manager_->SetFocus(this, true);
     return S_OK;
   }
 
@@ -1125,6 +1123,8 @@ HRESULT WINAPI BrowserAccessibilityWin::InternalQueryInterface(
 
 // Initialize this object and mark it as active.
 void BrowserAccessibilityWin::Initialize() {
+  BrowserAccessibility::Initialize();
+
   InitRoleAndState();
 
   // Expose headings levels to NVDA with the "level" object attribute.
@@ -1147,29 +1147,16 @@ void BrowserAccessibilityWin::Initialize() {
   // announce the name.
   if (name_.empty() && HasAttribute(WebAccessibility::ATTR_DESCRIPTION))
     GetAttribute(WebAccessibility::ATTR_DESCRIPTION, &name_);
-
-  instance_active_ = true;
 }
 
-// Mark this object as inactive, and remove references to all children.
-// When no other clients hold any references to this object it will be
-// deleted, and in the meantime, calls to any methods will return E_FAIL.
-void BrowserAccessibilityWin::ReleaseTree() {
-  if (!instance_active_)
-    return;
-
-  // Mark this object as inactive, so calls to all COM methods will return
-  // failure.
-  instance_active_ = false;
-
-  BrowserAccessibility::ReleaseTree();
+void BrowserAccessibilityWin::NativeAddReference() {
+  AddRef();
 }
 
-void BrowserAccessibilityWin::ReleaseReference() {
+void BrowserAccessibilityWin::NativeReleaseReference() {
   Release();
 }
 
-
 BrowserAccessibilityWin* BrowserAccessibilityWin::NewReference() {
   AddRef();
   return this;
diff --git a/chrome/browser/accessibility/browser_accessibility_win.h b/chrome/browser/accessibility/browser_accessibility_win.h
index 214f0c0..fab1f2a 100644
--- a/chrome/browser/accessibility/browser_accessibility_win.h
+++ b/chrome/browser/accessibility/browser_accessibility_win.h
@@ -64,8 +64,8 @@ class BrowserAccessibilityWin
   // BrowserAccessibility methods.
   //
   virtual void Initialize();
-  virtual void ReleaseTree();
-  virtual void ReleaseReference();
+  virtual void NativeAddReference();
+  virtual void NativeReleaseReference();
 
   //
   // IAccessible methods.
@@ -478,13 +478,6 @@ class BrowserAccessibilityWin
                     LONG start_offset,
                     LONG direction);
 
-  // COM objects are reference-counted. When we're done with this object
-  // and it's removed from our accessibility tree, a client may still be
-  // holding onto a pointer to this object, so we mark it as inactive
-  // so that calls to any of this object's methods immediately return
-  // failure.
-  bool instance_active_;
-
   // IAccessible role and state.
   int32 ia_role_;
   int32 ia_state_;
diff --git a/chrome/browser/accessibility/browser_accessibility_win_unittest.cc b/chrome/browser/accessibility/browser_accessibility_win_unittest.cc
index fe5fa04..7c8595d 100644
--- a/chrome/browser/accessibility/browser_accessibility_win_unittest.cc
+++ b/chrome/browser/accessibility/browser_accessibility_win_unittest.cc
@@ -11,6 +11,8 @@
 
 using webkit_glue::WebAccessibility;
 
+namespace {
+
 // Subclass of BrowserAccessibilityWin that counts the number of instances.
 class CountedBrowserAccessibility : public BrowserAccessibilityWin {
  public:
@@ -36,6 +38,8 @@ class CountedBrowserAccessibilityFactory
   }
 };
 
+}  // anonymous namespace
+
 VARIANT CreateI4Variant(LONG value) {
   VARIANT variant = {0};
 
diff --git a/chrome/browser/accessibility/renderer_accessibility_browsertest.cc b/chrome/browser/accessibility/renderer_accessibility_browsertest.cc
index 1e8697f..c065263 100644
--- a/chrome/browser/accessibility/renderer_accessibility_browsertest.cc
+++ b/chrome/browser/accessibility/renderer_accessibility_browsertest.cc
@@ -201,4 +201,40 @@ IN_PROC_BROWSER_TEST_F(RendererAccessibilityBrowserTest,
   EXPECT_STREQ("Hello, world.", UTF16ToUTF8(text.value).c_str());
 }
 
+IN_PROC_BROWSER_TEST_F(RendererAccessibilityBrowserTest,
+                       CrossPlatformMultipleInheritanceAccessibility) {
+  // In a WebKit accessibility render tree for a table, each cell is a
+  // child of both a row and a column, so it appears to use multiple
+  // inheritance. Make sure that the WebAccessibilityObject tree only
+  // keeps one copy of each cell, and uses an indirect child id for the
+  // additional reference to it.
+  const char url_str[] =
+      "data:text/html,"
+      "<!doctype html>"
+      "<table border=1><tr><td>1</td><td>2</td></tr></table>";
+  GURL url(url_str);
+  browser()->OpenURL(url, GURL(), CURRENT_TAB, PageTransition::TYPED);
+
+  const WebAccessibility& tree = GetWebAccessibilityTree();
+  ASSERT_EQ(1U, tree.children.size());
+  const WebAccessibility& table = tree.children[0];
+  EXPECT_EQ(WebAccessibility::ROLE_TABLE, table.role);
+  const WebAccessibility& row = table.children[0];
+  EXPECT_EQ(WebAccessibility::ROLE_ROW, row.role);
+  const WebAccessibility& cell1 = row.children[0];
+  EXPECT_EQ(WebAccessibility::ROLE_CELL, cell1.role);
+  const WebAccessibility& cell2 = row.children[1];
+  EXPECT_EQ(WebAccessibility::ROLE_CELL, cell2.role);
+  const WebAccessibility& column1 = table.children[1];
+  EXPECT_EQ(WebAccessibility::ROLE_COLUMN, column1.role);
+  EXPECT_EQ(0U, column1.children.size());
+  EXPECT_EQ(1U, column1.indirect_child_ids.size());
+  EXPECT_EQ(cell1.id, column1.indirect_child_ids[0]);
+  const WebAccessibility& column2 = table.children[2];
+  EXPECT_EQ(WebAccessibility::ROLE_COLUMN, column2.role);
+  EXPECT_EQ(0U, column2.children.size());
+  EXPECT_EQ(1U, column2.indirect_child_ids.size());
+  EXPECT_EQ(cell2.id, column2.indirect_child_ids[0]);
+}
+
 }  // namespace