Eliminates the dependencies between views and browser, and also does some other minor cleanups. The special casing for the OmniBox that used to live in the ViewAccessibility code could be removed due to the fact that the only assistive technologies depending on this case are now testing tools. The end-user tools will see no difference, and the testing tools can access the OmniBox IAccessible by directly giving it keyboard/mouse focus.

BUG=2237


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

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

6 files changed, 29 insertions, 443 deletions

diff --git a/chrome/views/SConscript b/chrome/views/SConscript
index 5512c6b..4221600 100644
--- a/chrome/views/SConscript
+++ b/chrome/views/SConscript
@@ -34,7 +34,6 @@ input_files = [
     'accelerator.cc',
     'accelerator_handler.cc',
     'accessibility/accessible_wrapper.cc',
-    'accessibility/autocomplete_accessibility.cc',
     'accessibility/view_accessibility.cc',
     'aero_tooltip_manager.cc',
     'background.cc',
diff --git a/chrome/views/accessibility/accessible_wrapper.cc b/chrome/views/accessibility/accessible_wrapper.cc
index bc7a792..1774556 100644
--- a/chrome/views/accessibility/accessible_wrapper.cc
+++ b/chrome/views/accessibility/accessible_wrapper.cc
@@ -4,7 +4,6 @@
 
 #include "chrome/views/accessibility/accessible_wrapper.h"
 
-#include "base/logging.h"
 #include "chrome/views/accessibility/view_accessibility.h"
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -13,9 +12,9 @@
 //
 ////////////////////////////////////////////////////////////////////////////////
 
-AccessibleWrapper::AccessibleWrapper(views::View* view) :
-    accessibility_info_(NULL),
-    view_(view) {
+AccessibleWrapper::AccessibleWrapper(views::View* view)
+    : accessibility_info_(NULL),
+      view_(view) {
 }
 
 STDMETHODIMP AccessibleWrapper::CreateDefaultInstance(REFIID iid) {
@@ -26,9 +25,8 @@ STDMETHODIMP AccessibleWrapper::CreateDefaultInstance(REFIID iid) {
       CComObject<ViewAccessibility>* instance = NULL;
 
       HRESULT hr = CComObject<ViewAccessibility>::CreateInstance(&instance);
-      DCHECK(SUCCEEDED(hr));
 
-      if (!instance)
+      if (!SUCCEEDED(hr) || !instance)
         return E_FAIL;
 
       CComPtr<IAccessible> accessibility_instance(instance);
diff --git a/chrome/views/accessibility/autocomplete_accessibility.cc b/chrome/views/accessibility/autocomplete_accessibility.cc
deleted file mode 100644
index 4318a4a..0000000
--- a/chrome/views/accessibility/autocomplete_accessibility.cc
+++ /dev/null
@@ -1,268 +0,0 @@
-// Copyright (c) 2006-2008 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 "chrome/views/accessibility/autocomplete_accessibility.h"
-
-#include "base/logging.h"
-#include "chrome/common/l10n_util.h"
-#include "chrome/views/accessibility/accessible_wrapper.h"
-
-#include "generated_resources.h"
-
-HRESULT AutocompleteAccessibility::Initialize(
-    const AutocompleteEditView* edit_box) {
-  if (edit_box == NULL) {
-    return E_INVALIDARG;
-  }
-
-  edit_box_ = edit_box;
-
-  // Create a default accessible object for this instance.
-  return CreateStdAccessibleObject(edit_box_->m_hWnd, OBJID_CLIENT,
-                                   IID_IAccessible, reinterpret_cast<void **>
-                                       (&default_accessibility_server_));
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accChildCount(LONG* child_count) {
-  if (!child_count) {
-    return E_INVALIDARG;
-  }
-
-  DCHECK(default_accessibility_server_);
-  return default_accessibility_server_->get_accChildCount(child_count);
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accChild(VARIANT var_child,
-                                                     IDispatch** disp_child) {
-  if (var_child.vt != VT_I4 || !disp_child) {
-    return E_INVALIDARG;
-  }
-
-  // If var_child is the parent, remain with the same IDispatch
-  if (var_child.lVal == CHILDID_SELF)
-    return S_OK;
-
-  *disp_child = NULL;
-  return S_FALSE;
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accParent(IDispatch** disp_parent) {
-  if (!disp_parent) {
-    return E_INVALIDARG;
-  }
-
-  if (edit_box_->parent_view() == NULL) {
-    *disp_parent = NULL;
-    return S_FALSE;
-  }
-
-  // Retrieve the IUnknown interface for the parent view, and assign the
-  // IDispatch returned.
-  if (edit_box_->parent_view()->GetAccessibleWrapper()->GetInstance(
-      IID_IAccessible, reinterpret_cast<void**>(disp_parent)) == S_OK) {
-    // Increment the reference count for the retrieved interface.
-    (*disp_parent)->AddRef();
-    return S_OK;
-  } else {
-    return E_NOINTERFACE;
-  }
-}
-
-STDMETHODIMP AutocompleteAccessibility::accNavigate(LONG nav_dir, VARIANT start,
-                                                    VARIANT* end) {
-  if (start.vt != VT_I4 || !end) {
-    return E_INVALIDARG;
-  }
-
-  DCHECK(default_accessibility_server_);
-  return default_accessibility_server_->accNavigate(nav_dir, start, end);
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accFocus(VARIANT* focus_child) {
-  if (!focus_child) {
-    return E_INVALIDARG;
-  }
-
-  DCHECK(default_accessibility_server_);
-  return default_accessibility_server_->get_accFocus(focus_child);
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accName(VARIANT var_id,
-                                                    BSTR* name) {
-  if (var_id.vt != VT_I4 || !name) {
-    return E_INVALIDARG;
-  }
-
-  std::wstring temp_name = l10n_util::GetString(IDS_ACCNAME_LOCATION);
-
-  if (!temp_name.empty()) {
-    // Return name retrieved.
-    *name = CComBSTR(temp_name.c_str()).Detach();
-  } else {
-    // If no name is found, return S_FALSE.
-    return S_FALSE;
-  }
-  DCHECK(*name);
-
-  return S_OK;
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accDescription(VARIANT var_id,
-                                                           BSTR* desc) {
-  if (var_id.vt != VT_I4 || !desc) {
-    return E_INVALIDARG;
-  }
-
-  return S_FALSE;
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accValue(VARIANT var_id,
-                                                     BSTR* value) {
-  if (var_id.vt != VT_I4 || !value) {
-    return E_INVALIDARG;
-  }
-
-  std::wstring temp_value;
-
-  if (var_id.lVal != CHILDID_SELF)
-   return E_INVALIDARG;
-
-  // Edit box has no children, only handle self.
-  temp_value = edit_box_->GetText();
-  if (temp_value.empty())
-   return S_FALSE;
-
-   // Return value retrieved.
-  *value = CComBSTR(temp_value.c_str()).Detach();
-
-  DCHECK(*value);
-
-  return S_OK;
-
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accState(VARIANT var_id,
-                                                     VARIANT* state) {
-  if (var_id.vt != VT_I4 || !state) {
-    return E_INVALIDARG;
-  }
-
-  DCHECK(default_accessibility_server_);
-  HRESULT hr = default_accessibility_server_->get_accState(var_id, state);
-
-  if (hr != S_OK)
-    return hr;
-
-  // Adding on state to convey the fact that there is a dropdown.
-  state->lVal |= STATE_SYSTEM_HASPOPUP;
-  return S_OK;
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accRole(VARIANT var_id,
-                                                    VARIANT* role) {
-  if (var_id.vt != VT_I4 || !role) {
-    return E_INVALIDARG;
-  }
-
-  role->vt = VT_I4;
-
-  // Need to override the default role, which is ROLE_SYSTEM_CLIENT.
-  if (var_id.lVal == CHILDID_SELF) {
-    role->lVal = ROLE_SYSTEM_TEXT;
-  } else {
-    return S_FALSE;
-  }
-
-  return S_OK;
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accDefaultAction(VARIANT var_id,
-                                                             BSTR* def_action) {
-  if (var_id.vt != VT_I4 || !def_action) {
-    return E_INVALIDARG;
-  }
-
-  return S_FALSE;
-}
-
-STDMETHODIMP AutocompleteAccessibility::accLocation(LONG* x_left, LONG* y_top,
-                                                    LONG* width, LONG* height,
-                                                    VARIANT var_id) {
-  if (var_id.vt != VT_I4 || !x_left || !y_top || !width || !height) {
-    return E_INVALIDARG;
-  }
-
-  DCHECK(default_accessibility_server_);
-  return default_accessibility_server_->accLocation(x_left, y_top, width,
-                                                    height, var_id);
-}
-
-STDMETHODIMP AutocompleteAccessibility::accHitTest(LONG x_left, LONG y_top,
-                                                   VARIANT* child) {
-  if (!child) {
-    return E_INVALIDARG;
-  }
-
-  DCHECK(default_accessibility_server_);
-  return default_accessibility_server_->accHitTest(x_left, y_top, child);
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accKeyboardShortcut(VARIANT var_id,
-                                                                BSTR* acc_key) {
-  if (var_id.vt != VT_I4 || !acc_key) {
-    return E_INVALIDARG;
-  }
-
-  return S_FALSE;
-}
-
-// IAccessible functions not supported.
-
-HRESULT AutocompleteAccessibility::accDoDefaultAction(VARIANT var_id) {
-  return DISP_E_MEMBERNOTFOUND;
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accSelection(VARIANT* selected) {
-  if (selected)
-    selected->vt = VT_EMPTY;
-  return DISP_E_MEMBERNOTFOUND;
-}
-
-STDMETHODIMP AutocompleteAccessibility::accSelect(LONG flagsSelect,
-                                                  VARIANT var_id) {
-  return DISP_E_MEMBERNOTFOUND;
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accHelp(VARIANT var_id,
-                                                    BSTR* help) {
-  if (help)
-    *help = NULL;
-  return DISP_E_MEMBERNOTFOUND;
-}
-
-STDMETHODIMP AutocompleteAccessibility::get_accHelpTopic(BSTR* help_file,
-                                                         VARIANT var_id,
-                                                         LONG* topic_id) {
-  if (help_file) {
-    *help_file = NULL;
-  }
-  if (topic_id) {
-    *topic_id = static_cast<LONG>(-1);
-  }
-  return DISP_E_MEMBERNOTFOUND;
-}
-
-STDMETHODIMP AutocompleteAccessibility::put_accName(VARIANT var_id,
-                                                    BSTR put_name) {
-  // Deprecated.
-  return DISP_E_MEMBERNOTFOUND;
-}
-
-STDMETHODIMP AutocompleteAccessibility::put_accValue(VARIANT var_id,
-                                                     BSTR put_val) {
-  // Deprecated.
-  return DISP_E_MEMBERNOTFOUND;
-}
-
-
diff --git a/chrome/views/accessibility/autocomplete_accessibility.h b/chrome/views/accessibility/autocomplete_accessibility.h
deleted file mode 100644
index 847e251..0000000
--- a/chrome/views/accessibility/autocomplete_accessibility.h
+++ /dev/null
@@ -1,115 +0,0 @@
-// Copyright (c) 2006-2008 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.
-
-#ifndef CHROME_BROWSER_ACCESSIBILITY_AUTOCOMPLETE_ACCESSIBILITY_H__
-#define CHROME_BROWSER_ACCESSIBILITY_AUTOCOMPLETE_ACCESSIBILITY_H__
-
-#include <atlbase.h>
-#include <atlcom.h>
-
-#include <oleacc.h>
-
-#include "chrome/browser/autocomplete/autocomplete_edit.h"
-#include "chrome/views/view.h"
-
-////////////////////////////////////////////////////////////////////////////////
-//
-// AutocompleteAccessibility
-//
-// Class implementing the MSAA IAccessible COM interface for
-// AutocompleteEditView, providing accessibility to be used by screen readers
-// and other assistive technology (AT).
-//
-////////////////////////////////////////////////////////////////////////////////
-class ATL_NO_VTABLE AutocompleteAccessibility
-  : public CComObjectRootEx<CComMultiThreadModel>,
-    public IDispatchImpl<IAccessible, &IID_IAccessible, &LIBID_Accessibility> {
- public:
-  BEGIN_COM_MAP(AutocompleteAccessibility)
-    COM_INTERFACE_ENTRY2(IDispatch, IAccessible)
-    COM_INTERFACE_ENTRY(IAccessible)
-  END_COM_MAP()
-
-  AutocompleteAccessibility() {}
-  ~AutocompleteAccessibility() {}
-
-  HRESULT Initialize(const AutocompleteEditView* edit_box);
-
-  // Supported IAccessible methods.
-
-  // Retrieves the number of accessible children.
-  STDMETHODIMP get_accChildCount(LONG* child_count);
-
-  // Retrieves an IDispatch interface pointer for the specified child.
-  STDMETHODIMP get_accChild(VARIANT var_child, IDispatch** disp_child);
-
-  // Retrieves the IDispatch interface of the object's parent.
-  STDMETHODIMP get_accParent(IDispatch** disp_parent);
-
-  // Traverses to another UI element and retrieves the object.
-  STDMETHODIMP accNavigate(LONG nav_dir, VARIANT start, VARIANT* end);
-
-  // Retrieves the object that has the keyboard focus.
-  STDMETHODIMP get_accFocus(VARIANT* focus_child);
-
-  // Retrieves the name of the specified object.
-  STDMETHODIMP get_accName(VARIANT var_id, BSTR* name);
-
-  // Retrieves the tooltip description.
-  STDMETHODIMP get_accDescription(VARIANT var_id, BSTR* desc);
-
-  // Returns the current value of the edit box.
-  STDMETHODIMP get_accValue(VARIANT var_id, BSTR* value);
-
-  // Retrieves the current state of the specified object.
-  STDMETHODIMP get_accState(VARIANT var_id, VARIANT* state);
-
-  // Retrieves information describing the role of the specified object.
-  STDMETHODIMP get_accRole(VARIANT var_id, VARIANT* role);
-
-  // Retrieves a string that describes the object's default action.
-  STDMETHODIMP get_accDefaultAction(VARIANT var_id, BSTR* default_action);
-
-  // Retrieves the specified object's current screen location.
-  STDMETHODIMP accLocation(LONG* x_left, LONG* y_top, LONG* width, LONG* height,
-                           VARIANT var_id);
-
-  // Retrieves the child element or child object at a given point on the screen.
-  STDMETHODIMP accHitTest(LONG x_left, LONG y_top, VARIANT* child);
-
-  // Retrieves the specified object's shortcut.
-  STDMETHODIMP get_accKeyboardShortcut(VARIANT var_id, BSTR* access_key);
-
-  // Non-supported IAccessible methods.
-
-  // Out-dated and can be safely said to be very rarely used.
-  STDMETHODIMP accDoDefaultAction(VARIANT var_id);
-
-  // Selections not applicable to views.
-  STDMETHODIMP get_accSelection(VARIANT* selected);
-  STDMETHODIMP accSelect(LONG flags_sel, VARIANT var_id);
-
-  // Help functions not supported.
-  STDMETHODIMP get_accHelp(VARIANT var_id, BSTR* help);
-  STDMETHODIMP get_accHelpTopic(BSTR* help_file, VARIANT var_id,
-                                LONG* topic_id);
-
-  // Deprecated functions, not implemented here.
-  STDMETHODIMP put_accName(VARIANT var_id, BSTR put_name);
-  STDMETHODIMP put_accValue(VARIANT var_id, BSTR put_val);
-
- protected:
-  // A pointer containing the Windows' default IAccessible implementation for
-  // this object. Used where it is acceptable to return default MSAA
-  // information.
-  CComPtr<IAccessible> default_accessibility_server_;
-
- private:
-  const AutocompleteEditView* edit_box_;
-
-  DISALLOW_EVIL_CONSTRUCTORS(AutocompleteAccessibility);
-};
-#endif  // CHROME_BROWSER_ACCESSIBILITY_AUTOCOMPLETE_ACCESSIBILITY_H__
-
-
diff --git a/chrome/views/accessibility/view_accessibility.cc b/chrome/views/accessibility/view_accessibility.cc
index ac7ed71..bf88d09 100644
--- a/chrome/views/accessibility/view_accessibility.cc
+++ b/chrome/views/accessibility/view_accessibility.cc
@@ -4,10 +4,8 @@
 
 #include "chrome/views/accessibility/view_accessibility.h"
 
-#include "base/logging.h"
-#include "chrome/browser/view_ids.h"
-#include "chrome/browser/views/location_bar_view.h"
 #include "chrome/views/accessibility/accessible_wrapper.h"
+#include "chrome/views/widget.h"
 
 HRESULT ViewAccessibility::Initialize(views::View* view) {
   if (!view) {
@@ -19,11 +17,10 @@ HRESULT ViewAccessibility::Initialize(views::View* view) {
 }
 
 STDMETHODIMP ViewAccessibility::get_accChildCount(LONG* child_count) {
-  if (!child_count) {
+  if (!child_count || !view_) {
     return E_INVALIDARG;
   }
 
-  DCHECK(view_);
   *child_count = view_->GetChildViewCount();
   return S_OK;
 }
@@ -39,52 +36,33 @@ STDMETHODIMP ViewAccessibility::get_accChild(VARIANT var_child,
     return S_OK;
   }
 
-  views::View* child = NULL;
+  views::View* child_view = NULL;
   bool get_iaccessible = false;
 
   // Check to see if child is out-of-bounds.
   if (IsValidChild((var_child.lVal - 1), view_)) {
-    child = view_->GetChildViewAt(var_child.lVal - 1);
+    child_view = view_->GetChildViewAt(var_child.lVal - 1);
   } else {
-    // Child is further down the hierarchy, get ID and adjust for MSAA.
-    child = view_->GetViewByID(static_cast<int>(var_child.lVal));
+    // Child is located elsewhere in the hierarchy, get ID and adjust for MSAA.
+    child_view = view_->GetViewByID(static_cast<int>(var_child.lVal));
+
+    // Child view does not exist, or id is incorrect.
+    if (!child_view)
+      return E_INVALIDARG;
+
     get_iaccessible = true;
   }
-  // TODO(klink): Add bounds checking for View IDs and an else for OOB error.
 
-  if (!child) {
+  if (!child_view) {
     // No child found.
     *disp_child = NULL;
     return E_FAIL;
   }
 
-  // Sprecial case to handle the AutocompleteEdit MSAA.
-  if (child->GetID() == VIEW_ID_AUTOCOMPLETE) {
-    views::View* parent = child->GetParent();
-
-    // Paranoia check, to make sure we are making a correct cast.
-    if (parent->GetID() == VIEW_ID_LOCATION_BAR) {
-      LocationBarView* location_bar =
-          static_cast<LocationBarView*>(parent);
-
-      // Set the custom IAccessible for the HWNDView containing
-      // AutocompleteEdit.
-      IAccessible* location_entry_accessibility =
-          location_bar->location_entry()->GetIAccessible();
-      if (!location_entry_accessibility)
-        return E_NOINTERFACE;
-
-      GetAccessibleWrapper(child)->SetInstance(location_entry_accessibility);
-      // Setting bool to be true, as we have inserted an IAccessible on a
-      // leaf, and we need ref counting to happen properly.
-      get_iaccessible = true;
-    }
-  }
-
-  if (get_iaccessible || child->GetChildViewCount() != 0) {
+  if (get_iaccessible || child_view->GetChildViewCount() != 0) {
     // Retrieve the IUnknown interface for the requested child view, and
     // assign the IDispatch returned.
-    if ((GetAccessibleWrapper(child))->
+    if ((GetAccessibleWrapper(child_view))->
         GetInstance(IID_IAccessible,
                     reinterpret_cast<void**>(disp_child)) == S_OK) {
       // Increment the reference count for the retrieved interface.
@@ -106,9 +84,9 @@ STDMETHODIMP ViewAccessibility::get_accParent(IDispatch** disp_parent) {
     return E_INVALIDARG;
   }
 
-  views::View* parent = view_->GetParent();
+  views::View* parent_view = view_->GetParent();
 
-  if (!parent) {
+  if (!parent_view) {
     // This function can get called during teardown of WidetWin so we
     // should bail out if we fail to get the HWND.
     if (!view_->GetWidget() || !view_->GetWidget()->GetHWND()) {
@@ -134,7 +112,7 @@ STDMETHODIMP ViewAccessibility::get_accParent(IDispatch** disp_parent) {
 
   // Retrieve the IUnknown interface for the parent view, and assign the
   // IDispatch returned.
-  if ((GetAccessibleWrapper(parent))->
+  if ((GetAccessibleWrapper(parent_view))->
       GetInstance(IID_IAccessible,
                   reinterpret_cast<void**>(disp_parent)) == S_OK) {
     // Increment the reference count for the retrieved interface.
@@ -336,7 +314,6 @@ STDMETHODIMP ViewAccessibility::get_accName(VARIANT var_id, BSTR* name) {
     // If view has no name, return S_FALSE.
     return S_FALSE;
   }
-  DCHECK(*name);
 
   return S_OK;
 }
@@ -361,7 +338,6 @@ STDMETHODIMP ViewAccessibility::get_accDescription(VARIANT var_id, BSTR* desc) {
   } else {
     return S_FALSE;
   }
-  DCHECK(*desc);
 
   return S_OK;
 }
@@ -383,7 +359,10 @@ STDMETHODIMP ViewAccessibility::get_accState(VARIANT var_id, VARIANT* state) {
     // Retrieve all currently applicable states of the child.
     this->SetState(state, view_->GetChildViewAt(var_id.lVal - 1));
   }
-  DCHECK((*state).vt != VT_EMPTY);
+
+  // Make sure that state is not empty, and has the proper type.
+  if (state->vt == VT_EMPTY)
+    return E_FAIL;
 
   return S_OK;
 }
@@ -407,7 +386,10 @@ STDMETHODIMP ViewAccessibility::get_accRole(VARIANT var_id, VARIANT* role) {
       return E_FAIL;
     }
   }
-  DCHECK((*role).vt != VT_EMPTY);
+
+  // Make sure that role is not empty, and has the proper type.
+  if (role->vt == VT_EMPTY)
+    return E_FAIL;
 
   return S_OK;
 }
@@ -434,7 +416,6 @@ STDMETHODIMP ViewAccessibility::get_accDefaultAction(VARIANT var_id,
   } else {
     return S_FALSE;
   }
-  DCHECK(*def_action);
 
   return S_OK;
 }
@@ -566,7 +547,6 @@ STDMETHODIMP ViewAccessibility::get_accKeyboardShortcut(VARIANT var_id,
   } else {
     return S_FALSE;
   }
-  DCHECK(*acc_key);
 
   return S_OK;
 }
diff --git a/chrome/views/views.vcproj b/chrome/views/views.vcproj
index e4cdc56..48681bc 100644
--- a/chrome/views/views.vcproj
+++ b/chrome/views/views.vcproj
@@ -133,14 +133,6 @@
 				>

 			</File>

 			<File

-				RelativePath=".\accessibility\autocomplete_accessibility.cc"

-				>

-			</File>

-			<File

-				RelativePath=".\accessibility\autocomplete_accessibility.h"

-				>

-			</File>

-			<File

 				RelativePath=".\accessibility\view_accessibility.cc"

 				>

 			</File>