diff options
| author | kelvinp <kelvinp@chromium.org> | 2015-02-05 13:37:25 -0800 |
|---|---|---|
| committer | Commit bot <commit-bot@chromium.org> | 2015-02-05 21:39:40 +0000 |
| commit | 7fcb729df6369250da0eccb113a64073e272de39 (patch) | |
| tree | 0dfa8da1a98e1287f67e98bab5f436e25c839cae | |
| parent | bc2fbac8dcbb2a4daa87a906bf03f16861eab5fa (diff) | |
| download | chromium_src-7fcb729df6369250da0eccb113a64073e272de39.zip chromium_src-7fcb729df6369250da0eccb113a64073e272de39.tar.gz chromium_src-7fcb729df6369250da0eccb113a64073e272de39.tar.bz2 | |
Revert of Suppress accessibility events when user is navigating away. (patchset #6 id:100001 of https://codereview.chromium.org/830053004/)
Reason for revert:
NavigationAccessibilityTest.TestNavigateToNewUrl is failing on Win7 Tests (dbg)(1).
Sample failure:
http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/35087
Original issue's description:
> Suppress accessibility events when user is navigating away.
>
> When the user navigates to a new page, stop sending accessibility
> events on the old page.
>
> BUG=421116,450409
>
> Committed: https://crrev.com/6ce40a1e561892849c1f6ac070dda140f6cc0115
> Cr-Commit-Position: refs/heads/master@{#314812}
TBR=nasko@chromium.org,dmazzoni@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=421116,450409
Review URL: https://codereview.chromium.org/892513004
Cr-Commit-Position: refs/heads/master@{#314884}
7 files changed, 2 insertions, 328 deletions
diff --git a/chrome/browser/ui/views/accessibility/navigation_accessibility_uitest_win.cc b/chrome/browser/ui/views/accessibility/navigation_accessibility_uitest_win.cc deleted file mode 100644 index 483a9dc..0000000 --- a/chrome/browser/ui/views/accessibility/navigation_accessibility_uitest_win.cc +++ /dev/null @@ -1,255 +0,0 @@ -// Copyright (c) 2015 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 <oleacc.h> - -#include "base/strings/string_util.h" -#include "base/win/scoped_bstr.h" -#include "base/win/scoped_com_initializer.h" -#include "base/win/scoped_comptr.h" -#include "base/win/scoped_variant.h" -#include "chrome/app/chrome_command_ids.h" -#include "chrome/browser/ui/browser.h" -#include "chrome/browser/ui/browser_commands.h" -#include "chrome/browser/ui/browser_window.h" -#include "chrome/browser/ui/omnibox/omnibox_view.h" -#include "chrome/browser/ui/views/frame/browser_view.h" -#include "chrome/browser/ui/views/omnibox/omnibox_view_views.h" -#include "chrome/browser/ui/views/toolbar/toolbar_view.h" -#include "chrome/test/base/in_process_browser_test.h" -#include "chrome/test/base/interactive_test_utils.h" -#include "chrome/test/base/ui_test_utils.h" -#include "content/public/browser/browser_accessibility_state.h" -#include "net/dns/mock_host_resolver.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "ui/base/test/ui_controls.h" -#include "url/gurl.h" - -// We could move this into a utility file in the future if it ends up -// being useful to other tests. -class WinAccessibilityEventMonitor { - public: - WinAccessibilityEventMonitor(UINT event_min, UINT event_max); - ~WinAccessibilityEventMonitor(); - - // Blocks until the next event is received. When it's received, it - // queries accessibility information about the object that fired the - // event and populates the event, hwnd, role, state, and name in the - // passed arguments. - void WaitForNextEvent(DWORD* out_event, - HWND* out_hwnd, - UINT* out_role, - UINT* out_state, - std::string* out_name); - - private: - void OnWinEventHook(HWINEVENTHOOK handle, - DWORD event, - HWND hwnd, - LONG obj_id, - LONG child_id, - DWORD event_thread, - DWORD event_time); - - static void CALLBACK WinEventHookThunk( - HWINEVENTHOOK handle, - DWORD event, - HWND hwnd, - LONG obj_id, - LONG child_id, - DWORD event_thread, - DWORD event_time); - - struct EventInfo { - DWORD event; - HWND hwnd; - LONG obj_id; - LONG child_id; - }; - - std::deque<EventInfo> event_queue_; - scoped_refptr<content::MessageLoopRunner> loop_runner_; - HWINEVENTHOOK win_event_hook_handle_; - static WinAccessibilityEventMonitor* instance_; - - DISALLOW_COPY_AND_ASSIGN(WinAccessibilityEventMonitor); -}; - -// static -WinAccessibilityEventMonitor* WinAccessibilityEventMonitor::instance_ = NULL; - -WinAccessibilityEventMonitor::WinAccessibilityEventMonitor( - UINT event_min, UINT event_max) { - CHECK(!instance_) << "There can be only one instance of" - << " WinAccessibilityEventMonitor at a time."; - instance_ = this; - win_event_hook_handle_ = SetWinEventHook( - event_min, - event_max, - NULL, - &WinAccessibilityEventMonitor::WinEventHookThunk, - GetCurrentProcessId(), - 0, // Hook all threads - WINEVENT_OUTOFCONTEXT); -} - -WinAccessibilityEventMonitor::~WinAccessibilityEventMonitor() { - UnhookWinEvent(win_event_hook_handle_); - instance_ = NULL; -} - -void WinAccessibilityEventMonitor::WaitForNextEvent( - DWORD* out_event, - HWND* out_hwnd, - UINT* out_role, - UINT* out_state, - std::string* out_name) { - if (event_queue_.empty()) { - loop_runner_ = new content::MessageLoopRunner(); - loop_runner_->Run(); - loop_runner_ = NULL; - } - EventInfo event_info = event_queue_.front(); - event_queue_.pop_front(); - - *out_event = event_info.event; - *out_hwnd = event_info.hwnd; - - base::win::ScopedComPtr<IAccessible> acc_obj; - base::win::ScopedVariant child_variant; - CHECK(S_OK == AccessibleObjectFromEvent( - event_info.hwnd, event_info.obj_id, event_info.child_id, - acc_obj.Receive(), child_variant.Receive())); - - base::win::ScopedVariant role_variant; - if (S_OK == acc_obj->get_accRole(child_variant, role_variant.Receive())) - *out_role = V_I4(&role_variant); - else - *out_role = 0; - - base::win::ScopedVariant state_variant; - if (S_OK == acc_obj->get_accState(child_variant, state_variant.Receive())) - *out_state = V_I4(&state_variant); - else - *out_state = 0; - - base::win::ScopedBstr name_bstr; - HRESULT hr = acc_obj->get_accName(child_variant, name_bstr.Receive()); - if (S_OK == hr) - *out_name = base::UTF16ToUTF8(base::string16(name_bstr)); - else - *out_name = ""; -} - -void WinAccessibilityEventMonitor::OnWinEventHook( - HWINEVENTHOOK handle, - DWORD event, - HWND hwnd, - LONG obj_id, - LONG child_id, - DWORD event_thread, - DWORD event_time) { - EventInfo event_info; - event_info.event = event; - event_info.hwnd = hwnd; - event_info.obj_id = obj_id; - event_info.child_id = child_id; - event_queue_.push_back(event_info); - if (loop_runner_.get()) - loop_runner_->Quit(); -} - -// static -void CALLBACK WinAccessibilityEventMonitor::WinEventHookThunk( - HWINEVENTHOOK handle, - DWORD event, - HWND hwnd, - LONG obj_id, - LONG child_id, - DWORD event_thread, - DWORD event_time) { - if (instance_) { - instance_->OnWinEventHook(handle, event, hwnd, obj_id, child_id, - event_thread, event_time); - } -} - -class NavigationAccessibilityTest : public InProcessBrowserTest { - protected: - NavigationAccessibilityTest() {} - virtual ~NavigationAccessibilityTest() {} - - void SendKeyPress(ui::KeyboardCode key) { - gfx::NativeWindow native_window = browser()->window()->GetNativeWindow(); - ASSERT_NO_FATAL_FAILURE( - ASSERT_TRUE( - ui_test_utils::SendKeyPressToWindowSync( - native_window, key, false, false, false, false))); - } - - private: - base::win::ScopedCOMInitializer com_initializer_; - - DISALLOW_COPY_AND_ASSIGN(NavigationAccessibilityTest); -}; - -// Tests that when focus is in the omnibox and the user types a url and -// presses enter, no focus events are sent on the old document. -IN_PROC_BROWSER_TEST_F(NavigationAccessibilityTest, TestNavigateToNewUrl) { - content::BrowserAccessibilityState::GetInstance()->EnableAccessibility(); - - ui_test_utils::NavigateToURL(browser(), - GURL("data:text/html;charset=utf-8," - "<head><title>First Page</title></head>")); - - chrome::ExecuteCommand(browser(), IDC_FOCUS_LOCATION); - - host_resolver()->AddRule("*", "127.0.0.1"); - ASSERT_TRUE(test_server()->Start()); - GURL main_url(test_server()->GetURL("files/english_page.html")); - - OmniboxViewViews* omnibox_view = - BrowserView::GetBrowserViewForBrowser(browser())-> - toolbar()->location_bar()->omnibox_view(); - omnibox_view->SetUserText(base::UTF8ToUTF16(main_url.spec()), - base::UTF8ToUTF16(main_url.spec()), - false); - - WinAccessibilityEventMonitor monitor(EVENT_OBJECT_FOCUS, EVENT_OBJECT_FOCUS); - SendKeyPress(ui::VKEY_RETURN); - - for (;;) { - DWORD event; - HWND hwnd; - UINT role; - UINT state; - std::string name; - monitor.WaitForNextEvent(&event, &hwnd, &role, &state, &name); - - LOG(INFO) << "Got event: " - << " event=" << event - << " hwnd=" << hwnd - << " role=" << role - << " state=" << state - << " name=" << name; - - // We should get only focus events. - EXPECT_EQ(EVENT_OBJECT_FOCUS, event); - - // We should get only focus events on document objects. (On a page with - // JavaScript or autofocus, additional focus events would be expected.) - EXPECT_EQ(ROLE_SYSTEM_DOCUMENT, role); - - // We shouldn't get any events on the first page because from the time - // we start monitoring, the user has already initiated a load to the - // second page. - EXPECT_NE("First Page", name); - - // Finish when we get an event on the second page. - if (name == "This page is in English") { - LOG(INFO) << "Got event on second page, finishing test."; - break; - } - } -} diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index fd92958..383f35f 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -941,7 +941,6 @@ 'browser/ui/toolbar/test_toolbar_model.h', 'browser/ui/webui/options/autofill_options_interactive_uitest.cc', 'browser/ui/webui/options/language_options_interactive_uitest.cc', - 'browser/ui/views/accessibility/navigation_accessibility_uitest_win.cc', 'test/base/interactive_test_utils.cc', 'test/base/interactive_test_utils.h', 'test/base/interactive_test_utils_aura.cc', diff --git a/content/browser/accessibility/browser_accessibility_manager.cc b/content/browser/accessibility/browser_accessibility_manager.cc index 34a8225..6049901 100644 --- a/content/browser/accessibility/browser_accessibility_manager.cc +++ b/content/browser/accessibility/browser_accessibility_manager.cc @@ -79,7 +79,6 @@ BrowserAccessibilityManager::BrowserAccessibilityManager( factory_(factory), tree_(new ui::AXSerializableTree()), focus_(NULL), - user_is_navigating_away_(false), osk_state_(OSK_ALLOWED) { tree_->SetDelegate(this); } @@ -92,7 +91,6 @@ BrowserAccessibilityManager::BrowserAccessibilityManager( factory_(factory), tree_(new ui::AXSerializableTree()), focus_(NULL), - user_is_navigating_away_(false), osk_state_(OSK_ALLOWED) { tree_->SetDelegate(this); Initialize(initial_tree); @@ -154,22 +152,6 @@ void BrowserAccessibilityManager::OnWindowBlurred() { NotifyAccessibilityEvent(ui::AX_EVENT_BLUR, GetFromAXNode(focus_)); } -void BrowserAccessibilityManager::UserIsNavigatingAway() { - user_is_navigating_away_ = true; -} - -void BrowserAccessibilityManager::UserIsReloading() { - user_is_navigating_away_ = true; -} - -void BrowserAccessibilityManager::NavigationSucceeded() { - user_is_navigating_away_ = false; -} - -void BrowserAccessibilityManager::NavigationFailed() { - user_is_navigating_away_ = false; -} - void BrowserAccessibilityManager::GotMouseDown() { osk_state_ = OSK_ALLOWED_WITHIN_FOCUSED_OBJECT; NotifyAccessibilityEvent(ui::AX_EVENT_FOCUS, GetFromAXNode(focus_)); diff --git a/content/browser/accessibility/browser_accessibility_manager.h b/content/browser/accessibility/browser_accessibility_manager.h index f60bd02..e59a1ed 100644 --- a/content/browser/accessibility/browser_accessibility_manager.h +++ b/content/browser/accessibility/browser_accessibility_manager.h @@ -146,12 +146,6 @@ class CONTENT_EXPORT BrowserAccessibilityManager : public ui::AXTreeDelegate { // view lost focus. virtual void OnWindowBlurred(); - // Notify the accessibility manager about page navigation. - void UserIsNavigatingAway(); - virtual void UserIsReloading(); - void NavigationSucceeded(); - void NavigationFailed(); - // Called to notify the accessibility manager that a mouse down event // occurred in the tab. void GotMouseDown(); @@ -310,9 +304,6 @@ class CONTENT_EXPORT BrowserAccessibilityManager : public ui::AXTreeDelegate { // A mapping from a node id to its wrapper of type BrowserAccessibility. base::hash_map<int32, BrowserAccessibility*> id_wrapper_map_; - // True if the user has initiated a navigation to another page. - bool user_is_navigating_away_; - // The on-screen keyboard state. OnScreenKeyboardState osk_state_; diff --git a/content/browser/accessibility/browser_accessibility_manager_win.cc b/content/browser/accessibility/browser_accessibility_manager_win.cc index 79d9361..ae4f089 100644 --- a/content/browser/accessibility/browser_accessibility_manager_win.cc +++ b/content/browser/accessibility/browser_accessibility_manager_win.cc @@ -179,10 +179,6 @@ void BrowserAccessibilityManagerWin::OnWindowFocused() { BrowserAccessibilityManager::OnWindowFocused(); } -void BrowserAccessibilityManagerWin::UserIsReloading() { - MaybeCallNotifyWinEvent(IA2_EVENT_DOCUMENT_RELOAD, GetRoot()); -} - void BrowserAccessibilityManagerWin::NotifyAccessibilityEvent( ui::AXEvent event_type, BrowserAccessibility* node) { @@ -192,11 +188,6 @@ void BrowserAccessibilityManagerWin::NotifyAccessibilityEvent( return; } - // Don't fire events when this document might be stale as the user has - // started navigating to a new document. - if (user_is_navigating_away_) - return; - // Inline text boxes are an internal implementation detail, we don't // expose them to Windows. if (node->GetRole() == ui::AX_ROLE_INLINE_TEXT_BOX) diff --git a/content/browser/accessibility/browser_accessibility_manager_win.h b/content/browser/accessibility/browser_accessibility_manager_win.h index 24c0b77..7f0039e 100644 --- a/content/browser/accessibility/browser_accessibility_manager_win.h +++ b/content/browser/accessibility/browser_accessibility_manager_win.h @@ -41,9 +41,8 @@ class CONTENT_EXPORT BrowserAccessibilityManagerWin virtual void OnNodeCreated(ui::AXNode* node) override; // BrowserAccessibilityManager methods - void OnWindowFocused() override; - void UserIsReloading() override; - void NotifyAccessibilityEvent( + virtual void OnWindowFocused() override; + virtual void NotifyAccessibilityEvent( ui::AXEvent event_type, BrowserAccessibility* node) override; // Track this object and post a VISIBLE_DATA_CHANGED notification when diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index bb05464..fb5de4a 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -2566,17 +2566,6 @@ void WebContentsImpl::DidStartProvisionalLoad( observers_, DidStartProvisionalLoadForFrame( render_frame_host, validated_url, is_error_page, is_iframe_srcdoc)); - - // Notify accessibility if this is a reload. - NavigationEntry* entry = controller_.GetVisibleEntry(); - if (entry && ui::PageTransitionCoreTypeIs( - entry->GetTransitionType(), ui::PAGE_TRANSITION_RELOAD)) { - FrameTreeNode* ftn = render_frame_host->frame_tree_node(); - BrowserAccessibilityManager* manager = - ftn->current_frame_host()->browser_accessibility_manager(); - if (manager) - manager->UserIsReloading(); - } } void WebContentsImpl::DidStartNavigationTransition( @@ -2598,12 +2587,6 @@ void WebContentsImpl::DidFailProvisionalLoadWithError( validated_url, params.error_code, params.error_description)); - - FrameTreeNode* ftn = render_frame_host->frame_tree_node(); - BrowserAccessibilityManager* manager = - ftn->current_frame_host()->browser_accessibility_manager(); - if (manager) - manager->NavigationFailed(); } void WebContentsImpl::DidFailLoadWithError( @@ -2679,11 +2662,6 @@ void WebContentsImpl::DidCommitProvisionalLoad( observers_, DidCommitProvisionalLoadForFrame( render_frame_host, url, transition_type)); - - BrowserAccessibilityManager* manager = - render_frame_host->browser_accessibility_manager(); - if (manager) - manager->NavigationSucceeded(); } void WebContentsImpl::DidNavigateMainFramePreCommit( @@ -3855,17 +3833,6 @@ void WebContentsImpl::DidStartLoading(RenderFrameHost* render_frame_host, bool to_different_document) { SetIsLoading(render_frame_host->GetRenderViewHost(), true, to_different_document, NULL); - - // Notify accessibility that the user is navigating away from the - // current document. - // - // TODO(dmazzoni): do this using a WebContentsObserver. - FrameTreeNode* ftn = static_cast<RenderFrameHostImpl*>(render_frame_host)-> - frame_tree_node(); - BrowserAccessibilityManager* manager = - ftn->current_frame_host()->browser_accessibility_manager(); - if (manager) - manager->UserIsNavigatingAway(); } void WebContentsImpl::DidStopLoading(RenderFrameHost* render_frame_host) { |