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

9 files changed, 48 insertions, 71 deletions

diff --git a/chrome/views/accessibility/autocomplete_accessibility.cc b/chrome/browser/autocomplete/autocomplete_accessibility.cc
index 4318a4a..76eddca 100644
--- a/chrome/views/accessibility/autocomplete_accessibility.cc
+++ b/chrome/browser/autocomplete/autocomplete_accessibility.cc
@@ -2,11 +2,11 @@
 // 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 "chrome/browser/autocomplete/autocomplete_accessibility.h"
 
-#include "base/logging.h"
 #include "chrome/common/l10n_util.h"
 #include "chrome/views/accessibility/accessible_wrapper.h"
+#include "chrome/views/view.h"
 
 #include "generated_resources.h"
 
diff --git a/chrome/views/accessibility/autocomplete_accessibility.h b/chrome/browser/autocomplete/autocomplete_accessibility.h
index 847e251..3bddf15 100644
--- a/chrome/views/accessibility/autocomplete_accessibility.h
+++ b/chrome/browser/autocomplete/autocomplete_accessibility.h
@@ -2,8 +2,8 @@
 // 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__
+#ifndef CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_ACCESSIBILITY_H_
+#define CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_ACCESSIBILITY_H_
 
 #include <atlbase.h>
 #include <atlcom.h>
@@ -11,7 +11,6 @@
 #include <oleacc.h>
 
 #include "chrome/browser/autocomplete/autocomplete_edit.h"
-#include "chrome/views/view.h"
 
 ////////////////////////////////////////////////////////////////////////////////
 //
@@ -110,6 +109,6 @@ class ATL_NO_VTABLE AutocompleteAccessibility
 
   DISALLOW_EVIL_CONSTRUCTORS(AutocompleteAccessibility);
 };
-#endif  // CHROME_BROWSER_ACCESSIBILITY_AUTOCOMPLETE_ACCESSIBILITY_H__
+#endif  // CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_ACCESSIBILITY_H_
 
 
diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc
index 6fb50f1..994315e 100644
--- a/chrome/browser/autocomplete/autocomplete_edit.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit.cc
@@ -13,6 +13,7 @@
 #include "base/scoped_clipboard_writer.h"
 #include "base/string_util.h"
 #include "chrome/app/chrome_dll_resource.h"
+#include "chrome/browser/autocomplete/autocomplete_accessibility.h"
 #include "chrome/browser/autocomplete/autocomplete_popup.h"
 #include "chrome/browser/autocomplete/edit_drop_target.h"
 #include "chrome/browser/autocomplete/keyword_provider.h"
@@ -32,7 +33,6 @@
 #include "chrome/common/l10n_util.h"
 #include "chrome/common/os_exchange_data.h"
 #include "chrome/common/win_util.h"
-#include "chrome/views/accessibility/autocomplete_accessibility.h"
 #include "googleurl/src/url_util.h"
 #include "skia/ext/skia_utils_win.h"
 
diff --git a/chrome/browser/browser.scons b/chrome/browser/browser.scons
index 010ebd0..04f8eac 100644
--- a/chrome/browser/browser.scons
+++ b/chrome/browser/browser.scons
@@ -107,6 +107,7 @@ if env['PLATFORM'] == 'win32':
       'alternate_nav_url_fetcher.cc',
       'app_modal_dialog_queue.cc',
       'autocomplete/autocomplete.cc',
+      'autocomplete/autocomplete_accessibility.cc',
       'autocomplete/autocomplete_edit.cc',
       'autocomplete/autocomplete_popup.cc',
       'autocomplete/edit_drop_target.cc',
diff --git a/chrome/browser/browser.vcproj b/chrome/browser/browser.vcproj
index 251ceff..35d90cb 100644
--- a/chrome/browser/browser.vcproj
+++ b/chrome/browser/browser.vcproj
@@ -142,6 +142,14 @@
 				>

 			</File>

 			<File

+				RelativePath=".\autocomplete\autocomplete_accessibility.cc"

+				>

+			</File>

+			<File

+				RelativePath=".\autocomplete\autocomplete_accessibility.h"

+				>

+			</File>

+			<File

 				RelativePath=".\autocomplete\autocomplete_edit.cc"

 				>

 			</File>

@@ -1490,19 +1498,19 @@
 				>

 			</File>

 			<File

-				RelativePath=".\sessions\session_id.cc"

+				RelativePath=".\sessions\session_command.cc"

 				>

 			</File>

 			<File

-				RelativePath=".\sessions\session_id.h"

+				RelativePath=".\sessions\session_command.h"

 				>

 			</File>

 			<File

-				RelativePath=".\sessions\session_command.cc"

+				RelativePath=".\sessions\session_id.cc"

 				>

 			</File>

 			<File

-				RelativePath=".\sessions\session_command.h"

+				RelativePath=".\sessions\session_id.h"

 				>

 			</File>

 			<File

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/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>