Skip to content

GitLab

T
tangled
Commit 5b4190a6 authored by Hiroki Nakagawa's avatar Hiroki Nakagawa Committed by Commit Bot
Browse files
Options

Revert "Add pixel test for AccessibilityFocusHighlight based on FocusRingBrowserTest" 

This reverts commit b89144a4.

Reason for revert:
AccessibilityFocusHighlightBrowserTest.FocusAppearance is constantly
failing on the "Mac10.11 Tests" bot.
https://ci.chromium.org/p/chromium/builders/ci/Mac10.11%20Tests/54112

Original change's description:
> Add pixel test for AccessibilityFocusHighlight based on FocusRingBrowserTest
> 
> AX-Relnotes: n/a
> 
> Bug: 1114959
> Change-Id: Ib6d9f51a04fa8059dc6bd6192d182b8e0244367d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352451
> Commit-Queue: Alice Boxhall <aboxhall@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Reviewed-by: Joey Arhar <jarhar@chromium.org>
> Reviewed-by: Mason Freed <masonfreed@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#803007}

TBR=ellyjones@chromium.org,dmazzoni@chromium.org,aboxhall@chromium.org,masonfreed@chromium.org,jarhar@chromium.org

Change-Id: I61f57468a6f4172dc73cb72fb601855e4ab99484
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1114959
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2383376Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803019}
parent 7a26ab64
Hide whitespace changes
Inline Side-by-side
Showing with 19 additions and 173 deletions
chrome/browser/ui/views/accessibility/accessibility_focus_highlight.cc
View file @ 5b4190a6
......@@ -77,9 +77,6 @@ bool AccessibilityFocusHighlight::skip_activation_check_for_testing_ = false;
// static
bool AccessibilityFocusHighlight::use_default_color_for_testing_ = false;
// static
bool AccessibilityFocusHighlight::no_fade_for_testing_ = false;
AccessibilityFocusHighlight::AccessibilityFocusHighlight(
BrowserView* browser_view)
: browser_view_(browser_view) {
......@@ -101,7 +98,7 @@ AccessibilityFocusHighlight::AccessibilityFocusHighlight(
fade_in_time_ = kFadeInTime;
persist_time_ = kHighlightPersistTime;
fade_out_time_ = kFadeOutTime;
default_color_ = SkColorSetRGB(0x10, 0x10, 0x10); // #101010
default_color_ = SkColorSetRGB(16, 16, 16); // #101010
}
}
......@@ -112,7 +109,9 @@ AccessibilityFocusHighlight::~AccessibilityFocusHighlight() {
// static
void AccessibilityFocusHighlight::SetNoFadeForTesting() {
no_fade_for_testing_ = true;
fade_in_time_ = base::TimeDelta();
persist_time_ = base::TimeDelta::FromHours(1);
fade_out_time_ = base::TimeDelta();
}
// static
......@@ -125,17 +124,7 @@ void AccessibilityFocusHighlight::UseDefaultColorForTesting() {
use_default_color_for_testing_ = true;
}
// static
ui::Layer* AccessibilityFocusHighlight::GetLayerForTesting() {
return layer_.get();
}
SkColor AccessibilityFocusHighlight::GetHighlightColor() {
#if !defined(OS_MAC)
// Match behaviour with renderer_preferences_util::UpdateFromSystemSettings
// setting prefs->focus_ring_color
return default_color_;
#else
ui::NativeTheme* native_theme = ui::NativeTheme::GetInstanceForWeb();
SkColor theme_color = native_theme->GetSystemColor(
ui::NativeTheme::kColorId_FocusedBorderColor);
......@@ -144,7 +133,6 @@ SkColor AccessibilityFocusHighlight::GetHighlightColor() {
return default_color_;
return native_theme->FocusRingColorForBaseColor(theme_color);
#endif
}
void AccessibilityFocusHighlight::CreateOrUpdateLayer(gfx::Rect node_bounds) {
......@@ -205,9 +193,6 @@ void AccessibilityFocusHighlight::CreateOrUpdateLayer(gfx::Rect node_bounds) {
}
void AccessibilityFocusHighlight::RemoveLayer() {
if (no_fade_for_testing_)
return;
layer_.reset();
if (compositor_) {
compositor_->RemoveAnimationObserver(this);
......@@ -326,29 +311,6 @@ void AccessibilityFocusHighlight::OnPaintLayer(
recorder.canvas()->DrawRoundRect(bounds, kBorderRadius, original_flags);
}
float AccessibilityFocusHighlight::ComputeOpacity(
base::TimeDelta time_since_layer_create,
base::TimeDelta time_since_focus_move) {
float opacity = 1.0f;
if (no_fade_for_testing_)
return opacity;
if (time_since_layer_create < fade_in_time_) {
// We're fading in.
opacity = time_since_layer_create / fade_in_time_;
}
if (time_since_focus_move > persist_time_) {
// Fading out.
base::TimeDelta time_since_began_fading =
time_since_focus_move - (fade_in_time_ + persist_time_);
opacity = 1.0f - (time_since_began_fading / fade_out_time_);
}
return base::ClampToRange(opacity, 0.0f, 1.0f);
}
void AccessibilityFocusHighlight::OnAnimationStep(base::TimeTicks timestamp) {
if (!layer_)
return;
......@@ -376,8 +338,21 @@ void AccessibilityFocusHighlight::OnAnimationStep(base::TimeTicks timestamp) {
return;
}
float opacity =
ComputeOpacity(time_since_layer_create, time_since_focus_move);
// Compute the opacity based on the fade in and fade out times.
// TODO(aboxhall): figure out how to use cubic beziers
float opacity = 1.0f;
if (time_since_layer_create < fade_in_time_) {
// We're fading in.
opacity = time_since_layer_create / fade_in_time_;
} else if (time_since_focus_move > persist_time_) {
// Fading out.
base::TimeDelta time_since_began_fading =
time_since_focus_move - (fade_in_time_ + persist_time_);
opacity = 1.0f - (time_since_began_fading / fade_out_time_);
}
// Layer::SetOpacity will throw an error if we're not within 0...1.
opacity = base::ClampToRange(opacity, 0.0f, 1.0f);
layer_->SetOpacity(opacity);
}
......
chrome/browser/ui/views/accessibility/accessibility_focus_highlight.h
View file @ 5b4190a6
......@@ -42,13 +42,10 @@ class AccessibilityFocusHighlight : public ui::LayerDelegate,
private:
FRIEND_TEST_ALL_PREFIXES(AccessibilityFocusHighlightBrowserTest,
DrawsHighlight);
FRIEND_TEST_ALL_PREFIXES(AccessibilityFocusHighlightBrowserTest,
FocusAppearance);
// For testing.
static void SetNoFadeForTesting();
static void SkipActivationCheckForTesting();
static void UseDefaultColorForTesting();
ui::Layer* GetLayerForTesting();
// Create the layer if needed, and set node_bounds_
void CreateOrUpdateLayer(gfx::Rect node_bounds);
......@@ -76,11 +73,6 @@ class AccessibilityFocusHighlight : public ui::LayerDelegate,
// Compute the highlight color based on theme colors and defaults.
SkColor GetHighlightColor();
// Compute the opacity based on the fade in and fade out times.
// TODO(aboxhall): figure out how to use cubic beziers
float ComputeOpacity(base::TimeDelta time_since_layer_create,
base::TimeDelta time_since_focus_move);
// The layer, if visible.
std::unique_ptr<ui::Layer> layer_;
......@@ -102,9 +94,6 @@ class AccessibilityFocusHighlight : public ui::LayerDelegate,
// The default color used for the highlight.
static SkColor default_color_;
// Whether to skip fade in/fade out for testing.
static bool no_fade_for_testing_;
// The amount of time it should take for the highlight to fade in.
static base::TimeDelta fade_in_time_;
......
chrome/browser/ui/views/accessibility/accessibility_focus_highlight_browsertest.cc
View file @ 5b4190a6
......@@ -4,18 +4,12 @@
#include "math.h"
#include "base/path_service.h"
#include "build/build_config.h"
#include "cc/test/pixel_comparator.h"
#include "cc/test/pixel_test_utils.h"
#include "chrome/browser/ui/views/accessibility/accessibility_focus_highlight.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/viz/common/frame_sinks/copy_output_request.h"
#include "components/viz/common/frame_sinks/copy_output_result.h"
#include "content/public/browser/focused_node_details.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/notification_types.h"
......@@ -26,19 +20,6 @@
#include "ui/snapshot/snapshot.h"
#include "ui/views/widget/widget.h"
#if defined(OS_MAC)
#include "base/mac/mac_util.h"
#endif
// To rebaseline this test on all platforms:
// 1. Run a CQ+1 dry run.
// 2. Click the failing bots for android, windows, mac, and linux.
// 3. Find the failing interactive_ui_browsertests step.
// 4. Click the "Deterministic failure" link for the failing test case.
// 5. Copy the "Actual pixels" data url and paste into browser.
// 6. Save the image into your chromium checkout in
// chrome/test/data/accessibility.
class AccessibilityFocusHighlightBrowserTest : public InProcessBrowserTest {
public:
AccessibilityFocusHighlightBrowserTest() = default;
......@@ -248,98 +229,3 @@ IN_PROC_BROWSER_TEST_F(AccessibilityFocusHighlightBrowserTest,
// Not sure where the extra px of height are coming from...
EXPECT_EQ(147, bounds.height());
}
class ReadbackHolder : public base::RefCountedThreadSafe<ReadbackHolder> {
public:
ReadbackHolder() : run_loop_(std::make_unique<base::RunLoop>()) {}
void OutputRequestCallback(std::unique_ptr<viz::CopyOutputResult> result) {
if (result->IsEmpty())
result_.reset();
else
result_ = std::make_unique<SkBitmap>(result->AsSkBitmap());
run_loop_->Quit();
}
void WaitForReadback() { run_loop_->Run(); }
const SkBitmap& result() const { return *result_; }
private:
friend class base::RefCountedThreadSafe<ReadbackHolder>;
virtual ~ReadbackHolder() = default;
std::unique_ptr<SkBitmap> result_;
std::unique_ptr<base::RunLoop> run_loop_;
};
const cc::ExactPixelComparator pixel_comparator(/*discard_alpha=*/false);
IN_PROC_BROWSER_TEST_F(AccessibilityFocusHighlightBrowserTest,
FocusAppearance) {
base::ScopedAllowBlockingForTesting allow_blocking;
AccessibilityFocusHighlight::SetNoFadeForTesting();
AccessibilityFocusHighlight::SkipActivationCheckForTesting();
browser()->profile()->GetPrefs()->SetBoolean(
prefs::kAccessibilityFocusHighlightEnabled, true);
ui_test_utils::NavigateToURL(browser(), GURL("data:text/html,"
"<a id='link' href=''>"
"<img width='10' height='10'>"
"</a>"));
FocusedNodeChangedObserver observer;
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
std::string script("document.getElementById('link').focus();");
ASSERT_TRUE(content::ExecuteScript(web_contents, script));
observer.WaitForFocusChangeInPage();
BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
AccessibilityFocusHighlight* highlight =
browser_view->GetAccessibilityFocusHighlightForTesting();
ui::Layer* layer = highlight->GetLayerForTesting();
gfx::Rect source_rect = gfx::Rect(layer->size());
scoped_refptr<ReadbackHolder> holder(new ReadbackHolder);
std::unique_ptr<viz::CopyOutputRequest> request =
std::make_unique<viz::CopyOutputRequest>(
viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
base::BindOnce(&ReadbackHolder::OutputRequestCallback, holder));
request->set_area(source_rect);
layer->RequestCopyOfOutput(std::move(request));
holder->WaitForReadback();
SkBitmap bitmap(holder->result());
ASSERT_FALSE(bitmap.drawsNothing());
base::FilePath dir_test_data;
ASSERT_TRUE(base::PathService::Get(chrome::DIR_TEST_DATA, &dir_test_data));
std::string screenshot_filename = "focus_highlight_appearance";
std::string platform_suffix;
#if defined(OS_MAC)
// Mac 10.14 used a subtly different highlight color.
if (base::mac::IsOS10_14())
platform_suffix = "_mac10.14";
else
platform_suffix = "_mac";
#elif defined(OS_WIN)
platform_suffix = "_win";
#endif
base::FilePath golden_filepath =
dir_test_data.AppendASCII("accessibility")
.AppendASCII(screenshot_filename + ".png");
base::FilePath golden_filepath_platform =
golden_filepath.InsertBeforeExtensionASCII(platform_suffix);
if (base::PathExists(golden_filepath_platform)) {
golden_filepath = golden_filepath_platform;
}
bool snapshot_matches =
cc::MatchesPNGFile(bitmap, golden_filepath, pixel_comparator);
EXPECT_TRUE(snapshot_matches);
}
chrome/browser/ui/views/frame/browser_view.h
View file @ 5b4190a6
......@@ -593,10 +593,6 @@ class BrowserView : public BrowserWindow,
// Create and open the tab search bubble.
void CreateTabSearchBubble() override;
AccessibilityFocusHighlight* GetAccessibilityFocusHighlightForTesting() {
return accessibility_focus_highlight_.get();
}
private:
// Do not friend BrowserViewLayout. Use the BrowserViewLayoutDelegate
// interface to keep these two classes decoupled and testable.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment