diff --git a/DEPS b/DEPS index 45cc132..6a1024c 100644 --- a/DEPS +++ b/DEPS
@@ -96,7 +96,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling catapult # and whatever else without interference from each other. - 'catapult_revision': '56214f64fe3b5996e1f2a09738a8b5e343199ea5', + 'catapult_revision': 'c9d8aeb226cd14cd176285d0c1c6a8f7031944f7', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling libFuzzer # and whatever else without interference from each other. @@ -232,7 +232,7 @@ Var('chromium_git') + '/native_client/src/third_party/scons-2.0.1.git' + '@' + '1c1550e17fc26355d08627fbdec13d8291227067', 'src/third_party/webrtc': - Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + 'f50f6abafa57f43498e23a9f09492f0fa504c815', # commit position 16764 + Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + 'cc7a3f62e4339daa1021b1bad24507aea24942ef', # commit position 16791 'src/third_party/openmax_dl': Var('chromium_git') + '/external/webrtc/deps/third_party/openmax.git' + '@' + Var('openmax_dl_revision'), @@ -299,7 +299,7 @@ Var('chromium_git') + '/external/github.com/cisco/openh264' + '@' + '0fd88df93c5dcaf858c57eb7892bd27763f0f0ac', 'src/third_party/re2/src': - Var('chromium_git') + '/external/github.com/google/re2.git' + '@' + '193486d7164a40ce28d8ec64552df129c0558bad', + Var('chromium_git') + '/external/github.com/google/re2.git' + '@' + 'db20d46c05900a12539225fe800dd860b14e0061', # Used for building libFuzzers (only supports Linux). 'src/third_party/libFuzzer/src':
diff --git a/ash/BUILD.gn b/ash/BUILD.gn index 3554cc5..ebb51f7 100644 --- a/ash/BUILD.gn +++ b/ash/BUILD.gn
@@ -1046,6 +1046,7 @@ "//ui/views", "//ui/views:test_support", "//ui/views/examples:views_examples_lib", + "//ui/wm", ] }
diff --git a/ash/common/ash_constants.cc b/ash/common/ash_constants.cc index e8b1434..d562a0db 100644 --- a/ash/common/ash_constants.cc +++ b/ash/common/ash_constants.cc
@@ -19,4 +19,6 @@ const SkColor kFocusBorderColor = SkColorSetA(gfx::kGoogleBlue500, 0x99); const float kFocusBorderThickness = 2.f; +const int kDefaultLargeCursorSize = 64; + } // namespace ash
diff --git a/ash/common/ash_constants.h b/ash/common/ash_constants.h index 9ff5092..66ce6d0 100644 --- a/ash/common/ash_constants.h +++ b/ash/common/ash_constants.h
@@ -30,6 +30,8 @@ extern const SkColor kFocusBorderColor; extern const float kFocusBorderThickness; +ASH_EXPORT extern const int kDefaultLargeCursorSize; + } // namespace ash #endif // ASH_COMMON_ASH_CONSTANTS_H_
diff --git a/ash/common/ash_switches.cc b/ash/common/ash_switches.cc index 877bc57..e4cd626 100644 --- a/ash/common/ash_switches.cc +++ b/ash/common/ash_switches.cc
@@ -10,6 +10,9 @@ namespace ash { namespace switches { +// Enables adjustable large cursor. +const char kAshAdjustableLargeCursor[] = "ash-adjustable-large-cursor"; + // Enables an animated transition from the boot splash screen (Chrome logo on a // white background) to the login screen. Implies // |kAshCopyHostBackgroundAtBoot| and doesn't make much sense if used in
diff --git a/ash/common/ash_switches.h b/ash/common/ash_switches.h index fefd9e2a..6295e5e 100644 --- a/ash/common/ash_switches.h +++ b/ash/common/ash_switches.h
@@ -16,6 +16,7 @@ // Please keep alphabetized. // TODO(sky): fix order! +ASH_EXPORT extern const char kAshAdjustableLargeCursor[]; ASH_EXPORT extern const char kAshAnimateFromBootSplashScreen[]; ASH_EXPORT extern const char kAshCopyHostBackgroundAtBoot[]; ASH_EXPORT extern const char kAshDebugShortcuts[];
diff --git a/ash/common/wm/dock/docked_window_layout_manager.cc b/ash/common/wm/dock/docked_window_layout_manager.cc index 3148ab7..3e620c3f 100644 --- a/ash/common/wm/dock/docked_window_layout_manager.cc +++ b/ash/common/wm/dock/docked_window_layout_manager.cc
@@ -21,7 +21,6 @@ #include "ash/public/cpp/shell_window_ids.h" #include "ash/resources/grit/ash_resources.h" #include "ash/root_window_controller.h" -#include "ash/screen_util.h" #include "ash/wm/window_state_aura.h" #include "base/auto_reset.h" #include "base/metrics/histogram_macros.h" @@ -31,6 +30,7 @@ #include "ui/display/display.h" #include "ui/display/screen.h" #include "ui/views/background.h" +#include "ui/wm/core/coordinate_conversion.h" #include "ui/wm/core/window_animations.h" namespace ash { @@ -333,8 +333,8 @@ void OnWindowBoundsChanged(aura::Window* window, const gfx::Rect& old_bounds, const gfx::Rect& new_bounds) override { - shelf_bounds_in_screen_ = - ScreenUtil::ConvertRectToScreen(window->parent(), new_bounds); + shelf_bounds_in_screen_ = new_bounds; + ::wm::ConvertRectToScreen(window->parent(), &shelf_bounds_in_screen_); // When the shelf is auto-hidden, it has an invisible height of 3px used // as a hit region which is specific to Chrome OS MD (for non-MD, the 3
diff --git a/ash/common/wm_window.cc b/ash/common/wm_window.cc index 0fbde9b..0162092 100644 --- a/ash/common/wm_window.cc +++ b/ash/common/wm_window.cc
@@ -15,7 +15,6 @@ #include "ash/public/cpp/shell_window_ids.h" #include "ash/public/cpp/window_properties.h" #include "ash/root_window_controller.h" -#include "ash/screen_util.h" #include "ash/shell.h" #include "ash/wm/resize_handle_window_targeter.h" #include "ash/wm/resize_shadow_controller.h" @@ -244,11 +243,15 @@ } gfx::Rect WmWindow::ConvertRectToScreen(const gfx::Rect& rect) const { - return ScreenUtil::ConvertRectToScreen(window_, rect); + gfx::Rect result(rect); + ::wm::ConvertRectToScreen(window_, &result); + return result; } gfx::Rect WmWindow::ConvertRectFromScreen(const gfx::Rect& rect) const { - return ScreenUtil::ConvertRectFromScreen(window_, rect); + gfx::Rect result(rect); + ::wm::ConvertRectFromScreen(window_, &result); + return result; } gfx::Size WmWindow::GetMinimumSize() const {
diff --git a/ash/display/cursor_window_controller.cc b/ash/display/cursor_window_controller.cc index c9968fe4..4c654c5 100644 --- a/ash/display/cursor_window_controller.cc +++ b/ash/display/cursor_window_controller.cc
@@ -4,11 +4,14 @@ #include "ash/display/cursor_window_controller.h" +#include "ash/common/ash_constants.h" +#include "ash/common/ash_switches.h" #include "ash/display/mirror_window_controller.h" #include "ash/display/window_tree_host_manager.h" #include "ash/public/cpp/shell_window_ids.h" #include "ash/root_window_controller.h" #include "ash/shell.h" +#include "base/command_line.h" #include "ui/aura/env.h" #include "ui/aura/window_delegate.h" #include "ui/aura/window_event_dispatcher.h" @@ -26,6 +29,12 @@ #include "ui/gfx/image/image_skia_operations.h" namespace ash { +namespace { + +const int kMinLargeCursorSize = 25; +const int kMaxLargeCursorSize = 64; + +} // namespace class CursorWindowDelegate : public aura::WindowDelegate { public: @@ -84,12 +93,33 @@ cursor_type_(ui::kCursorNone), visible_(true), cursor_set_(ui::CURSOR_SET_NORMAL), - delegate_(new CursorWindowDelegate()) {} + large_cursor_size_in_dip_(ash::kDefaultLargeCursorSize), + delegate_(new CursorWindowDelegate()) { + enable_adjustable_large_cursor_ = + base::CommandLine::ForCurrentProcess()->HasSwitch( + ash::switches::kAshAdjustableLargeCursor); +} CursorWindowController::~CursorWindowController() { SetContainer(NULL); } +void CursorWindowController::SetLargeCursorSizeInDip( + int large_cursor_size_in_dip) { + large_cursor_size_in_dip = + std::min(large_cursor_size_in_dip, kMaxLargeCursorSize); + large_cursor_size_in_dip = + std::max(large_cursor_size_in_dip, kMinLargeCursorSize); + + if (large_cursor_size_in_dip_ == large_cursor_size_in_dip) + return; + + large_cursor_size_in_dip_ = large_cursor_size_in_dip; + + if (enable_adjustable_large_cursor_ && display_.is_valid()) + UpdateCursorImage(); +} + void CursorWindowController::SetCursorCompositingEnabled(bool enabled) { if (is_cursor_compositing_enabled_ != enabled) { is_cursor_compositing_enabled_ = enabled; @@ -260,12 +290,31 @@ image_rep.pixel_size(), gfx::ImageSkia::CreateFrom1xBitmap(image_rep.sk_bitmap())); } else { - const gfx::ImageSkiaRep& image_rep = image->GetRepresentation(cursor_scale); + gfx::ImageSkia resized = *image; + + // Rescale cursor size. This is used with the combination of accessibility + // large cursor. We don't need to care about the case where cursor + // compositing is disabled as we always use cursor compositing if + // accessibility large cursor is enabled. + if (enable_adjustable_large_cursor_ && + cursor_set_ == ui::CursorSetType::CURSOR_SET_LARGE && + large_cursor_size_in_dip_ != image->size().width()) { + float rescale = static_cast<float>(large_cursor_size_in_dip_) / + static_cast<float>(image->size().width()); + resized = gfx::ImageSkiaOperations::CreateResizedImage( + *image, skia::ImageOperations::ResizeMethod::RESIZE_GOOD, + gfx::ScaleToCeiledSize(image->size(), rescale)); + hot_point_ = gfx::ScaleToCeiledPoint(hot_point_, rescale); + } + + const gfx::ImageSkiaRep& image_rep = + resized.GetRepresentation(cursor_scale); delegate_->SetCursorImage( - image->size(), + resized.size(), gfx::ImageSkia(gfx::ImageSkiaRep(image_rep.sk_bitmap(), cursor_scale))); hot_point_ = gfx::ConvertPointToDIP(cursor_scale, hot_point_); } + if (cursor_window_) { cursor_window_->SetBounds(gfx::Rect(delegate_->size())); cursor_window_->SchedulePaintInRect(
diff --git a/ash/display/cursor_window_controller.h b/ash/display/cursor_window_controller.h index 667f3f3..ca98f9e5 100644 --- a/ash/display/cursor_window_controller.h +++ b/ash/display/cursor_window_controller.h
@@ -35,6 +35,8 @@ return is_cursor_compositing_enabled_; } + void SetLargeCursorSizeInDip(int large_cursor_size_in_dip); + // Sets cursor compositing mode on/off. void SetCursorCompositingEnabled(bool enabled); @@ -85,6 +87,9 @@ ui::CursorSetType cursor_set_; gfx::Point hot_point_; + bool enable_adjustable_large_cursor_; + int large_cursor_size_in_dip_; + // The display on which the cursor is drawn. // For mirroring mode, the display is always the primary display. display::Display display_;
diff --git a/ash/magnifier/magnification_controller.cc b/ash/magnifier/magnification_controller.cc index 9a32419..aa8295b 100644 --- a/ash/magnifier/magnification_controller.cc +++ b/ash/magnifier/magnification_controller.cc
@@ -15,7 +15,6 @@ #include "ash/host/ash_window_tree_host.h" #include "ash/host/root_window_transformer.h" #include "ash/root_window_controller.h" -#include "ash/screen_util.h" #include "ash/shell.h" #include "base/command_line.h" #include "base/synchronization/waitable_event.h" @@ -440,8 +439,8 @@ if (node_bounds_in_screen.IsEmpty()) return; - gfx::Rect node_bounds_in_root = - ScreenUtil::ConvertRectFromScreen(root_window_, node_bounds_in_screen); + gfx::Rect node_bounds_in_root = node_bounds_in_screen; + ::wm::ConvertRectFromScreen(root_window_, &node_bounds_in_root); if (GetViewportRect().Contains(node_bounds_in_root)) return;
diff --git a/ash/screen_util.cc b/ash/screen_util.cc index 29baadf4..ea11d87 100644 --- a/ash/screen_util.cc +++ b/ash/screen_util.cc
@@ -14,6 +14,7 @@ #include "ui/display/manager/display_manager.h" #include "ui/display/screen.h" #include "ui/gfx/geometry/size_conversions.h" +#include "ui/wm/core/coordinate_conversion.h" namespace ash { @@ -28,37 +29,18 @@ // static gfx::Rect ScreenUtil::GetDisplayBoundsInParent(aura::Window* window) { - return ConvertRectFromScreen( - window->parent(), - display::Screen::GetScreen()->GetDisplayNearestWindow(window).bounds()); + gfx::Rect result = + display::Screen::GetScreen()->GetDisplayNearestWindow(window).bounds(); + ::wm::ConvertRectFromScreen(window->parent(), &result); + return result; } // static gfx::Rect ScreenUtil::GetDisplayWorkAreaBoundsInParent(aura::Window* window) { - return ConvertRectFromScreen(window->parent(), - display::Screen::GetScreen() - ->GetDisplayNearestWindow(window) - .work_area()); + gfx::Rect result = + display::Screen::GetScreen()->GetDisplayNearestWindow(window).work_area(); + ::wm::ConvertRectFromScreen(window->parent(), &result); + return result; } -// static -gfx::Rect ScreenUtil::ConvertRectToScreen(aura::Window* window, - const gfx::Rect& rect) { - gfx::Point point = rect.origin(); - aura::client::GetScreenPositionClient(window->GetRootWindow()) - ->ConvertPointToScreen(window, &point); - return gfx::Rect(point, rect.size()); -} - -// static -gfx::Rect ScreenUtil::ConvertRectFromScreen(aura::Window* window, - const gfx::Rect& rect) { - gfx::Point point = rect.origin(); - aura::client::GetScreenPositionClient(window->GetRootWindow()) - ->ConvertPointFromScreen(window, &point); - return gfx::Rect(point, rect.size()); -} - -// static - } // namespace ash
diff --git a/ash/screen_util.h b/ash/screen_util.h index c0c3ce9..36ca242 100644 --- a/ash/screen_util.h +++ b/ash/screen_util.h
@@ -30,16 +30,6 @@ // Returns the display's work area bounds in parent coordinates. static gfx::Rect GetDisplayWorkAreaBoundsInParent(aura::Window* window); - // TODO(oshima): Move following two to wm/coordinate_conversion.h - // Converts |rect| from |window|'s coordinates to the virtual screen - // coordinates. - static gfx::Rect ConvertRectToScreen(aura::Window* window, - const gfx::Rect& rect); - - // Converts |rect| from virtual screen coordinates to the |window|'s - // coordinates. - static gfx::Rect ConvertRectFromScreen(aura::Window* window, - const gfx::Rect& rect); private: ScreenUtil() {} ~ScreenUtil() {}
diff --git a/ash/screen_util_unittest.cc b/ash/screen_util_unittest.cc index 71f470e..7090555 100644 --- a/ash/screen_util_unittest.cc +++ b/ash/screen_util_unittest.cc
@@ -16,6 +16,7 @@ #include "ui/display/manager/display_manager.h" #include "ui/views/widget/widget.h" #include "ui/views/widget/widget_delegate.h" +#include "ui/wm/core/coordinate_conversion.h" namespace ash { namespace test { @@ -86,23 +87,21 @@ NULL, CurrentContext(), gfx::Rect(610, 10, 100, 100)); secondary->Show(); - EXPECT_EQ("0,0 100x100", - ScreenUtil::ConvertRectFromScreen(primary->GetNativeView(), - gfx::Rect(10, 10, 100, 100)) - .ToString()); - EXPECT_EQ("10,10 100x100", - ScreenUtil::ConvertRectFromScreen(secondary->GetNativeView(), - gfx::Rect(620, 20, 100, 100)) - .ToString()); + gfx::Rect r1(10, 10, 100, 100); + ::wm::ConvertRectFromScreen(primary->GetNativeView(), &r1); + EXPECT_EQ("0,0 100x100", r1.ToString()); - EXPECT_EQ("40,40 100x100", - ScreenUtil::ConvertRectToScreen(primary->GetNativeView(), - gfx::Rect(30, 30, 100, 100)) - .ToString()); - EXPECT_EQ("650,50 100x100", - ScreenUtil::ConvertRectToScreen(secondary->GetNativeView(), - gfx::Rect(40, 40, 100, 100)) - .ToString()); + gfx::Rect r2(620, 20, 100, 100); + ::wm::ConvertRectFromScreen(secondary->GetNativeView(), &r2); + EXPECT_EQ("10,10 100x100", r2.ToString()); + + gfx::Rect r3(30, 30, 100, 100); + ::wm::ConvertRectToScreen(primary->GetNativeView(), &r3); + EXPECT_EQ("40,40 100x100", r3.ToString()); + + gfx::Rect r4(40, 40, 100, 100); + ::wm::ConvertRectToScreen(secondary->GetNativeView(), &r4); + EXPECT_EQ("650,50 100x100", r4.ToString()); } TEST_F(ScreenUtilTest, ShelfDisplayBoundsInUnifiedDesktop) {
diff --git a/ash/shell.cc b/ash/shell.cc index b24e3d73..213fa9b 100644 --- a/ash/shell.cc +++ b/ash/shell.cc
@@ -371,6 +371,11 @@ return new FirstRunHelperImpl; } +void Shell::SetLargeCursorSizeInDip(int large_cursor_size_in_dip) { + window_tree_host_manager_->cursor_window_controller() + ->SetLargeCursorSizeInDip(large_cursor_size_in_dip); +} + void Shell::SetCursorCompositingEnabled(bool enabled) { window_tree_host_manager_->cursor_window_controller() ->SetCursorCompositingEnabled(enabled);
diff --git a/ash/shell.h b/ash/shell.h index 8731782e..1e547ab 100644 --- a/ash/shell.h +++ b/ash/shell.h
@@ -388,6 +388,8 @@ // returned object. ash::FirstRunHelper* CreateFirstRunHelper(); + void SetLargeCursorSizeInDip(int large_cursor_size_in_dip); + // Toggles cursor compositing on/off. Native cursor is disabled when cursor // compositing is enabled, and vice versa. void SetCursorCompositingEnabled(bool enabled);
diff --git a/ash/shell/panel_window.cc b/ash/shell/panel_window.cc index e10c68c..5566a024 100644 --- a/ash/shell/panel_window.cc +++ b/ash/shell/panel_window.cc
@@ -6,12 +6,12 @@ #include "ash/common/wm/panels/panel_frame_view.h" #include "ash/public/cpp/window_properties.h" -#include "ash/screen_util.h" #include "ash/shell.h" #include "base/strings/utf_string_conversions.h" #include "ui/aura/window.h" #include "ui/gfx/canvas.h" #include "ui/views/widget/widget.h" +#include "ui/wm/core/coordinate_conversion.h" namespace { const int kMinWidth = 100; @@ -44,8 +44,7 @@ params().bounds.set_width(kDefaultWidth); if (params().bounds.height() == 0) params().bounds.set_height(kDefaultHeight); - params().bounds = ScreenUtil::ConvertRectToScreen( - Shell::GetTargetRootWindow(), params().bounds); + ::wm::ConvertRectToScreen(Shell::GetTargetRootWindow(), ¶ms().bounds); widget->Init(params()); widget->GetNativeView()->SetName(name_);
diff --git a/ash/wm/drag_window_controller.cc b/ash/wm/drag_window_controller.cc index 698f0d9..365ce2f 100644 --- a/ash/wm/drag_window_controller.cc +++ b/ash/wm/drag_window_controller.cc
@@ -8,7 +8,6 @@ #include "ash/display/window_tree_host_manager.h" #include "ash/public/cpp/shell_window_ids.h" -#include "ash/screen_util.h" #include "ash/shell.h" #include "ash/wm/window_util.h" #include "base/memory/ptr_util.h" @@ -24,6 +23,7 @@ #include "ui/compositor/scoped_layer_animation_settings.h" #include "ui/views/view.h" #include "ui/views/widget/widget.h" +#include "ui/wm/core/coordinate_conversion.h" #include "ui/wm/core/shadow_types.h" #include "ui/wm/core/window_util.h" @@ -59,8 +59,8 @@ if (!drag_window_) CreateDragWindow(original_window, bounds_in_screen); - gfx::Rect bounds_in_root = ScreenUtil::ConvertRectFromScreen( - drag_window_->parent(), bounds_in_screen); + gfx::Rect bounds_in_root = bounds_in_screen; + ::wm::ConvertRectFromScreen(drag_window_->parent(), &bounds_in_root); drag_window_->SetBounds(bounds_in_root); if (root_bounds_in_screen.Contains(drag_location_in_screen)) { SetOpacity(original_window, 1.f);
diff --git a/ash/wm/drag_window_resizer.cc b/ash/wm/drag_window_resizer.cc index d4cf8aae..0ccaf949 100644 --- a/ash/wm/drag_window_resizer.cc +++ b/ash/wm/drag_window_resizer.cc
@@ -8,7 +8,6 @@ #include "ash/common/wm/window_state.h" #include "ash/common/wm_window.h" #include "ash/display/mouse_cursor_event_filter.h" -#include "ash/screen_util.h" #include "ash/shell.h" #include "ash/wm/drag_window_controller.h" #include "ash/wm/window_util.h" @@ -91,8 +90,8 @@ if (bounds.height() > size.height()) bounds.set_height(size.height()); - gfx::Rect dst_bounds = - ScreenUtil::ConvertRectToScreen(GetAuraTarget()->parent(), bounds); + gfx::Rect dst_bounds = bounds; + ::wm::ConvertRectToScreen(GetAuraTarget()->parent(), &dst_bounds); // Adjust the position so that the cursor is on the window. if (!dst_bounds.Contains(last_mouse_location_in_screen)) { @@ -144,8 +143,8 @@ if (!drag_window_controller_) drag_window_controller_.reset(new DragWindowController(GetAuraTarget())); - const gfx::Rect bounds_in_screen = ScreenUtil::ConvertRectToScreen( - GetAuraTarget()->parent(), bounds_in_parent); + gfx::Rect bounds_in_screen = bounds_in_parent; + ::wm::ConvertRectToScreen(GetAuraTarget()->parent(), &bounds_in_screen); gfx::Rect root_bounds_in_screen = GetAuraTarget()->GetRootWindow()->GetBoundsInScreen();
diff --git a/ash/wm/overview/window_selector_unittest.cc b/ash/wm/overview/window_selector_unittest.cc index d3cc05a..61f5d17 100644 --- a/ash/wm/overview/window_selector_unittest.cc +++ b/ash/wm/overview/window_selector_unittest.cc
@@ -31,7 +31,6 @@ #include "ash/drag_drop/drag_drop_controller.h" #include "ash/public/cpp/shell_window_ids.h" #include "ash/root_window_controller.h" -#include "ash/screen_util.h" #include "ash/shell.h" #include "ash/test/ash_test_base.h" #include "ash/test/shelf_view_test_api.h" @@ -71,6 +70,7 @@ #include "ui/views/controls/label.h" #include "ui/views/widget/native_widget_aura.h" #include "ui/views/widget/widget_delegate.h" +#include "ui/wm/core/coordinate_conversion.h" #include "ui/wm/core/window_util.h" #include "ui/wm/public/activation_delegate.h" @@ -199,8 +199,9 @@ } gfx::Rect GetTransformedBounds(aura::Window* window) { - gfx::RectF bounds(ScreenUtil::ConvertRectToScreen( - window->parent(), window->layer()->bounds())); + gfx::Rect bounds_in_screen = window->layer()->bounds(); + ::wm::ConvertRectToScreen(window->parent(), &bounds_in_screen); + gfx::RectF bounds(bounds_in_screen); gfx::Transform transform(gfx::TransformAboutPivot( gfx::ToFlooredPoint(bounds.origin()), window->layer()->transform())); transform.TransformRect(&bounds); @@ -208,8 +209,9 @@ } gfx::Rect GetTransformedTargetBounds(aura::Window* window) { - gfx::RectF bounds(ScreenUtil::ConvertRectToScreen( - window->parent(), window->layer()->GetTargetBounds())); + gfx::Rect bounds_in_screen = window->layer()->GetTargetBounds(); + ::wm::ConvertRectToScreen(window->parent(), &bounds_in_screen); + gfx::RectF bounds(bounds_in_screen); gfx::Transform transform( gfx::TransformAboutPivot(gfx::ToFlooredPoint(bounds.origin()), window->layer()->GetTargetTransform()));
diff --git a/ash/wm/window_animations.cc b/ash/wm/window_animations.cc index 190a8e8..c36a0c6 100644 --- a/ash/wm/window_animations.cc +++ b/ash/wm/window_animations.cc
@@ -13,7 +13,6 @@ #include "ash/common/wm/window_animation_types.h" #include "ash/common/wm/workspace_controller.h" #include "ash/common/wm_window.h" -#include "ash/screen_util.h" #include "ash/wm/window_util.h" #include "base/i18n/rtl.h" #include "base/logging.h" @@ -35,6 +34,7 @@ #include "ui/display/screen.h" #include "ui/gfx/interpolated_transform.h" #include "ui/gfx/transform.h" +#include "ui/wm/core/coordinate_conversion.h" #include "ui/wm/core/window_util.h" namespace ash { @@ -93,8 +93,7 @@ // moved while the window was minimized. gfx::Rect bounds = window->bounds(); gfx::Rect target_bounds = GetMinimizeAnimationTargetBoundsInScreen(window); - target_bounds = - ScreenUtil::ConvertRectFromScreen(window->parent(), target_bounds); + ::wm::ConvertRectFromScreen(window->parent(), &target_bounds); float scale_x = static_cast<float>(target_bounds.width()) / bounds.width(); float scale_y = static_cast<float>(target_bounds.height()) / bounds.height();
diff --git a/build/config/linux/gtk/gtk.gni b/build/config/linux/gtk/gtk.gni index ae4746e4..b3fd973d 100644 --- a/build/config/linux/gtk/gtk.gni +++ b/build/config/linux/gtk/gtk.gni
@@ -8,5 +8,5 @@ declare_args() { # Whether to compile agains GTKv3 instead of GTKv2. - use_gtk3 = true + use_gtk3 = false }
diff --git a/cc/scheduler/begin_frame_source_unittest.cc b/cc/scheduler/begin_frame_source_unittest.cc index 3fba80b..bce3439 100644 --- a/cc/scheduler/begin_frame_source_unittest.cc +++ b/cc/scheduler/begin_frame_source_unittest.cc
@@ -883,5 +883,30 @@ source_->RemoveObserver(obs_.get()); } +// https://crbug.com/690127: Duplicate BeginFrame caused DCHECK crash. +TEST_F(ExternalBeginFrameSourceTest, OnBeginFrameChecksBeginFrameContinuity) { + EXPECT_BEGIN_FRAME_SOURCE_PAUSED(*obs_, false); + EXPECT_CALL((*client_), OnNeedsBeginFrames(true)).Times(1); + source_->AddObserver(obs_.get()); + + BeginFrameArgs args = CreateBeginFrameArgsForTesting( + BEGINFRAME_FROM_HERE, 0, 2, base::TimeTicks::FromInternalValue(10000)); + EXPECT_BEGIN_FRAME_ARGS_USED(*obs_, args); + source_->OnBeginFrame(args); + + // Providing same args again to OnBeginFrame() should not notify observer. + EXPECT_CALL((*client_), OnDidFinishFrame(BeginFrameAck(0, 2, 0, 0, false))) + .Times(1); + source_->OnBeginFrame(args); + + // Providing same args through a different ExternalBeginFrameSource also does + // not notify observer. + EXPECT_BEGIN_FRAME_SOURCE_PAUSED(*obs_, false); + EXPECT_CALL((*client_), OnNeedsBeginFrames(true)).Times(1); + ExternalBeginFrameSource source2(client_.get()); + source2.AddObserver(obs_.get()); + source2.OnBeginFrame(args); +} + } // namespace } // namespace cc
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java b/chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java index d3bbe34441..2de24ac 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java
@@ -6,7 +6,6 @@ import android.app.Activity; import android.content.Context; -import android.content.Intent; import android.view.View; import com.google.ipc.invalidation.external.client.android.service.AndroidLogger; @@ -210,7 +209,7 @@ AccountsChangedReceiver.addObserver( new AccountsChangedReceiver.AccountsChangedObserver() { @Override - public void onAccountsChanged(Context context, Intent intent) { + public void onAccountsChanged() { ThreadUtils.runOnUiThread(new Runnable() { @Override public void run() {
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/services/AccountsChangedReceiver.java b/chrome/android/java/src/org/chromium/chrome/browser/services/AccountsChangedReceiver.java index 84293f94..559a4a72 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/services/AccountsChangedReceiver.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/services/AccountsChangedReceiver.java
@@ -29,45 +29,48 @@ /** * This receiver is notified when accounts are added, accounts are removed, or * an account's credentials (saved password, etc) are changed. + * All public methods must be called from the UI thread. */ public class AccountsChangedReceiver extends BroadcastReceiver { private static final String TAG = "AccountsChangedRx"; /** - * Receives a callback whenever {@link AccountManager#LOGIN_ACCOUNTS_CHANGED_ACTION} is - * broadcasted. Use {@link #addObserver} and {@link #removeObserver} to update registrations. + * Observer that receives account change notifications from {@link AccountManager}. + * Use {@link #addObserver} and {@link #removeObserver} to update registrations. * * The callback will only ever be called after the browser process has been initialized. */ public interface AccountsChangedObserver { /** - * Called when {@link AccountManager#LOGIN_ACCOUNTS_CHANGED_ACTION} is triggered. - * @param context the application context. - * @param intent the broadcasted intent. + * Called on every change to the accounts. */ - void onAccountsChanged(Context context, Intent intent); + void onAccountsChanged(); } private static ObserverList<AccountsChangedObserver> sObservers = new ObserverList<>(); /** - * Adds an observer to the {@link AccountManager#LOGIN_ACCOUNTS_CHANGED_ACTION} broadcasts. + * Adds an observer to receive accounts change notifications from {@link AccountManager}. * @param observer the observer to add. */ public static void addObserver(AccountsChangedObserver observer) { + ThreadUtils.assertOnUiThread(); sObservers.addObserver(observer); } /** - * Removes an observer from the {@link AccountManager#LOGIN_ACCOUNTS_CHANGED_ACTION} broadcasts. + * Removes an observer that was previously added using {@link #addObserver}. * @param observer the observer to remove. */ public static void removeObserver(AccountsChangedObserver observer) { + ThreadUtils.assertOnUiThread(); sObservers.removeObserver(observer); } @Override public void onReceive(Context context, final Intent intent) { + if (!AccountManager.LOGIN_ACCOUNTS_CHANGED_ACTION.equals(intent.getAction())) return; + final Context appContext = context.getApplicationContext(); AsyncTask<Void, Void, Void> task = new AsyncTask<Void, Void, Void>() { @Override @@ -78,15 +81,13 @@ @Override protected void onPostExecute(Void result) { - continueHandleAccountChangeIfNeeded(appContext, intent); + continueHandleAccountChangeIfNeeded(appContext); } }; task.execute(); } - private void continueHandleAccountChangeIfNeeded(final Context context, final Intent intent) { - if (!AccountManager.LOGIN_ACCOUNTS_CHANGED_ACTION.equals(intent.getAction())) return; - + private void continueHandleAccountChangeIfNeeded(final Context context) { AccountTrackerService.get(context).invalidateAccountSeedStatus( false /* don't refresh right now */); boolean isChromeVisible = ApplicationStatus.hasVisibleActivities(); @@ -96,7 +97,7 @@ // Notify SigninHelper of changed accounts (via shared prefs). SigninHelper.markAccountsChangedPref(context); } - notifyAccountsChangedOnBrowserStartup(context, intent); + notifyAccountsChangedOnBrowserStartup(context); } @SuppressFBWarnings("DM_EXIT") @@ -128,13 +129,12 @@ } } - private static void notifyAccountsChangedOnBrowserStartup( - final Context context, final Intent intent) { + private static void notifyAccountsChangedOnBrowserStartup(final Context context) { StartupCallback notifyAccountsChangedCallback = new StartupCallback() { @Override public void onSuccess(boolean alreadyStarted) { for (AccountsChangedObserver observer : sObservers) { - observer.onAccountsChanged(context, intent); + observer.onAccountsChanged(); } }
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index d0b2cc30..7aa54c0 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd
@@ -14876,6 +14876,12 @@ </message> </if> + <message name="IDS_FLAGS_ENABLE_ADJUSTABLE_LARGE_CURSOR_NAME" desc="Name of the flag that enables adjustable large cursor."> + Enable adjustable large cursor + </message> + <message name="IDS_FLAGS_ENABLE_ADJUSTABLE_LARGE_CURSOR_DESCRIPTION" desc="Description of the flag that enables adjustable large cursor."> + Make size of accessibility large cursor adjustable. + </message> </messages> </release> </grit>
diff --git a/chrome/app/settings_strings.grdp b/chrome/app/settings_strings.grdp index 67c203e..9f33dfb8 100644 --- a/chrome/app/settings_strings.grdp +++ b/chrome/app/settings_strings.grdp
@@ -145,6 +145,9 @@ <message name="IDS_SETTINGS_LARGE_MOUSE_CURSOR_LABEL" desc="Label for checkbox which enables showing a larger mouse cursor than normal."> Show large mouse cursor </message> + <message name="IDS_SETTINGS_LARGE_MOUSE_CURSOR_SIZE_LABEL" desc="Label for a slider which changes the size of large mouse cursor."> + Cursor size: + </message> <message name="IDS_SETTINGS_HIGH_CONTRAST_LABEL" desc="Label for checkbox which enables high-contrast UI."> Use high contrast mode </message>
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc index c9d5066b..7910bd9 100644 --- a/chrome/browser/about_flags.cc +++ b/chrome/browser/about_flags.cc
@@ -2279,6 +2279,10 @@ IDS_FLAGS_ENABLE_TOUCH_SUPPORT_FOR_SCREEN_MAGNIFIER_DESCRIPTION, kOsCrOS, SINGLE_VALUE_TYPE( chromeos::switches::kEnableTouchSupportForScreenMagnifier)}, + {"ash-adjustable-large-cursor", + IDS_FLAGS_ENABLE_ADJUSTABLE_LARGE_CURSOR_NAME, + IDS_FLAGS_ENABLE_ADJUSTABLE_LARGE_CURSOR_DESCRIPTION, kOsCrOS, + SINGLE_VALUE_TYPE(ash::switches::kAshAdjustableLargeCursor)} #endif // OS_CHROMEOS // NOTE: Adding new command-line switches requires adding corresponding
diff --git a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc index 64a954a..70a3d60 100644 --- a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc +++ b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
@@ -8,6 +8,7 @@ #include "base/run_loop.h" #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" +#include "base/test/scoped_feature_list.h" #include "base/time/time.h" #include "chrome/browser/autofill/personal_data_manager_factory.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h" @@ -27,6 +28,7 @@ #include "chrome/browser/safe_browsing/safe_browsing_service.h" #include "chrome/browser/storage/durable_storage_permission_context.h" #include "chrome/browser/translate/language_model_factory.h" +#include "chrome/common/chrome_features.h" #include "chrome/common/pref_names.h" #include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_profile.h" @@ -570,11 +572,18 @@ return autoblocker_->RecordIgnore(url, permission); } - bool ShouldChangeDismissalToBlock(const GURL& url, - ContentSettingsType permission) { + bool RecordDismissAndEmbargo(const GURL& url, + ContentSettingsType permission) { return autoblocker_->RecordDismissAndEmbargo(url, permission); } + void CheckEmbargo(const GURL& url, + ContentSettingsType permission, + ContentSetting expected_setting) { + EXPECT_EQ(expected_setting, + autoblocker_->GetEmbargoResult(permission, url).content_setting); + } + private: PermissionDecisionAutoBlocker* autoblocker_; @@ -1571,6 +1580,8 @@ std::unique_ptr<BrowsingDataFilterBuilder> filter_builder_2( BrowsingDataFilterBuilder::Create(BrowsingDataFilterBuilder::BLACKLIST)); filter_builder_2->AddRegisterableDomain(kTestRegisterableDomain1); + base::test::ScopedFeatureList feature_list; + feature_list.InitWithFeatures({features::kBlockPromptsIfDismissedOften}, {}); { // Test REMOVE_HISTORY. @@ -1580,12 +1591,20 @@ CONTENT_SETTINGS_TYPE_GEOLOCATION)); EXPECT_EQ(1, tester.RecordIgnore(kOrigin1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); - tester.ShouldChangeDismissalToBlock(kOrigin1, - CONTENT_SETTINGS_TYPE_MIDI_SYSEX); + EXPECT_FALSE(tester.RecordDismissAndEmbargo( + kOrigin1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); EXPECT_EQ(1, tester.RecordIgnore(kOrigin2, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE)); - tester.ShouldChangeDismissalToBlock(kOrigin2, - CONTENT_SETTINGS_TYPE_NOTIFICATIONS); + tester.CheckEmbargo(kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS, + CONTENT_SETTING_ASK); + EXPECT_FALSE(tester.RecordDismissAndEmbargo( + kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); + EXPECT_FALSE(tester.RecordDismissAndEmbargo( + kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); + EXPECT_TRUE(tester.RecordDismissAndEmbargo( + kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); + tester.CheckEmbargo(kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS, + CONTENT_SETTING_BLOCK); BlockUntilOriginDataRemoved(AnHourAgo(), base::Time::Max(), BrowsingDataRemover::REMOVE_SITE_USAGE_DATA, @@ -1600,8 +1619,10 @@ CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); EXPECT_EQ(1, tester.GetIgnoreCount( kOrigin2, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE)); - EXPECT_EQ(1, tester.GetDismissCount( - kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); + EXPECT_EQ(3, tester.GetDismissCount(kOrigin2, + CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); + tester.CheckEmbargo(kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS, + CONTENT_SETTING_BLOCK); BlockUntilBrowsingDataRemoved(AnHourAgo(), base::Time::Max(), BrowsingDataRemover::REMOVE_HISTORY, false); @@ -1617,6 +1638,8 @@ kOrigin2, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE)); EXPECT_EQ(0, tester.GetDismissCount( kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); + tester.CheckEmbargo(kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS, + CONTENT_SETTING_ASK); } { // Test REMOVE_SITE_DATA. @@ -1626,12 +1649,14 @@ CONTENT_SETTINGS_TYPE_GEOLOCATION)); EXPECT_EQ(1, tester.RecordIgnore(kOrigin1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); - tester.ShouldChangeDismissalToBlock(kOrigin1, - CONTENT_SETTINGS_TYPE_MIDI_SYSEX); + EXPECT_FALSE(tester.RecordDismissAndEmbargo( + kOrigin1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); + tester.CheckEmbargo(kOrigin1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX, + CONTENT_SETTING_ASK); EXPECT_EQ(1, tester.RecordIgnore(kOrigin2, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE)); - tester.ShouldChangeDismissalToBlock(kOrigin2, - CONTENT_SETTINGS_TYPE_NOTIFICATIONS); + EXPECT_FALSE(tester.RecordDismissAndEmbargo( + kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); BlockUntilOriginDataRemoved(AnHourAgo(), base::Time::Max(), BrowsingDataRemover::REMOVE_SITE_USAGE_DATA, @@ -1649,6 +1674,15 @@ EXPECT_EQ(0, tester.GetDismissCount( kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); + EXPECT_FALSE(tester.RecordDismissAndEmbargo( + kOrigin1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); + EXPECT_TRUE(tester.RecordDismissAndEmbargo( + kOrigin1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); + EXPECT_EQ( + 3, tester.GetDismissCount(kOrigin1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); + tester.CheckEmbargo(kOrigin1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX, + CONTENT_SETTING_BLOCK); + BlockUntilBrowsingDataRemoved(AnHourAgo(), base::Time::Max(), BrowsingDataRemover::REMOVE_SITE_USAGE_DATA, false); @@ -1664,6 +1698,8 @@ kOrigin2, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE)); EXPECT_EQ(0, tester.GetDismissCount( kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); + tester.CheckEmbargo(kOrigin1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX, + CONTENT_SETTING_ASK); } }
diff --git a/chrome/browser/chromeos/accessibility/accessibility_manager.cc b/chrome/browser/chromeos/accessibility/accessibility_manager.cc index a392a26..6cc9829 100644 --- a/chrome/browser/chromeos/accessibility/accessibility_manager.cc +++ b/chrome/browser/chromeos/accessibility/accessibility_manager.cc
@@ -12,6 +12,7 @@ #include "ash/autoclick/autoclick_controller.h" #include "ash/autoclick/mus/public/interfaces/autoclick.mojom.h" +#include "ash/common/ash_constants.h" #include "ash/common/session/session_state_delegate.h" #include "ash/common/shelf/shelf_layout_manager.h" #include "ash/common/shelf/wm_shelf.h" @@ -247,6 +248,7 @@ select_to_speak_pref_handler_(prefs::kAccessibilitySelectToSpeakEnabled), switch_access_pref_handler_(prefs::kAccessibilitySwitchAccessEnabled), large_cursor_enabled_(false), + large_cursor_size_in_dip_(ash::kDefaultLargeCursorSize), sticky_keys_enabled_(false), spoken_feedback_enabled_(false), high_contrast_enabled_(false), @@ -385,10 +387,16 @@ const bool enabled = profile_->GetPrefs()->GetBoolean(prefs::kAccessibilityLargeCursorEnabled); - if (large_cursor_enabled_ == enabled) + const int large_cursor_size_in_dip = + profile_->GetPrefs()->GetInteger(prefs::kAccessibilityLargeCursorDipSize); + + if (large_cursor_enabled_ == enabled && + large_cursor_size_in_dip_ == large_cursor_size_in_dip) { return; + } large_cursor_enabled_ = enabled; + large_cursor_size_in_dip_ = large_cursor_size_in_dip; AccessibilityStatusEventDetails details(ACCESSIBILITY_TOGGLE_LARGE_CURSOR, enabled, ash::A11Y_NOTIFICATION_NONE); @@ -397,6 +405,7 @@ ash::Shell::GetInstance()->cursor_manager()->SetCursorSet( enabled ? ui::CURSOR_SET_LARGE : ui::CURSOR_SET_NORMAL); + ash::Shell::GetInstance()->SetLargeCursorSizeInDip(large_cursor_size_in_dip); ash::Shell::GetInstance()->SetCursorCompositingEnabled( ShouldEnableCursorCompositing()); } @@ -1033,6 +1042,10 @@ base::Bind(&AccessibilityManager::UpdateLargeCursorFromPref, base::Unretained(this))); pref_change_registrar_->Add( + prefs::kAccessibilityLargeCursorDipSize, + base::Bind(&AccessibilityManager::UpdateLargeCursorFromPref, + base::Unretained(this))); + pref_change_registrar_->Add( prefs::kAccessibilityStickyKeysEnabled, base::Bind(&AccessibilityManager::UpdateStickyKeysFromPref, base::Unretained(this)));
diff --git a/chrome/browser/chromeos/accessibility/accessibility_manager.h b/chrome/browser/chromeos/accessibility/accessibility_manager.h index da1ecde8..c224c732 100644 --- a/chrome/browser/chromeos/accessibility/accessibility_manager.h +++ b/chrome/browser/chromeos/accessibility/accessibility_manager.h
@@ -371,6 +371,7 @@ PrefHandler switch_access_pref_handler_; bool large_cursor_enabled_; + int large_cursor_size_in_dip_; bool sticky_keys_enabled_; bool spoken_feedback_enabled_; bool high_contrast_enabled_;
diff --git a/chrome/browser/chromeos/accessibility/magnification_controller_browsertest.cc b/chrome/browser/chromeos/accessibility/magnification_controller_browsertest.cc index a1383fb3..62230e7b5 100644 --- a/chrome/browser/chromeos/accessibility/magnification_controller_browsertest.cc +++ b/chrome/browser/chromeos/accessibility/magnification_controller_browsertest.cc
@@ -5,7 +5,6 @@ #include <string> #include "ash/magnifier/magnification_controller.h" -#include "ash/screen_util.h" #include "ash/shell.h" #include "base/command_line.h" #include "base/macros.h" @@ -21,6 +20,7 @@ #include "content/public/browser/web_contents.h" #include "content/public/test/browser_test_utils.h" #include "testing/gtest/include/gtest/gtest.h" +#include "ui/wm/core/coordinate_conversion.h" namespace chromeos { @@ -151,9 +151,8 @@ origin.Offset(view_bounds_in_screen.x(), view_bounds_in_screen.y()); gfx::Rect rect_in_screen(origin.x(), origin.y(), rect.width(), rect.height()); - - return ash::ScreenUtil::ConvertRectFromScreen(GetRootWindow(), - rect_in_screen); + ::wm::ConvertRectFromScreen(GetRootWindow(), &rect_in_screen); + return rect_in_screen; } void SetFocusOnElement(const std::string& element_id) {
diff --git a/chrome/browser/chromeos/preferences.cc b/chrome/browser/chromeos/preferences.cc index 1103be6..a5eeea9f 100644 --- a/chrome/browser/chromeos/preferences.cc +++ b/chrome/browser/chromeos/preferences.cc
@@ -8,6 +8,7 @@ #include "ash/autoclick/autoclick_controller.h" #include "ash/common/accessibility_types.h" +#include "ash/common/ash_constants.h" #include "ash/common/wm_shell.h" #include "ash/shell.h" #include "base/command_line.h" @@ -154,6 +155,8 @@ prefs::kAccessibilityLargeCursorEnabled, false, user_prefs::PrefRegistrySyncable::SYNCABLE_PREF); + registry->RegisterIntegerPref(prefs::kAccessibilityLargeCursorDipSize, + ash::kDefaultLargeCursorSize); registry->RegisterBooleanPref(prefs::kAccessibilitySpokenFeedbackEnabled, false); registry->RegisterBooleanPref(
diff --git a/chrome/browser/extensions/api/settings_private/prefs_util.cc b/chrome/browser/extensions/api/settings_private/prefs_util.cc index 4df3bd5..e32baa1 100644 --- a/chrome/browser/extensions/api/settings_private/prefs_util.cc +++ b/chrome/browser/extensions/api/settings_private/prefs_util.cc
@@ -234,6 +234,8 @@ settings_private::PrefType::PREF_TYPE_BOOLEAN; (*s_whitelist)[::prefs::kAccessibilityLargeCursorEnabled] = settings_private::PrefType::PREF_TYPE_BOOLEAN; + (*s_whitelist)[::prefs::kAccessibilityLargeCursorDipSize] = + settings_private::PrefType::PREF_TYPE_NUMBER; (*s_whitelist)[::prefs::kAccessibilityScreenMagnifierEnabled] = settings_private::PrefType::PREF_TYPE_BOOLEAN; (*s_whitelist)[::prefs::kAccessibilitySelectToSpeakEnabled] =
diff --git a/chrome/browser/permissions/permission_context_base.cc b/chrome/browser/permissions/permission_context_base.cc index c8b3596..de06305 100644 --- a/chrome/browser/permissions/permission_context_base.cc +++ b/chrome/browser/permissions/permission_context_base.cc
@@ -23,7 +23,6 @@ #include "chrome/browser/permissions/permission_request_impl.h" #include "chrome/browser/permissions/permission_request_manager.h" #include "chrome/browser/permissions/permission_uma_util.h" -#include "chrome/browser/permissions/permission_util.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/safe_browsing/safe_browsing_service.h" #include "chrome/common/chrome_features.h" @@ -49,12 +48,6 @@ const char PermissionContextBase::kPermissionsKillSwitchBlockedValue[] = "blocked"; -PermissionResult::PermissionResult(ContentSetting cs, - PermissionStatusSource pss) - : content_setting(cs), source(pss) {} - -PermissionResult::~PermissionResult() {} - PermissionContextBase::PermissionContextBase( Profile* profile, const ContentSettingsType content_settings_type) @@ -80,20 +73,6 @@ const BrowserPermissionCallback& callback) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - // First check if this permission has been disabled. - if (IsPermissionKillSwitchOn()) { - // Log to the developer console. - web_contents->GetMainFrame()->AddMessageToConsole( - content::CONSOLE_MESSAGE_LEVEL_INFO, - base::StringPrintf( - "%s permission has been blocked.", - PermissionUtil::GetPermissionString(content_settings_type_) - .c_str())); - // The kill switch is enabled for this permission; Block all requests. - callback.Run(CONTENT_SETTING_BLOCK); - return; - } - GURL requesting_origin = requesting_frame.GetOrigin(); GURL embedding_origin = web_contents->GetLastCommittedURL().GetOrigin(); @@ -118,15 +97,28 @@ if (result.content_setting == CONTENT_SETTING_ALLOW || result.content_setting == CONTENT_SETTING_BLOCK) { + if (result.source == PermissionStatusSource::KILL_SWITCH) { + // Block the request and log to the developer console. + web_contents->GetMainFrame()->AddMessageToConsole( + content::CONSOLE_MESSAGE_LEVEL_INFO, + base::StringPrintf( + "%s permission has been blocked.", + PermissionUtil::GetPermissionString(content_settings_type_) + .c_str())); + callback.Run(CONTENT_SETTING_BLOCK); + return; + } + + // If we are under embargo, record the embargo reason for which we have + // suppressed the prompt. + PermissionUmaUtil::RecordEmbargoPromptSuppressionFromSource(result.source); NotifyPermissionSet(id, requesting_origin, embedding_origin, callback, false /* persist */, result.content_setting); return; } // Asynchronously check whether the origin should be blocked from making this - // permission request. It may be on the Safe Browsing API blacklist, or it may - // have been dismissed too many times in a row. If the origin is allowed to - // request, that request will be made to ContinueRequestPermission(). + // permission request, e.g. it may be on the Safe Browsing API blacklist. PermissionDecisionAutoBlocker::GetForProfile(profile_)->UpdateEmbargoedStatus( content_settings_type_, requesting_origin, web_contents, base::Bind(&PermissionContextBase::ContinueRequestPermission, @@ -150,15 +142,21 @@ "%s permission has been auto-blocked.", PermissionUtil::GetPermissionString(content_settings_type_) .c_str())); - // Permission has been automatically blocked. - PermissionUmaUtil::RecordPermissionEmbargoStatus( + // Permission has been automatically blocked. Record that the prompt was + // suppressed and that we hit the blacklist. + PermissionUmaUtil::RecordEmbargoPromptSuppression( + PermissionEmbargoStatus::PERMISSIONS_BLACKLISTING); + PermissionUmaUtil::RecordEmbargoStatus( PermissionEmbargoStatus::PERMISSIONS_BLACKLISTING); callback.Run(CONTENT_SETTING_BLOCK); return; } + // We are going to show a prompt now. PermissionUmaUtil::PermissionRequested( content_settings_type_, requesting_origin, embedding_origin, profile_); + PermissionUmaUtil::RecordEmbargoPromptSuppression( + PermissionEmbargoStatus::NOT_EMBARGOED); DecidePermission(web_contents, id, requesting_origin, embedding_origin, user_gesture, callback); @@ -167,13 +165,10 @@ PermissionResult PermissionContextBase::GetPermissionStatus( const GURL& requesting_origin, const GURL& embedding_origin) const { - // TODO(raymes): Ensure we return appropriate decision reasons in the - // PermissionResult. We should add these as each is needed. - // If the permission has been disabled through Finch, block all requests. if (IsPermissionKillSwitchOn()) { return PermissionResult(CONTENT_SETTING_BLOCK, - PermissionStatusSource::UNSPECIFIED); + PermissionStatusSource::KILL_SWITCH); } if (IsRestrictedToSecureOrigins() && @@ -184,19 +179,20 @@ ContentSetting content_setting = GetPermissionStatusInternal(requesting_origin, embedding_origin); - if (content_setting == CONTENT_SETTING_ASK && - PermissionDecisionAutoBlocker::GetForProfile(profile_)->IsUnderEmbargo( - content_settings_type_, requesting_origin)) { - return PermissionResult(CONTENT_SETTING_BLOCK, - PermissionStatusSource::UNSPECIFIED); + if (content_setting == CONTENT_SETTING_ASK) { + PermissionResult result = + PermissionDecisionAutoBlocker::GetForProfile(profile_) + ->GetEmbargoResult(content_settings_type_, requesting_origin); + DCHECK(result.content_setting == CONTENT_SETTING_ASK || + result.content_setting == CONTENT_SETTING_BLOCK); + return result; } return PermissionResult(content_setting, PermissionStatusSource::UNSPECIFIED); } -void PermissionContextBase::ResetPermission( - const GURL& requesting_origin, - const GURL& embedding_origin) { +void PermissionContextBase::ResetPermission(const GURL& requesting_origin, + const GURL& embedding_origin) { HostContentSettingsMapFactory::GetForProfile(profile_) ->SetContentSettingDefaultScope(requesting_origin, embedding_origin, content_settings_storage_type(), @@ -324,7 +320,7 @@ embargo_status = PermissionEmbargoStatus::REPEATED_DISMISSALS; } } - PermissionUmaUtil::RecordPermissionEmbargoStatus(embargo_status); + PermissionUmaUtil::RecordEmbargoStatus(embargo_status); } NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
diff --git a/chrome/browser/permissions/permission_context_base.h b/chrome/browser/permissions/permission_context_base.h index 1957a44..cf995ef 100644 --- a/chrome/browser/permissions/permission_context_base.h +++ b/chrome/browser/permissions/permission_context_base.h
@@ -12,6 +12,7 @@ #include "base/memory/weak_ptr.h" #include "build/build_config.h" #include "chrome/browser/permissions/permission_request.h" +#include "chrome/browser/permissions/permission_util.h" #include "components/content_settings/core/common/content_settings.h" #include "components/content_settings/core/common/content_settings_types.h" #include "components/keyed_service/core/keyed_service.h" @@ -29,32 +30,6 @@ using BrowserPermissionCallback = base::Callback<void(ContentSetting)>; -// Identifies the source or reason for a permission status being returned. -// TODO(raymes): Add more reasons here and return them correctly. -enum class PermissionStatusSource { - // The status is the result of being blocked due to the user dismissing a - // permission prompt multiple times. - MULTIPLE_DISMISSALS, - - // The status is the resultof being blocked because the permission is on the - // safe browsing blacklist. - SAFE_BROWSING_BLACKLIST, - - // The reason for the status is not specified. Avoid returning this value. It - // only exists until code has been added to correctly return a source in all - // cases. - UNSPECIFIED -}; - -struct PermissionResult { - PermissionResult(ContentSetting content_setting, - PermissionStatusSource source); - ~PermissionResult(); - - ContentSetting content_setting; - PermissionStatusSource source; -}; - // This base class contains common operations for granting permissions. // It offers the following functionality: // - Creates a permission request when needed.
diff --git a/chrome/browser/permissions/permission_context_base_unittest.cc b/chrome/browser/permissions/permission_context_base_unittest.cc index 9dd2f2a..a5c0a6c 100644 --- a/chrome/browser/permissions/permission_context_base_unittest.cc +++ b/chrome/browser/permissions/permission_context_base_unittest.cc
@@ -317,6 +317,9 @@ permission_context.GetContentSettingFromMap(url, url)); } + histograms.ExpectUniqueSample( + "Permissions.AutoBlocker.EmbargoPromptSuppression", + PermissionEmbargoStatus::NOT_EMBARGOED, 1); histograms.ExpectUniqueSample("Permissions.AutoBlocker.EmbargoStatus", PermissionEmbargoStatus::NOT_EMBARGOED, 1); } @@ -330,8 +333,6 @@ // Dismiss |iterations| times. The final dismiss should change the decision // from dismiss to block, and hence change the persisted content setting. for (uint32_t i = 0; i < iterations; ++i) { - ContentSetting expected = - (i < 2) ? CONTENT_SETTING_ASK : CONTENT_SETTING_BLOCK; TestPermissionContext permission_context(profile(), content_settings_type); const PermissionRequestID id( @@ -355,25 +356,45 @@ "Permissions.Prompt.Dismissed.PriorDismissCount." + PermissionUtil::GetPermissionString(content_settings_type), i, 1); + +// On Android, repeatedly requesting and deciding permissions has the side +// effect of overcounting any metrics recorded in the PermissionInfoBarDelegate +// destructor. This is because we directly call +// PermissionQueueController::OnPermissionSet without setting the action_taken +// bit in PermissionInfoBarDelegate. When PermissionQueueController is deleted +// all OS_ANDROID ifdefs in this test can be removed. +#if !defined(OS_ANDROID) histograms.ExpectTotalCount("Permissions.AutoBlocker.EmbargoStatus", i + 1); +#endif + + PermissionResult result = + permission_context.GetPermissionStatus(url, url); + + histograms.ExpectUniqueSample( + "Permissions.AutoBlocker.EmbargoPromptSuppression", + PermissionEmbargoStatus::NOT_EMBARGOED, i + 1); if (i < 2) { + EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); + EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting); +#if !defined(OS_ANDROID) histograms.ExpectUniqueSample("Permissions.AutoBlocker.EmbargoStatus", PermissionEmbargoStatus::NOT_EMBARGOED, i + 1); +#endif } else { + EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source); + EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); +#if !defined(OS_ANDROID) histograms.ExpectBucketCount( "Permissions.AutoBlocker.EmbargoStatus", PermissionEmbargoStatus::REPEATED_DISMISSALS, 1); +#endif } ASSERT_EQ(1u, permission_context.decisions().size()); EXPECT_EQ(CONTENT_SETTING_ASK, permission_context.decisions()[0]); EXPECT_TRUE(permission_context.tab_context_updated()); - PermissionResult result = - permission_context.GetPermissionStatus(url, url); - EXPECT_EQ(expected, result.content_setting); - EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); } TestPermissionContext permission_context(profile(), content_settings_type); @@ -393,7 +414,10 @@ PermissionResult result = permission_context.GetPermissionStatus(url, url); EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); - EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); + EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source); + histograms.ExpectBucketCount( + "Permissions.AutoBlocker.EmbargoPromptSuppression", + PermissionEmbargoStatus::REPEATED_DISMISSALS, 1); } void TestBlockOnSeveralDismissals_TestContent() { @@ -423,9 +447,21 @@ i + 1); histograms.ExpectBucketCount( "Permissions.Prompt.Dismissed.PriorDismissCount.Geolocation", i, 1); + histograms.ExpectUniqueSample( + "Permissions.AutoBlocker.EmbargoPromptSuppression", + PermissionEmbargoStatus::NOT_EMBARGOED, i + 1); + +// On Android, repeatedly requesting and deciding permissions has the side +// effect of overcounting any metrics recorded in the PermissionInfoBarDelegate +// destructor. This is because we directly call +// PermissionQueueController::OnPermissionSet without setting the action_taken +// bit in PermissionInfoBarDelegate. When PermissionQueueController is deleted +// all OS_ANDROID ifdefs in this test can be removed. +#if !defined(OS_ANDROID) histograms.ExpectUniqueSample("Permissions.AutoBlocker.EmbargoStatus", PermissionEmbargoStatus::NOT_EMBARGOED, i + 1); +#endif ASSERT_EQ(1u, permission_context.decisions().size()); EXPECT_EQ(CONTENT_SETTING_ASK, permission_context.decisions()[0]); @@ -491,8 +527,6 @@ TestPermissionContext permission_context( profile(), CONTENT_SETTINGS_TYPE_MIDI_SYSEX); - ContentSetting expected = - (i < 4) ? CONTENT_SETTING_ASK : CONTENT_SETTING_BLOCK; const PermissionRequestID id( web_contents()->GetRenderProcessHost()->GetID(), web_contents()->GetMainFrame()->GetRoutingID(), i); @@ -510,24 +544,41 @@ EXPECT_TRUE(permission_context.tab_context_updated()); PermissionResult result = permission_context.GetPermissionStatus(url, url); - EXPECT_EQ(expected, result.content_setting); - EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); histograms.ExpectTotalCount( "Permissions.Prompt.Dismissed.PriorDismissCount.MidiSysEx", i + 1); histograms.ExpectBucketCount( "Permissions.Prompt.Dismissed.PriorDismissCount.MidiSysEx", i, 1); + histograms.ExpectUniqueSample( + "Permissions.AutoBlocker.EmbargoPromptSuppression", + PermissionEmbargoStatus::NOT_EMBARGOED, i + 1); +// On Android, repeatedly requesting and deciding permissions has the side +// effect of overcounting any metrics recorded in the PermissionInfoBarDelegate +// destructor. This is because we directly call +// PermissionQueueController::OnPermissionSet without setting the action_taken +// bit in PermissionInfoBarDelegate. When PermissionQueueController is deleted +// all OS_ANDROID ifdefs in this test can be removed. +#if !defined(OS_ANDROID) histograms.ExpectTotalCount("Permissions.AutoBlocker.EmbargoStatus", i + 1); +#endif if (i < 4) { + EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); +#if !defined(OS_ANDROID) histograms.ExpectUniqueSample("Permissions.AutoBlocker.EmbargoStatus", PermissionEmbargoStatus::NOT_EMBARGOED, i + 1); +#endif } else { + EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source); +#if !defined(OS_ANDROID) histograms.ExpectBucketCount( "Permissions.AutoBlocker.EmbargoStatus", PermissionEmbargoStatus::REPEATED_DISMISSALS, 1); +#endif } } @@ -536,13 +587,13 @@ CONTENT_SETTINGS_TYPE_MIDI_SYSEX); PermissionResult result = permission_context.GetPermissionStatus(url, url); EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); - EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); - + EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source); variations::testing::ClearAllVariationParams(); } void TestRequestPermissionInvalidUrl( ContentSettingsType content_settings_type) { + base::HistogramTester histograms; TestPermissionContext permission_context(profile(), content_settings_type); GURL url; ASSERT_FALSE(url.is_valid()); @@ -563,6 +614,8 @@ EXPECT_TRUE(permission_context.tab_context_updated()); EXPECT_EQ(CONTENT_SETTING_ASK, permission_context.GetContentSettingFromMap(url, url)); + histograms.ExpectTotalCount( + "Permissions.AutoBlocker.EmbargoPromptSuppression", 0); } void TestGrantAndRevoke_TestContent(ContentSettingsType content_settings_type, @@ -684,8 +737,7 @@ web_contents()->GetMainFrame()->GetRoutingID(), -1); // A response only needs to be made to the permission request if we do not - // expect he permission to be blacklisted, therefore set the response - // callback. + // expect the permission to be blacklisted. if (expected_permission_status == CONTENT_SETTING_ALLOW) { permission_context.SetRespondPermissionCallback( base::Bind(&PermissionContextBaseTests::RespondToPermission, @@ -699,12 +751,17 @@ base::Unretained(&permission_context))); PermissionResult result = permission_context.GetPermissionStatus(url, url); EXPECT_EQ(expected_permission_status, result.content_setting); - EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); if (expected_permission_status == CONTENT_SETTING_ALLOW) { ASSERT_EQ(1u, permission_context.decisions().size()); EXPECT_EQ(expected_permission_status, permission_context.decisions()[0]); + EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); + } else { + EXPECT_EQ(PermissionStatusSource::SAFE_BROWSING_BLACKLIST, result.source); } + histograms.ExpectUniqueSample( + "Permissions.AutoBlocker.EmbargoPromptSuppression", + expected_embargo_reason, 1); histograms.ExpectUniqueSample("Permissions.AutoBlocker.EmbargoStatus", expected_embargo_reason, 1); }
diff --git a/chrome/browser/permissions/permission_decision_auto_blocker.cc b/chrome/browser/permissions/permission_decision_auto_blocker.cc index 017553d..52df410 100644 --- a/chrome/browser/permissions/permission_decision_auto_blocker.cc +++ b/chrome/browser/permissions/permission_decision_auto_blocker.cc
@@ -13,7 +13,6 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/permissions/permission_blacklist_client.h" -#include "chrome/browser/permissions/permission_util.h" #include "chrome/browser/profiles/incognito_helpers.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/safe_browsing/safe_browsing_service.h" @@ -270,7 +269,8 @@ const GURL& request_origin, content::WebContents* web_contents, base::Callback<void(bool)> callback) { - DCHECK(!IsUnderEmbargo(permission, request_origin)); + DCHECK_EQ(CONTENT_SETTING_ASK, + GetEmbargoResult(permission, request_origin).content_setting); if (base::FeatureList::IsEnabled(features::kPermissionsBlacklist) && db_manager_) { @@ -289,7 +289,7 @@ callback.Run(false /* permission blocked */); } -bool PermissionDecisionAutoBlocker::IsUnderEmbargo( +PermissionResult PermissionDecisionAutoBlocker::GetEmbargoResult( ContentSettingsType permission, const GURL& request_origin) { HostContentSettingsMap* map = @@ -299,8 +299,7 @@ base::DictionaryValue* permission_dict = GetOrCreatePermissionDict( dict.get(), PermissionUtil::GetPermissionString(permission)); double embargo_date = -1; - bool is_under_dismiss_embargo = false; - bool is_under_blacklist_embargo = false; + base::Time current_time = clock_->Now(); if (base::FeatureList::IsEnabled(features::kPermissionsBlacklist) && permission_dict->GetDouble(kPermissionBlacklistEmbargoKey, @@ -308,7 +307,8 @@ if (current_time < base::Time::FromInternalValue(embargo_date) + base::TimeDelta::FromDays(g_blacklist_embargo_days)) { - is_under_blacklist_embargo = true; + return PermissionResult(CONTENT_SETTING_BLOCK, + PermissionStatusSource::SAFE_BROWSING_BLACKLIST); } } @@ -318,12 +318,13 @@ if (current_time < base::Time::FromInternalValue(embargo_date) + base::TimeDelta::FromDays(g_dismissal_embargo_days)) { - is_under_dismiss_embargo = true; + return PermissionResult(CONTENT_SETTING_BLOCK, + PermissionStatusSource::MULTIPLE_DISMISSALS); } } - // If either embargo is still in effect, return true. - return is_under_dismiss_embargo || is_under_blacklist_embargo; + return PermissionResult(CONTENT_SETTING_ASK, + PermissionStatusSource::UNSPECIFIED); } void PermissionDecisionAutoBlocker::CheckSafeBrowsingResult(
diff --git a/chrome/browser/permissions/permission_decision_auto_blocker.h b/chrome/browser/permissions/permission_decision_auto_blocker.h index ade0ff05..d35e6d7 100644 --- a/chrome/browser/permissions/permission_decision_auto_blocker.h +++ b/chrome/browser/permissions/permission_decision_auto_blocker.h
@@ -10,6 +10,7 @@ #include "base/memory/ref_counted.h" #include "base/memory/singleton.h" #include "base/time/default_clock.h" +#include "chrome/browser/permissions/permission_util.h" #include "components/content_settings/core/common/content_settings_types.h" #include "components/keyed_service/content/browser_context_keyed_service_factory.h" #include "components/keyed_service/core/keyed_service.h" @@ -80,10 +81,8 @@ // Updates the threshold to start blocking prompts from the field trial. static void UpdateFromVariations(); - // Checks if |request_origin| is under embargo for |permission|. Internally, - // this will make a call to IsUnderEmbargo to check the content setting first, - // but may also make a call to Safe Browsing to check the API blacklist, which - // is performed asynchronously. + // Updates whether |request_origin| should be under embargo for |permission|. + // Makes an asynchronous call to Safe Browsing to check the API blacklist. void UpdateEmbargoedStatus(ContentSettingsType permission, const GURL& request_origin, content::WebContents* web_contents, @@ -92,8 +91,8 @@ // Checks the status of the content setting to determine if |request_origin| // is under embargo for |permission|. This checks both embargo for Permissions // Blacklisting and repeated dismissals. - bool IsUnderEmbargo(ContentSettingsType permission, - const GURL& request_origin); + PermissionResult GetEmbargoResult(ContentSettingsType permission, + const GURL& request_origin); private: friend class PermissionContextBaseTests;
diff --git a/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc b/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc index 43b884e2..739d12e 100644 --- a/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc +++ b/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
@@ -189,14 +189,14 @@ url1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); // Record some ignores. - EXPECT_EQ(1, autoblocker()->RecordIgnore( - url1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); + EXPECT_EQ( + 1, autoblocker()->RecordIgnore(url1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); EXPECT_EQ(1, autoblocker()->RecordIgnore( url1, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE)); - EXPECT_EQ(1, autoblocker()->RecordIgnore( - url2, CONTENT_SETTINGS_TYPE_GEOLOCATION)); - EXPECT_EQ(2, autoblocker()->RecordIgnore( - url2, CONTENT_SETTINGS_TYPE_GEOLOCATION)); + EXPECT_EQ( + 1, autoblocker()->RecordIgnore(url2, CONTENT_SETTINGS_TYPE_GEOLOCATION)); + EXPECT_EQ( + 2, autoblocker()->RecordIgnore(url2, CONTENT_SETTINGS_TYPE_GEOLOCATION)); autoblocker()->RemoveCountsByUrl(base::Bind(&FilterGoogle)); @@ -205,8 +205,8 @@ url1, CONTENT_SETTINGS_TYPE_GEOLOCATION)); EXPECT_EQ(0, autoblocker()->GetDismissCount( url1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); - EXPECT_EQ(0, autoblocker()->GetIgnoreCount( - url1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); + EXPECT_EQ( + 0, autoblocker()->GetIgnoreCount(url1, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); EXPECT_EQ(0, autoblocker()->GetIgnoreCount( url1, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE)); @@ -231,14 +231,14 @@ EXPECT_EQ(2, autoblocker()->GetDismissCount( url2, CONTENT_SETTINGS_TYPE_GEOLOCATION)); - EXPECT_EQ(1, autoblocker()->RecordIgnore( - url1, CONTENT_SETTINGS_TYPE_GEOLOCATION)); + EXPECT_EQ( + 1, autoblocker()->RecordIgnore(url1, CONTENT_SETTINGS_TYPE_GEOLOCATION)); EXPECT_EQ(1, autoblocker()->RecordIgnore( url1, CONTENT_SETTINGS_TYPE_NOTIFICATIONS)); EXPECT_EQ(1, autoblocker()->RecordIgnore( url1, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE)); - EXPECT_EQ(1, autoblocker()->RecordIgnore( - url2, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); + EXPECT_EQ( + 1, autoblocker()->RecordIgnore(url2, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); // Remove everything and expect that it's all gone. autoblocker()->RemoveCountsByUrl(base::Bind(&FilterAll)); @@ -258,8 +258,8 @@ url2, CONTENT_SETTINGS_TYPE_GEOLOCATION)); EXPECT_EQ(0, autoblocker()->GetIgnoreCount( url2, CONTENT_SETTINGS_TYPE_DURABLE_STORAGE)); - EXPECT_EQ(0, autoblocker()->GetIgnoreCount( - url2, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); + EXPECT_EQ( + 0, autoblocker()->GetIgnoreCount(url2, CONTENT_SETTINGS_TYPE_MIDI_SYSEX)); } // Test that an origin that has been blacklisted for a permission is embargoed. @@ -307,42 +307,61 @@ "Permissions.AutoBlocker.SafeBrowsingResponseTime", 1); } -// Check that IsUnderEmbargo returns the correct value when the embargo is set +// Check that GetEmbargoResult returns the correct value when the embargo is set // and expires. TEST_F(PermissionDecisionAutoBlockerUnitTest, CheckEmbargoStatus) { GURL url("https://www.google.com"); clock()->SetNow(base::Time::Now()); + // Check the default state. + PermissionResult result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); + + // Place under embargo and verify. PlaceUnderBlacklistEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); - EXPECT_TRUE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::SAFE_BROWSING_BLACKLIST, result.source); // Check that the origin is not under embargo for a different permission. - EXPECT_FALSE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, url); + EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); // Confirm embargo status during the embargo period. clock()->Advance(base::TimeDelta::FromDays(5)); - EXPECT_TRUE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::SAFE_BROWSING_BLACKLIST, result.source); // Check embargo is lifted on expiry day. A small offset after the exact // embargo expiration date has been added to account for any precision errors // when removing the date stored as a double from the permission dictionary. clock()->Advance(base::TimeDelta::FromHours(3 * 24 + 1)); - EXPECT_FALSE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); // Check embargo is lifted well after the expiry day. clock()->Advance(base::TimeDelta::FromDays(1)); - EXPECT_FALSE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); // Place under embargo again and verify the embargo status. PlaceUnderBlacklistEmbargo(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, url); clock()->Advance(base::TimeDelta::FromDays(1)); - EXPECT_TRUE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, url); + EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::SAFE_BROWSING_BLACKLIST, result.source); } // Tests the alternating pattern of the block on multiple dismiss behaviour. On @@ -362,14 +381,18 @@ url, CONTENT_SETTINGS_TYPE_GEOLOCATION)); // A request with < 3 prior dismisses should not be automatically blocked. - EXPECT_FALSE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + PermissionResult result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); // After the 3rd dismiss subsequent permission requests should be autoblocked. EXPECT_TRUE(autoblocker()->RecordDismissAndEmbargo( url, CONTENT_SETTINGS_TYPE_GEOLOCATION)); - EXPECT_TRUE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source); histograms.ExpectTotalCount("Permissions.AutoBlocker.SafeBrowsingResponse", 0); @@ -378,26 +401,34 @@ // Accelerate time forward, check that the embargo status is lifted and the // request won't be automatically blocked. clock()->Advance(base::TimeDelta::FromDays(8)); - EXPECT_FALSE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); // Record another dismiss, subsequent requests should be autoblocked again. EXPECT_TRUE(autoblocker()->RecordDismissAndEmbargo( url, CONTENT_SETTINGS_TYPE_GEOLOCATION)); - EXPECT_TRUE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source); // Accelerate time again, check embargo is lifted and another permission // request is let through. clock()->Advance(base::TimeDelta::FromDays(8)); - EXPECT_FALSE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); // Record another dismiss, subsequent requests should be autoblocked again. EXPECT_TRUE(autoblocker()->RecordDismissAndEmbargo( url, CONTENT_SETTINGS_TYPE_GEOLOCATION)); - EXPECT_TRUE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source); histograms.ExpectTotalCount("Permissions.AutoBlocker.SafeBrowsingResponse", 0); histograms.ExpectTotalCount( @@ -412,8 +443,10 @@ // Place under blacklist embargo and check the status. PlaceUnderBlacklistEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); clock()->Advance(base::TimeDelta::FromDays(5)); - EXPECT_TRUE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + PermissionResult result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::SAFE_BROWSING_BLACKLIST, result.source); // Record dismisses to place it under dismissal embargo. EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo( @@ -426,8 +459,10 @@ // Accelerate time to a point where the blacklist embargo should be expired // and check that dismissal embargo is still set. clock()->Advance(base::TimeDelta::FromDays(3)); - EXPECT_TRUE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::MULTIPLE_DISMISSALS, result.source); } TEST_F(PermissionDecisionAutoBlockerUnitTest, TestSafeBrowsingTimeout) { @@ -446,8 +481,12 @@ UpdateEmbargoedStatus(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); EXPECT_TRUE(callback_was_run()); EXPECT_FALSE(last_embargoed_status()); - EXPECT_FALSE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + + PermissionResult result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_ASK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::UNSPECIFIED, result.source); + histograms.ExpectUniqueSample("Permissions.AutoBlocker.SafeBrowsingResponse", SafeBrowsingResponse::TIMEOUT, 1); histograms.ExpectTotalCount( @@ -467,8 +506,10 @@ histograms.ExpectBucketCount("Permissions.AutoBlocker.SafeBrowsingResponse", SafeBrowsingResponse::BLACKLISTED, 1); clock()->Advance(base::TimeDelta::FromDays(1)); - EXPECT_TRUE( - autoblocker()->IsUnderEmbargo(CONTENT_SETTINGS_TYPE_GEOLOCATION, url)); + result = + autoblocker()->GetEmbargoResult(CONTENT_SETTINGS_TYPE_GEOLOCATION, url); + EXPECT_EQ(CONTENT_SETTING_BLOCK, result.content_setting); + EXPECT_EQ(PermissionStatusSource::SAFE_BROWSING_BLACKLIST, result.source); } // TODO(raymes): See crbug.com/681709. Remove after M60. @@ -493,8 +534,8 @@ // Nothing should be migrated yet, so the current values should be 0. EXPECT_EQ(0, autoblocker()->GetDismissCount( url, CONTENT_SETTINGS_TYPE_GEOLOCATION)); - EXPECT_EQ(0, autoblocker()->GetIgnoreCount( - url, CONTENT_SETTINGS_TYPE_GEOLOCATION)); + EXPECT_EQ( + 0, autoblocker()->GetIgnoreCount(url, CONTENT_SETTINGS_TYPE_GEOLOCATION)); // Trigger pref migration which happens at the creation of the // HostContentSettingsMap.
diff --git a/chrome/browser/permissions/permission_infobar_delegate.cc b/chrome/browser/permissions/permission_infobar_delegate.cc index 1f4be38..95f748a2 100644 --- a/chrome/browser/permissions/permission_infobar_delegate.cc +++ b/chrome/browser/permissions/permission_infobar_delegate.cc
@@ -78,6 +78,9 @@ user_gesture_ ? PermissionRequestGestureType::GESTURE : PermissionRequestGestureType::NO_GESTURE, requesting_origin_, profile_); + + PermissionUmaUtil::RecordEmbargoStatus( + PermissionEmbargoStatus::NOT_EMBARGOED); } }
diff --git a/chrome/browser/permissions/permission_queue_controller.cc b/chrome/browser/permissions/permission_queue_controller.cc index c56db61..8f095ecb 100644 --- a/chrome/browser/permissions/permission_queue_controller.cc +++ b/chrome/browser/permissions/permission_queue_controller.cc
@@ -221,22 +221,21 @@ PermissionUtil::GetRequestType(content_settings_type_); PermissionRequestGestureType gesture_type = PermissionUtil::GetGestureType(user_gesture); + PermissionEmbargoStatus embargo_status = + PermissionEmbargoStatus::NOT_EMBARGOED; + switch (decision) { case GRANTED: PermissionUmaUtil::PermissionGranted(content_settings_type_, gesture_type, requesting_frame, profile_); PermissionUmaUtil::RecordPermissionPromptAccepted(request_type, gesture_type); - PermissionUmaUtil::RecordPermissionEmbargoStatus( - PermissionEmbargoStatus::NOT_EMBARGOED); break; case DENIED: PermissionUmaUtil::PermissionDenied(content_settings_type_, gesture_type, requesting_frame, profile_); PermissionUmaUtil::RecordPermissionPromptDenied(request_type, gesture_type); - PermissionUmaUtil::RecordPermissionEmbargoStatus( - PermissionEmbargoStatus::NOT_EMBARGOED); break; case DISMISSED: PermissionUmaUtil::PermissionDismissed( @@ -244,16 +243,13 @@ if (PermissionDecisionAutoBlocker::GetForProfile(profile_) ->RecordDismissAndEmbargo(requesting_frame, content_settings_type_)) { - PermissionUmaUtil::RecordPermissionEmbargoStatus( - PermissionEmbargoStatus::REPEATED_DISMISSALS); - } else { - PermissionUmaUtil::RecordPermissionEmbargoStatus( - PermissionEmbargoStatus::NOT_EMBARGOED); + embargo_status = PermissionEmbargoStatus::REPEATED_DISMISSALS; } break; default: NOTREACHED(); } + PermissionUmaUtil::RecordEmbargoStatus(embargo_status); // TODO(miguelg): move the permission persistence to // PermissionContextBase once all the types are moved there.
diff --git a/chrome/browser/permissions/permission_request_impl.cc b/chrome/browser/permissions/permission_request_impl.cc index 57556c1b..cf17b1c 100644 --- a/chrome/browser/permissions/permission_request_impl.cc +++ b/chrome/browser/permissions/permission_request_impl.cc
@@ -40,7 +40,7 @@ if (!action_taken_) { PermissionUmaUtil::PermissionIgnored( content_settings_type_, GetGestureType(), request_origin_, profile_); - PermissionUmaUtil::RecordPermissionEmbargoStatus( + PermissionUmaUtil::RecordEmbargoStatus( PermissionEmbargoStatus::NOT_EMBARGOED); } }
diff --git a/chrome/browser/permissions/permission_uma_util.cc b/chrome/browser/permissions/permission_uma_util.cc index 94ec8298..b7db409 100644 --- a/chrome/browser/permissions/permission_uma_util.cc +++ b/chrome/browser/permissions/permission_uma_util.cc
@@ -363,7 +363,35 @@ } } -void PermissionUmaUtil::RecordPermissionEmbargoStatus( +void PermissionUmaUtil::RecordEmbargoPromptSuppression( + PermissionEmbargoStatus embargo_status) { + UMA_HISTOGRAM_ENUMERATION("Permissions.AutoBlocker.EmbargoPromptSuppression", + embargo_status, + PermissionEmbargoStatus::STATUS_NUM); +} + +void PermissionUmaUtil::RecordEmbargoPromptSuppressionFromSource( + PermissionStatusSource source) { + // Explicitly switch to ensure that any new PermissionStatusSource values are + // dealt with appropriately. + switch (source) { + case PermissionStatusSource::MULTIPLE_DISMISSALS: + PermissionUmaUtil::RecordEmbargoPromptSuppression( + PermissionEmbargoStatus::REPEATED_DISMISSALS); + break; + case PermissionStatusSource::SAFE_BROWSING_BLACKLIST: + PermissionUmaUtil::RecordEmbargoPromptSuppression( + PermissionEmbargoStatus::PERMISSIONS_BLACKLISTING); + break; + case PermissionStatusSource::UNSPECIFIED: + case PermissionStatusSource::KILL_SWITCH: + // The permission wasn't under embargo, so don't record anything. We may + // embargo it later. + break; + } +} + +void PermissionUmaUtil::RecordEmbargoStatus( PermissionEmbargoStatus embargo_status) { UMA_HISTOGRAM_ENUMERATION("Permissions.AutoBlocker.EmbargoStatus", embargo_status,
diff --git a/chrome/browser/permissions/permission_uma_util.h b/chrome/browser/permissions/permission_uma_util.h index 6a4de411..de92418b 100644 --- a/chrome/browser/permissions/permission_uma_util.h +++ b/chrome/browser/permissions/permission_uma_util.h
@@ -48,6 +48,16 @@ RESPONSE_NUM, }; +// Any new values should be inserted immediately prior to STATUS_NUM. +enum PermissionEmbargoStatus { + NOT_EMBARGOED = 0, + PERMISSIONS_BLACKLISTING = 1, + REPEATED_DISMISSALS = 2, + + // Keep this at the end. + STATUS_NUM, +}; + // A bundle for the information sent in a PermissionReport. struct PermissionReportInfo { PermissionReportInfo( @@ -72,15 +82,6 @@ int num_prior_ignores; }; -enum PermissionEmbargoStatus { - NOT_EMBARGOED = 0, - PERMISSIONS_BLACKLISTING = 1, - REPEATED_DISMISSALS = 2, - - // Keep this at the end. - STATUS_NUM, -}; - // Provides a convenient way of logging UMA for permission related operations. class PermissionUmaUtil { public: @@ -131,9 +132,14 @@ const GURL& revoked_origin, Profile* profile); - static void RecordPermissionEmbargoStatus( + static void RecordEmbargoPromptSuppression( PermissionEmbargoStatus embargo_status); + static void RecordEmbargoPromptSuppressionFromSource( + PermissionStatusSource source); + + static void RecordEmbargoStatus(PermissionEmbargoStatus embargo_status); + static void RecordSafeBrowsingResponse(base::TimeDelta response_time, SafeBrowsingResponse response);
diff --git a/chrome/browser/permissions/permission_util.cc b/chrome/browser/permissions/permission_util.cc index bae51e0..09ae219 100644 --- a/chrome/browser/permissions/permission_util.cc +++ b/chrome/browser/permissions/permission_util.cc
@@ -15,6 +15,12 @@ using content::PermissionType; +PermissionResult::PermissionResult(ContentSetting cs, + PermissionStatusSource pss) + : content_setting(cs), source(pss) {} + +PermissionResult::~PermissionResult() {} + // The returned strings must match the RAPPOR metrics in rappor.xml, // and any Field Trial configs for the Permissions kill switch e.g. // Permissions.Action.Geolocation etc..
diff --git a/chrome/browser/permissions/permission_util.h b/chrome/browser/permissions/permission_util.h index 097203a..3f7c14c 100644 --- a/chrome/browser/permissions/permission_util.h +++ b/chrome/browser/permissions/permission_util.h
@@ -35,6 +35,33 @@ PERMISSION_ACTION_NUM, }; +// Identifies the source or reason for a permission status being returned. This +// enum backs an UMA histogram and must be treated as append-only. +enum class PermissionStatusSource { + // The reason for the status is not specified. + UNSPECIFIED, + + // The status is the result of being blocked due to the user dismissing a + // permission prompt multiple times. + MULTIPLE_DISMISSALS, + + // The status is the result of being blocked because the permission is on the + // safe browsing blacklist. + SAFE_BROWSING_BLACKLIST, + + // The status is the result of being blocked by the permissions kill switch. + KILL_SWITCH, +}; + +struct PermissionResult { + PermissionResult(ContentSetting content_setting, + PermissionStatusSource source); + ~PermissionResult(); + + ContentSetting content_setting; + PermissionStatusSource source; +}; + // A utility class for permissions. class PermissionUtil { public:
diff --git a/chrome/browser/printing/print_dialog_cloud.cc b/chrome/browser/printing/print_dialog_cloud.cc index cb0ffcc..b22d999a 100644 --- a/chrome/browser/printing/print_dialog_cloud.cc +++ b/chrome/browser/printing/print_dialog_cloud.cc
@@ -77,7 +77,7 @@ add_account ? BrowserWindow::AVATAR_BUBBLE_MODE_ADD_ACCOUNT : BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN, signin::ManageAccountsParams(), - signin_metrics::AccessPoint::ACCESS_POINT_CLOUD_PRINT); + signin_metrics::AccessPoint::ACCESS_POINT_CLOUD_PRINT, false); } else { GURL url = add_account ? cloud_devices::GetCloudPrintAddAccountURL() : cloud_devices::GetCloudPrintSigninURL();
diff --git a/chrome/browser/resources/settings/a11y_page/manage_a11y_page.html b/chrome/browser/resources/settings/a11y_page/manage_a11y_page.html index 06e6f2a..d2fef10 100644 --- a/chrome/browser/resources/settings/a11y_page/manage_a11y_page.html +++ b/chrome/browser/resources/settings/a11y_page/manage_a11y_page.html
@@ -147,6 +147,15 @@ pref="{{prefs.settings.a11y.large_cursor_enabled}}" label="$i18n{largeMouseCursorLabel}"> </settings-toggle-button> + <template is="dom-if" if="[[enableAdjustableLargeCursor_]]"> + <div class="list-item"> + <div>$i18n{largeMouseCursorSizeLabel}</div> + <cr-slider + disabled="[[!prefs.settings.a11y.large_cursor_enabled.value]]" + value="{{prefs.settings.a11y.large_cursor_dip_size.value}}" + min="25" max="64"></cr-slider> + </div> + </template> <settings-toggle-button pref="{{prefs.settings.a11y.cursor_highlight}}" label="$i18n{cursorHighlightLabel}">
diff --git a/chrome/browser/resources/settings/a11y_page/manage_a11y_page.js b/chrome/browser/resources/settings/a11y_page/manage_a11y_page.js index 605336d0..354082b 100644 --- a/chrome/browser/resources/settings/a11y_page/manage_a11y_page.js +++ b/chrome/browser/resources/settings/a11y_page/manage_a11y_page.js
@@ -51,6 +51,17 @@ return loadTimeData.getBoolean('showExperimentalA11yFeatures'); }, }, + + /** + * Whether adjustable large cursor is enabled or not. + * @private {boolean} + */ + enableAdjustableLargeCursor_: { + type: Boolean, + value: function() { + return loadTimeData.getBoolean('enableAdjustableLargeCursor'); + }, + }, }, /** @private */
diff --git a/chrome/browser/signin/chrome_signin_helper.cc b/chrome/browser/signin/chrome_signin_helper.cc index d009afb..7e326dd5 100644 --- a/chrome/browser/signin/chrome_signin_helper.cc +++ b/chrome/browser/signin/chrome_signin_helper.cc
@@ -76,7 +76,7 @@ account_reconcilor->GetState()); browser->window()->ShowAvatarBubbleFromAvatarButton( bubble_mode, manage_accounts_params, - signin_metrics::AccessPoint::ACCESS_POINT_CONTENT_AREA); + signin_metrics::AccessPoint::ACCESS_POINT_CONTENT_AREA, false); } #else // defined(OS_ANDROID) if (service_type == signin::GAIA_SERVICE_TYPE_INCOGNITO) {
diff --git a/chrome/browser/signin/signin_global_error.cc b/chrome/browser/signin/signin_global_error.cc index cac74cfc..c83baa28 100644 --- a/chrome/browser/signin/signin_global_error.cc +++ b/chrome/browser/signin/signin_global_error.cc
@@ -100,7 +100,7 @@ signin_metrics::HISTOGRAM_REAUTH_MAX); browser->window()->ShowAvatarBubbleFromAvatarButton( BrowserWindow::AVATAR_BUBBLE_MODE_REAUTH, signin::ManageAccountsParams(), - signin_metrics::AccessPoint::ACCESS_POINT_MENU); + signin_metrics::AccessPoint::ACCESS_POINT_MENU, false); #endif }
diff --git a/chrome/browser/ui/browser_commands.cc b/chrome/browser/ui/browser_commands.cc index 5beb96d4..5225294 100644 --- a/chrome/browser/ui/browser_commands.cc +++ b/chrome/browser/ui/browser_commands.cc
@@ -1125,14 +1125,14 @@ void ShowAvatarMenu(Browser* browser) { browser->window()->ShowAvatarBubbleFromAvatarButton( BrowserWindow::AVATAR_BUBBLE_MODE_DEFAULT, signin::ManageAccountsParams(), - signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN); + signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN, true); } void ShowFastUserSwitcher(Browser* browser) { browser->window()->ShowAvatarBubbleFromAvatarButton( BrowserWindow::AVATAR_BUBBLE_MODE_FAST_USER_SWITCH, signin::ManageAccountsParams(), - signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN); + signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN, false); } void OpenUpdateChromeDialog(Browser* browser) {
diff --git a/chrome/browser/ui/browser_window.h b/chrome/browser/ui/browser_window.h index 58f4c448..2422d59 100644 --- a/chrome/browser/ui/browser_window.h +++ b/chrome/browser/ui/browser_window.h
@@ -368,7 +368,8 @@ virtual void ShowAvatarBubbleFromAvatarButton( AvatarBubbleMode mode, const signin::ManageAccountsParams& manage_accounts_params, - signin_metrics::AccessPoint access_point) = 0; + signin_metrics::AccessPoint access_point, + bool is_source_keyboard) = 0; // Returns the height inset for RenderView when detached bookmark bar is // shown. Invoked when a new RenderHostView is created for a non-NTP
diff --git a/chrome/browser/ui/chrome_pages.cc b/chrome/browser/ui/chrome_pages.cc index 95ad166..44687a07 100644 --- a/chrome/browser/ui/chrome_pages.cc +++ b/chrome/browser/ui/chrome_pages.cc
@@ -437,7 +437,7 @@ if (show_avatar_bubble) { browser->window()->ShowAvatarBubbleFromAvatarButton( BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN, - signin::ManageAccountsParams(), access_point); + signin::ManageAccountsParams(), access_point, false); } else { NavigateToSingletonTab( browser,
diff --git a/chrome/browser/ui/cocoa/browser_window_cocoa.h b/chrome/browser/ui/cocoa/browser_window_cocoa.h index ecf9ad8..f8025a26 100644 --- a/chrome/browser/ui/cocoa/browser_window_cocoa.h +++ b/chrome/browser/ui/cocoa/browser_window_cocoa.h
@@ -148,7 +148,8 @@ void ShowAvatarBubbleFromAvatarButton( AvatarBubbleMode mode, const signin::ManageAccountsParams& manage_accounts_params, - signin_metrics::AccessPoint access_point) override; + signin_metrics::AccessPoint access_point, + bool is_source_keyboard) override; int GetRenderViewHeightInsetWithDetachedBookmarkBar() override; void ExecuteExtensionCommand(const extensions::Extension* extension, const extensions::Command& command) override;
diff --git a/chrome/browser/ui/cocoa/browser_window_cocoa.mm b/chrome/browser/ui/cocoa/browser_window_cocoa.mm index 751979ed..0f2bf1a 100644 --- a/chrome/browser/ui/cocoa/browser_window_cocoa.mm +++ b/chrome/browser/ui/cocoa/browser_window_cocoa.mm
@@ -801,7 +801,8 @@ void BrowserWindowCocoa::ShowAvatarBubbleFromAvatarButton( AvatarBubbleMode mode, const signin::ManageAccountsParams& manage_accounts_params, - signin_metrics::AccessPoint access_point) { + signin_metrics::AccessPoint access_point, + bool is_source_keyboard) { profiles::BubbleViewMode bubble_view_mode; profiles::TutorialMode tutorial_mode; profiles::BubbleViewModeFromAvatarBubbleMode(mode, &bubble_view_mode,
diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc index e11d186..ae8e69f 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
@@ -340,9 +340,8 @@ void ManagePasswordsUIController::NavigateToChromeSignIn() { Browser* browser = chrome::FindBrowserWithWebContents(web_contents()); browser->window()->ShowAvatarBubbleFromAvatarButton( - BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN, - signin::ManageAccountsParams(), - signin_metrics::AccessPoint::ACCESS_POINT_PASSWORD_BUBBLE); + BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN, signin::ManageAccountsParams(), + signin_metrics::AccessPoint::ACCESS_POINT_PASSWORD_BUBBLE, false); } void ManagePasswordsUIController::OnDialogHidden() {
diff --git a/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc b/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc index 65e254eb..6e5d89c 100644 --- a/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc +++ b/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
@@ -16,7 +16,6 @@ #include "ash/common/wm_window.h" #include "ash/public/cpp/shell_window_ids.h" #include "ash/public/cpp/window_properties.h" -#include "ash/screen_util.h" #include "ash/shared/app_types.h" #include "ash/shared/immersive_fullscreen_controller.h" #include "ash/shell.h" @@ -40,6 +39,7 @@ #include "ui/views/controls/menu/menu_model_adapter.h" #include "ui/views/controls/menu/menu_runner.h" #include "ui/views/widget/widget.h" +#include "ui/wm/core/coordinate_conversion.h" using extensions::AppWindow; @@ -200,8 +200,9 @@ } else if (ash::Shell::HasInstance() && use_default_bounds) { // Open a new panel on the target root. init_params->context = ash::Shell::GetTargetRootWindow(); - init_params->bounds = ash::ScreenUtil::ConvertRectToScreen( - ash::Shell::GetTargetRootWindow(), gfx::Rect(GetPreferredSize())); + init_params->bounds = gfx::Rect(GetPreferredSize()); + wm::ConvertRectToScreen(ash::Shell::GetTargetRootWindow(), + &init_params->bounds); } }
diff --git a/chrome/browser/ui/views/elevation_icon_setter.cc b/chrome/browser/ui/views/elevation_icon_setter.cc index bd7c458c..d77614a 100644 --- a/chrome/browser/ui/views/elevation_icon_setter.cc +++ b/chrome/browser/ui/views/elevation_icon_setter.cc
@@ -5,10 +5,8 @@ #include "chrome/browser/ui/views/elevation_icon_setter.h" #include "base/callback.h" -#include "base/task_runner_util.h" -#include "base/threading/sequenced_worker_pool.h" +#include "base/task_scheduler/post_task.h" #include "build/build_config.h" -#include "content/public/browser/browser_thread.h" #include "ui/views/controls/button/label_button.h" #if defined(OS_WIN) @@ -63,13 +61,12 @@ const base::Closure& callback) : button_(button), weak_factory_(this) { - base::PostTaskAndReplyWithResult( - content::BrowserThread::GetBlockingPool(), - FROM_HERE, + base::PostTaskWithTraitsAndReplyWithResult( + FROM_HERE, base::TaskTraits().MayBlock().WithPriority( + base::TaskPriority::USER_BLOCKING), base::Bind(&GetElevationIcon), base::Bind(&ElevationIconSetter::SetButtonIcon, - weak_factory_.GetWeakPtr(), - callback)); + weak_factory_.GetWeakPtr(), callback)); } ElevationIconSetter::~ElevationIconSetter() {
diff --git a/chrome/browser/ui/views/frame/avatar_button_manager.cc b/chrome/browser/ui/views/frame/avatar_button_manager.cc index 21d52a28..231d749 100644 --- a/chrome/browser/ui/views/frame/avatar_button_manager.cc +++ b/chrome/browser/ui/views/frame/avatar_button_manager.cc
@@ -65,5 +65,5 @@ } frame_view_->browser_view()->ShowAvatarBubbleFromAvatarButton( mode, signin::ManageAccountsParams(), - signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN); + signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN, false); }
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc index 196f424..24ea275 100644 --- a/chrome/browser/ui/views/frame/browser_view.cc +++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -2469,7 +2469,8 @@ void BrowserView::ShowAvatarBubbleFromAvatarButton( AvatarBubbleMode mode, const signin::ManageAccountsParams& manage_accounts_params, - signin_metrics::AccessPoint access_point) { + signin_metrics::AccessPoint access_point, + bool focus_first_profile_button) { #if !defined(OS_CHROMEOS) // Do not show avatar bubble if there is no avatar menu button. if (!frame_->GetNewAvatarMenuButton()) @@ -2484,7 +2485,8 @@ } else { ProfileChooserView::ShowBubble(bubble_view_mode, tutorial_mode, manage_accounts_params, access_point, - frame_->GetNewAvatarMenuButton(), browser()); + frame_->GetNewAvatarMenuButton(), browser(), + focus_first_profile_button); ProfileMetrics::LogProfileOpenMethod(ProfileMetrics::ICON_AVATAR_BUBBLE); } #else
diff --git a/chrome/browser/ui/views/frame/browser_view.h b/chrome/browser/ui/views/frame/browser_view.h index bcfdd06..358cf19a 100644 --- a/chrome/browser/ui/views/frame/browser_view.h +++ b/chrome/browser/ui/views/frame/browser_view.h
@@ -360,7 +360,8 @@ void ShowAvatarBubbleFromAvatarButton( AvatarBubbleMode mode, const signin::ManageAccountsParams& manage_accounts_params, - signin_metrics::AccessPoint access_point) override; + signin_metrics::AccessPoint access_point, + bool is_source_keyboard) override; int GetRenderViewHeightInsetWithDetachedBookmarkBar() override; void ExecuteExtensionCommand(const extensions::Extension* extension, const extensions::Command& command) override;
diff --git a/chrome/browser/ui/views/frame/taskbar_decorator_win.cc b/chrome/browser/ui/views/frame/taskbar_decorator_win.cc index 8b47df2..85dafebf 100644 --- a/chrome/browser/ui/views/frame/taskbar_decorator_win.cc +++ b/chrome/browser/ui/views/frame/taskbar_decorator_win.cc
@@ -8,12 +8,11 @@ #include "base/bind.h" #include "base/location.h" -#include "base/threading/sequenced_worker_pool.h" +#include "base/task_scheduler/post_task.h" #include "base/win/scoped_comptr.h" #include "base/win/scoped_gdi_object.h" #include "base/win/windows_version.h" #include "chrome/browser/profiles/profile_avatar_icon_util.h" -#include "content/public/browser/browser_thread.h" #include "skia/ext/image_operations.h" #include "skia/ext/platform_canvas.h" #include "ui/gfx/icon_util.h" @@ -86,9 +85,12 @@ bitmap.reset(new SkBitmap( profiles::GetAvatarIconAsSquare(*image->ToSkBitmap(), 1))); } - content::BrowserThread::GetBlockingPool()->PostWorkerTaskWithShutdownBehavior( - FROM_HERE, base::Bind(&SetOverlayIcon, hwnd, base::Passed(&bitmap)), - base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN); + // TODO(robliao): Annotate this task with .WithCOM() once supported. + // https://crbug.com/662122 + base::PostTaskWithTraits( + FROM_HERE, base::TaskTraits().MayBlock().WithPriority( + base::TaskPriority::USER_VISIBLE), + base::Bind(&SetOverlayIcon, hwnd, base::Passed(&bitmap))); } } // namespace chrome
diff --git a/chrome/browser/ui/views/profiles/profile_chooser_view.cc b/chrome/browser/ui/views/profiles/profile_chooser_view.cc index a124ccbb..62702ba 100644 --- a/chrome/browser/ui/views/profiles/profile_chooser_view.cc +++ b/chrome/browser/ui/views/profiles/profile_chooser_view.cc
@@ -762,7 +762,8 @@ const signin::ManageAccountsParams& manage_accounts_params, signin_metrics::AccessPoint access_point, views::View* anchor_view, - Browser* browser) { + Browser* browser, + bool is_source_keyboard) { // Don't start creating the view if it would be an empty fast user switcher. // It has to happen here to prevent the view system from creating an empty // container. @@ -790,6 +791,8 @@ profile_bubble_->SetAlignment(views::BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE); profile_bubble_->SetArrowPaintType(views::BubbleBorder::PAINT_NONE); widget->Show(); + if (is_source_keyboard) + profile_bubble_->FocusFirstProfileButton(); } // static @@ -850,6 +853,7 @@ current_profile_photo_ = nullptr; current_profile_name_ = nullptr; current_profile_card_ = nullptr; + first_profile_button_ = nullptr; guest_profile_button_ = nullptr; users_button_ = nullptr; go_incognito_button_ = nullptr; @@ -1021,6 +1025,11 @@ } } +void ProfileChooserView::FocusFirstProfileButton() { + if (first_profile_button_) + first_profile_button_->RequestFocus(); +} + void ProfileChooserView::WindowClosing() { DCHECK_EQ(profile_bubble_, this); profile_bubble_ = NULL; @@ -1939,6 +1948,8 @@ button->SetImageLabelSpacing(kMaterialMenuEdgeMargin); open_other_profile_indexes_map_[button] = i; + if (!first_profile_button_) + first_profile_button_ = button; layout->StartRow(1, 0); layout->AddView(button); }
diff --git a/chrome/browser/ui/views/profiles/profile_chooser_view.h b/chrome/browser/ui/views/profiles/profile_chooser_view.h index 7da9bda..3d41e57a 100644 --- a/chrome/browser/ui/views/profiles/profile_chooser_view.h +++ b/chrome/browser/ui/views/profiles/profile_chooser_view.h
@@ -59,7 +59,8 @@ const signin::ManageAccountsParams& manage_accounts_params, signin_metrics::AccessPoint access_point, views::View* anchor_view, - Browser* browser); + Browser* browser, + bool is_source_keyboard); static bool IsShowing(); static void Hide(); @@ -127,6 +128,9 @@ AvatarMenu* avatar_menu); void ShowViewFromMode(profiles::BubbleViewMode mode); + // Focuses the first profile button in the menu list. + void FocusFirstProfileButton(); + // Creates the profile chooser view. views::View* CreateProfileChooserView(AvatarMenu* avatar_menu); @@ -261,6 +265,7 @@ views::LabelButton* current_profile_card_; // Action buttons. + views::LabelButton* first_profile_button_; views::LabelButton* guest_profile_button_; views::LabelButton* users_button_; views::LabelButton* go_incognito_button_;
diff --git a/chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc b/chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc index 6b190de..af69edfe 100644 --- a/chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc +++ b/chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc
@@ -212,7 +212,7 @@ browser->window()->ShowAvatarBubbleFromAvatarButton( BrowserWindow::AVATAR_BUBBLE_MODE_CONFIRM_SIGNIN, signin::ManageAccountsParams(), - signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN); + signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN, false); ASSERT_FALSE(ProfileChooserView::IsShowing()); CloseBrowserSynchronously(browser); }
diff --git a/chrome/browser/ui/webui/history_login_handler.cc b/chrome/browser/ui/webui/history_login_handler.cc index 01d39372..c7c54926 100644 --- a/chrome/browser/ui/webui/history_login_handler.cc +++ b/chrome/browser/ui/webui/history_login_handler.cc
@@ -59,7 +59,6 @@ Browser* browser = chrome::FindBrowserWithWebContents(web_ui()->GetWebContents()); browser->window()->ShowAvatarBubbleFromAvatarButton( - BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN, - signin::ManageAccountsParams(), - signin_metrics::AccessPoint::ACCESS_POINT_RECENT_TABS); + BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN, signin::ManageAccountsParams(), + signin_metrics::AccessPoint::ACCESS_POINT_RECENT_TABS, false); }
diff --git a/chrome/browser/ui/webui/options/sync_setup_handler.cc b/chrome/browser/ui/webui/options/sync_setup_handler.cc index c24a5fe..4e2457e 100644 --- a/chrome/browser/ui/webui/options/sync_setup_handler.cc +++ b/chrome/browser/ui/webui/options/sync_setup_handler.cc
@@ -384,7 +384,7 @@ if (!force_new_tab) { browser->window()->ShowAvatarBubbleFromAvatarButton( BrowserWindow::AVATAR_BUBBLE_MODE_REAUTH, - signin::ManageAccountsParams(), access_point); + signin::ManageAccountsParams(), access_point, false); } else { url = signin::GetReauthURL( access_point, signin_metrics::Reason::REASON_REAUTHENTICATION, @@ -394,7 +394,7 @@ if (!force_new_tab) { browser->window()->ShowAvatarBubbleFromAvatarButton( BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN, - signin::ManageAccountsParams(), access_point); + signin::ManageAccountsParams(), access_point, false); } else { url = signin::GetPromoURL( access_point, signin_metrics::Reason::REASON_SIGNIN_PRIMARY_ACCOUNT,
diff --git a/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc b/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc index 6d4c6f7..89c38b9f 100644 --- a/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc +++ b/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
@@ -33,6 +33,7 @@ #include "ui/base/l10n/l10n_util.h" #if defined(OS_CHROMEOS) +#include "ash/common/ash_switches.h" #include "ash/common/system/chromeos/devicetype_utils.h" #include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/ui/webui/chromeos/network_element_localized_strings_provider.h" @@ -109,6 +110,7 @@ #if defined(OS_CHROMEOS) {"optionsInMenuLabel", IDS_SETTINGS_OPTIONS_IN_MENU_LABEL}, {"largeMouseCursorLabel", IDS_SETTINGS_LARGE_MOUSE_CURSOR_LABEL}, + {"largeMouseCursorSizeLabel", IDS_SETTINGS_LARGE_MOUSE_CURSOR_SIZE_LABEL}, {"highContrastLabel", IDS_SETTINGS_HIGH_CONTRAST_LABEL}, {"stickyKeysLabel", IDS_SETTINGS_STICKY_KEYS_LABEL}, {"chromeVoxLabel", IDS_SETTINGS_CHROMEVOX_LABEL}, @@ -180,6 +182,10 @@ "showExperimentalA11yFeatures", base::CommandLine::ForCurrentProcess()->HasSwitch( chromeos::switches::kEnableExperimentalAccessibilityFeatures)); + + html_source->AddBoolean("enableAdjustableLargeCursor", + base::CommandLine::ForCurrentProcess()->HasSwitch( + ash::switches::kAshAdjustableLargeCursor)); #endif }
diff --git a/chrome/browser/ui/webui/settings/people_handler.cc b/chrome/browser/ui/webui/settings/people_handler.cc index 823dd2b..1d33be5d 100644 --- a/chrome/browser/ui/webui/settings/people_handler.cc +++ b/chrome/browser/ui/webui/settings/people_handler.cc
@@ -290,7 +290,7 @@ if (!force_new_tab) { browser->window()->ShowAvatarBubbleFromAvatarButton( BrowserWindow::AVATAR_BUBBLE_MODE_REAUTH, - signin::ManageAccountsParams(), access_point); + signin::ManageAccountsParams(), access_point, false); } else { url = signin::GetReauthURL( access_point, signin_metrics::Reason::REASON_REAUTHENTICATION, @@ -300,7 +300,7 @@ if (!force_new_tab) { browser->window()->ShowAvatarBubbleFromAvatarButton( BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN, - signin::ManageAccountsParams(), access_point); + signin::ManageAccountsParams(), access_point, false); } else { url = signin::GetPromoURL( access_point, signin_metrics::Reason::REASON_SIGNIN_PRIMARY_ACCOUNT,
diff --git a/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc b/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc index 4970af3..1c28984 100644 --- a/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc +++ b/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc
@@ -877,7 +877,8 @@ browser->window()->ShowAvatarBubbleFromAvatarButton( BrowserWindow::AVATAR_BUBBLE_MODE_ACCOUNT_MANAGEMENT, signin::ManageAccountsParams(), - signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN); + signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN, + false); } } }
diff --git a/chrome/browser/ui/webui/signin/login_ui_service.cc b/chrome/browser/ui/webui/signin/login_ui_service.cc index 80899978..9b8da8d 100644 --- a/chrome/browser/ui/webui/signin/login_ui_service.cc +++ b/chrome/browser/ui/webui/signin/login_ui_service.cc
@@ -83,7 +83,7 @@ error_message.empty() ? BrowserWindow::AVATAR_BUBBLE_MODE_CONFIRM_SIGNIN : BrowserWindow::AVATAR_BUBBLE_MODE_SHOW_ERROR, signin::ManageAccountsParams(), - signin_metrics::AccessPoint::ACCESS_POINT_EXTENSIONS); + signin_metrics::AccessPoint::ACCESS_POINT_EXTENSIONS, false); } }
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index 77ed648..666c385 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc
@@ -601,6 +601,10 @@ const char kAccessibilityLargeCursorEnabled[] = "settings.a11y.large_cursor_enabled"; +// A integer pref that specifies the size of large cursor for accessibility. +const char kAccessibilityLargeCursorDipSize[] = + "settings.a11y.large_cursor_dip_size"; + // A boolean pref which determines whether the sticky keys feature is enabled. const char kAccessibilityStickyKeysEnabled[] = "settings.a11y.sticky_keys_enabled";
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index 434ce26..95181d91 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h
@@ -227,6 +227,7 @@ // TODO(dmazzoni): move accessibility prefs out of chrome/. // http://crbug.com/594887 extern const char kAccessibilityLargeCursorEnabled[]; +extern const char kAccessibilityLargeCursorDipSize[]; extern const char kAccessibilityStickyKeysEnabled[]; extern const char kAccessibilitySpokenFeedbackEnabled[]; extern const char kAccessibilityHighContrastEnabled[];
diff --git a/chrome/installer/linux/debian/expected_deps_x64_jessie b/chrome/installer/linux/debian/expected_deps_x64_jessie index ce5804d..ab8362a 100644 --- a/chrome/installer/linux/debian/expected_deps_x64_jessie +++ b/chrome/installer/linux/debian/expected_deps_x64_jessie
@@ -2,7 +2,7 @@ libasound2 (>= 1.0.16) libatk1.0-0 (>= 1.12.4) libc6 (>= 2.15) -libcairo2 (>= 1.6.0) +libcairo2 (>= 1.2.4) libcups2 (>= 1.4.0) libdbus-1-3 (>= 1.1.4) libexpat1 (>= 2.0.1) @@ -12,9 +12,9 @@ libgconf-2-4 (>= 3.2.5) libgdk-pixbuf2.0-0 (>= 2.22.0) libglib2.0-0 (>= 2.41.1) -libgtk-3-0 (>= 3.3.16) -libnspr4 (>= 2:4.9-2~) -libnss3 (>= 2:3.13.4-2~) +libgtk2.0-0 (>= 2.24.0) +libnspr4 (>= 2:4.9-2~) | libnspr4-0d (>= 1.8.0.10) +libnss3 (>= 2:3.13.4-2~) | libnss3-1d (>= 3.12.4) libpango-1.0-0 (>= 1.14.0) libpangocairo-1.0-0 (>= 1.14.0) libstdc++6 (>= 4.8.1)
diff --git a/chrome/installer/linux/debian/expected_deps_x64_wheezy b/chrome/installer/linux/debian/expected_deps_x64_wheezy index cea1252..98c6bd6 100644 --- a/chrome/installer/linux/debian/expected_deps_x64_wheezy +++ b/chrome/installer/linux/debian/expected_deps_x64_wheezy
@@ -2,7 +2,7 @@ libasound2 (>= 1.0.16) libatk1.0-0 (>= 1.12.4) libc6 (>= 2.11) -libcairo2 (>= 1.6.0) +libcairo2 (>= 1.2.4) libcups2 (>= 1.4.0) libdbus-1-3 (>= 1.1.4) libexpat1 (>= 2.0.1) @@ -11,8 +11,8 @@ libgcc1 (>= 1:4.1.1) libgconf-2-4 (>= 2.31.1) libgdk-pixbuf2.0-0 (>= 2.22.0) -libglib2.0-0 (>= 2.28.0) -libgtk-3-0 (>= 3.3.16) +libglib2.0-0 (>= 2.26.0) +libgtk2.0-0 (>= 2.24.0) libnspr4 (>= 2:4.9-2~) | libnspr4-0d (>= 1.8.0.10) libnss3 (>= 2:3.13.4-2~) | libnss3-1d (>= 3.12.4) libpango1.0-0 (>= 1.14.0)
diff --git a/chrome/installer/linux/rpm/expected_deps_x86_64 b/chrome/installer/linux/rpm/expected_deps_x86_64 index ddaf415..5f6fb5a 100644 --- a/chrome/installer/linux/rpm/expected_deps_x86_64 +++ b/chrome/installer/linux/rpm/expected_deps_x86_64
@@ -36,13 +36,13 @@ libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_4.0.0)(64bit) libgconf-2.so.4()(64bit) -libgdk-3.so.0()(64bit) +libgdk-x11-2.0.so.0()(64bit) libgdk_pixbuf-2.0.so.0()(64bit) libgio-2.0.so.0()(64bit) libglib-2.0.so.0()(64bit) libgmodule-2.0.so.0()(64bit) libgobject-2.0.so.0()(64bit) -libgtk-3.so.0()(64bit) +libgtk-x11-2.0.so.0()(64bit) libm.so.6()(64bit) libm.so.6(GLIBC_2.2.5)(64bit) libnspr4.so()(64bit)
diff --git a/chrome/test/base/test_browser_window.h b/chrome/test/base/test_browser_window.h index d75242d..7c49275 100644 --- a/chrome/test/base/test_browser_window.h +++ b/chrome/test/base/test_browser_window.h
@@ -132,7 +132,8 @@ void ShowAvatarBubbleFromAvatarButton( AvatarBubbleMode mode, const signin::ManageAccountsParams& manage_accounts_params, - signin_metrics::AccessPoint access_point) override {} + signin_metrics::AccessPoint access_point, + bool is_source_keyboard) override {} int GetRenderViewHeightInsetWithDetachedBookmarkBar() override; void ExecuteExtensionCommand(const extensions::Extension* extension, const extensions::Command& command) override;
diff --git a/components/doodle/BUILD.gn b/components/doodle/BUILD.gn index 82e2dc0..6f4f06ab 100644 --- a/components/doodle/BUILD.gn +++ b/components/doodle/BUILD.gn
@@ -4,8 +4,9 @@ static_library("doodle") { sources = [ - "doodle_fetcher.cc", "doodle_fetcher.h", + "doodle_fetcher_impl.cc", + "doodle_fetcher_impl.h", "doodle_types.h", ] @@ -20,7 +21,7 @@ source_set("unit_tests") { testonly = true sources = [ - "doodle_fetcher_unittest.cc", + "doodle_fetcher_impl_unittest.cc", ] deps = [
diff --git a/components/doodle/doodle_fetcher.h b/components/doodle/doodle_fetcher.h index 951bbad..5745d6c 100644 --- a/components/doodle/doodle_fetcher.h +++ b/components/doodle/doodle_fetcher.h
@@ -5,104 +5,32 @@ #ifndef COMPONENTS_DOODLE_DOODLE_FETCHER_H_ #define COMPONENTS_DOODLE_DOODLE_FETCHER_H_ -#include <memory> -#include <string> -#include <utility> -#include <vector> - -#include "base/callback.h" -#include "base/macros.h" -#include "base/memory/ref_counted.h" -#include "base/memory/weak_ptr.h" +#include "base/callback_forward.h" #include "base/optional.h" #include "components/doodle/doodle_types.h" -#include "net/url_request/url_fetcher_delegate.h" -#include "net/url_request/url_request_context_getter.h" -#include "url/gurl.h" - -class GoogleURLTracker; - -namespace base { -class Clock; -class DictionaryValue; -class Value; -} namespace doodle { -// This class provides information about any recent doodle. -// It works asynchronously and calls a callback when finished fetching the -// information from the remote enpoint. -class DoodleFetcher : public net::URLFetcherDelegate { +// Interface for fetching the current doodle from the network. +// It asynchronously calls a callback when fetching the doodle information from +// the remote enpoint finishes. +// DoodleFetcherImpl is the default implementation; this interface exists to +// make it easy to use fakes or mocks in tests. +class DoodleFetcher { public: // Callback that is invoked when the fetching is done. // |doodle_config| will only contain a value if |state| is AVAILABLE. using FinishedCallback = base::OnceCallback<void( DoodleState state, const base::Optional<DoodleConfig>& doodle_config)>; - // Callback for JSON parsing to allow injecting platform-dependent code. - using ParseJSONCallback = base::Callback<void( - const std::string& unsafe_json, - const base::Callback<void(std::unique_ptr<base::Value> json)>& success, - const base::Callback<void(const std::string&)>& error)>; - DoodleFetcher(scoped_refptr<net::URLRequestContextGetter> download_context, - GoogleURLTracker* google_url_tracker, - const ParseJSONCallback& json_parsing_callback); - ~DoodleFetcher() override; + virtual ~DoodleFetcher() = default; // Fetches a doodle asynchronously. The |callback| is called with a // DoodleState indicating whether the request succeded in fetching a doodle. // If a fetch is already running, the callback will be queued and invoked with // result from the next completed request. - void FetchDoodle(FinishedCallback callback); - - // Overrides internal clock for testing purposes. - void SetClockForTesting(std::unique_ptr<base::Clock> clock) { - clock_ = std::move(clock); - } - - private: - // net::URLFetcherDelegate implementation. - void OnURLFetchComplete(const net::URLFetcher* source) override; - - // ParseJSONCallback Success callback - void OnJsonParsed(std::unique_ptr<base::Value> json); - // ParseJSONCallback Failure callback - void OnJsonParseFailed(const std::string& error_message); - - base::Optional<DoodleConfig> ParseDoodle( - const base::DictionaryValue& ddljson) const; - bool ParseImage(const base::DictionaryValue& image_dict, - const std::string& image_name, - DoodleImage* image) const; - void ParseBaseInformation(const base::DictionaryValue& ddljson, - DoodleConfig* config) const; - GURL ParseRelativeUrl(const base::DictionaryValue& dict_value, - const std::string& key) const; - - void RespondToAllCallbacks(DoodleState state, - const base::Optional<DoodleConfig>& config); - GURL GetGoogleBaseUrl() const; - - // Returns whether a fetch is still in progress. A fetch begins when a - // callback is added and ends when the last callback was called. - bool IsFetchInProgress() const { return !callbacks_.empty(); } - - // Parameters set from constructor. - scoped_refptr<net::URLRequestContextGetter> const download_context_; - ParseJSONCallback json_parsing_callback_; - GoogleURLTracker* google_url_tracker_; - - // Allow for an injectable tick clock for testing. - std::unique_ptr<base::Clock> clock_; - - std::vector<FinishedCallback> callbacks_; - std::unique_ptr<net::URLFetcher> fetcher_; - - base::WeakPtrFactory<DoodleFetcher> weak_ptr_factory_; - - DISALLOW_COPY_AND_ASSIGN(DoodleFetcher); + virtual void FetchDoodle(FinishedCallback callback) = 0; }; } // namespace doodle
diff --git a/components/doodle/doodle_fetcher.cc b/components/doodle/doodle_fetcher_impl.cc similarity index 82% rename from components/doodle/doodle_fetcher.cc rename to components/doodle/doodle_fetcher_impl.cc index 14dfc45..90fcdde 100644 --- a/components/doodle/doodle_fetcher.cc +++ b/components/doodle/doodle_fetcher_impl.cc
@@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "components/doodle/doodle_fetcher.h" +#include "components/doodle/doodle_fetcher_impl.h" #include <utility> @@ -74,7 +74,7 @@ DoodleConfig::DoodleConfig(const DoodleConfig& config) = default; DoodleConfig::~DoodleConfig() = default; -DoodleFetcher::DoodleFetcher( +DoodleFetcherImpl::DoodleFetcherImpl( scoped_refptr<net::URLRequestContextGetter> download_context, GoogleURLTracker* google_url_tracker, const ParseJSONCallback& json_parsing_callback) @@ -86,9 +86,9 @@ DCHECK(google_url_tracker_); } -DoodleFetcher::~DoodleFetcher() = default; +DoodleFetcherImpl::~DoodleFetcherImpl() = default; -void DoodleFetcher::FetchDoodle(FinishedCallback callback) { +void DoodleFetcherImpl::FetchDoodle(FinishedCallback callback) { if (IsFetchInProgress()) { callbacks_.push_back(std::move(callback)); return; // The callback will be called for the existing request's results. @@ -107,7 +107,7 @@ fetcher_->Start(); } -void DoodleFetcher::OnURLFetchComplete(const URLFetcher* source) { +void DoodleFetcherImpl::OnURLFetchComplete(const URLFetcher* source) { DCHECK_EQ(fetcher_.get(), source); std::unique_ptr<net::URLFetcher> free_fetcher = std::move(fetcher_); @@ -119,14 +119,14 @@ return; } - json_parsing_callback_.Run( - StripSafetyPreamble(std::move(json_string)), - base::Bind(&DoodleFetcher::OnJsonParsed, weak_ptr_factory_.GetWeakPtr()), - base::Bind(&DoodleFetcher::OnJsonParseFailed, - weak_ptr_factory_.GetWeakPtr())); + json_parsing_callback_.Run(StripSafetyPreamble(std::move(json_string)), + base::Bind(&DoodleFetcherImpl::OnJsonParsed, + weak_ptr_factory_.GetWeakPtr()), + base::Bind(&DoodleFetcherImpl::OnJsonParseFailed, + weak_ptr_factory_.GetWeakPtr())); } -void DoodleFetcher::OnJsonParsed(std::unique_ptr<base::Value> json) { +void DoodleFetcherImpl::OnJsonParsed(std::unique_ptr<base::Value> json) { std::unique_ptr<base::DictionaryValue> config = base::DictionaryValue::From(std::move(json)); if (!config.get()) { @@ -151,12 +151,12 @@ RespondToAllCallbacks(DoodleState::AVAILABLE, std::move(doodle)); } -void DoodleFetcher::OnJsonParseFailed(const std::string& error_message) { +void DoodleFetcherImpl::OnJsonParseFailed(const std::string& error_message) { DLOG(WARNING) << "JSON parsing failed: " << error_message; RespondToAllCallbacks(DoodleState::PARSING_ERROR, base::nullopt); } -base::Optional<DoodleConfig> DoodleFetcher::ParseDoodle( +base::Optional<DoodleConfig> DoodleFetcherImpl::ParseDoodle( const base::DictionaryValue& ddljson) const { DoodleConfig doodle; if (!ParseImage(ddljson, "large_image", &doodle.large_image)) { @@ -169,9 +169,9 @@ return doodle; } -bool DoodleFetcher::ParseImage(const base::DictionaryValue& image_parent, - const std::string& image_name, - DoodleImage* image) const { +bool DoodleFetcherImpl::ParseImage(const base::DictionaryValue& image_parent, + const std::string& image_name, + DoodleImage* image) const { DCHECK(image); const base::DictionaryValue* image_dict = nullptr; if (!image_parent.GetDictionary(image_name, &image_dict)) { @@ -189,8 +189,9 @@ return true; } -void DoodleFetcher::ParseBaseInformation(const base::DictionaryValue& ddljson, - DoodleConfig* config) const { +void DoodleFetcherImpl::ParseBaseInformation( + const base::DictionaryValue& ddljson, + DoodleConfig* config) const { config->search_url = ParseRelativeUrl(ddljson, "search_url"); config->target_url = ParseRelativeUrl(ddljson, "target_url"); config->fullpage_interactive_url = @@ -213,8 +214,9 @@ config->expiry_date = clock_->Now() + base::TimeDelta::FromMillisecondsD(ttl); } -GURL DoodleFetcher::ParseRelativeUrl(const base::DictionaryValue& dict_value, - const std::string& key) const { +GURL DoodleFetcherImpl::ParseRelativeUrl( + const base::DictionaryValue& dict_value, + const std::string& key) const { std::string str_url; dict_value.GetString(key, &str_url); if (str_url.empty()) { @@ -223,7 +225,7 @@ return GetGoogleBaseUrl().Resolve(str_url); } -void DoodleFetcher::RespondToAllCallbacks( +void DoodleFetcherImpl::RespondToAllCallbacks( DoodleState state, const base::Optional<DoodleConfig>& config) { for (auto& callback : callbacks_) { @@ -232,7 +234,7 @@ callbacks_.clear(); } -GURL DoodleFetcher::GetGoogleBaseUrl() const { +GURL DoodleFetcherImpl::GetGoogleBaseUrl() const { GURL cmd_line_url = google_util::CommandLineGoogleBaseURL(); if (cmd_line_url.is_valid()) { return cmd_line_url;
diff --git a/components/doodle/doodle_fetcher_impl.h b/components/doodle/doodle_fetcher_impl.h new file mode 100644 index 0000000..82c9c3f8 --- /dev/null +++ b/components/doodle/doodle_fetcher_impl.h
@@ -0,0 +1,107 @@ +// Copyright 2017 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 COMPONENTS_DOODLE_DOODLE_FETCHER_IMPL_H_ +#define COMPONENTS_DOODLE_DOODLE_FETCHER_IMPL_H_ + +#include <memory> +#include <string> +#include <utility> +#include <vector> + +#include "base/callback.h" +#include "base/macros.h" +#include "base/memory/ref_counted.h" +#include "base/memory/weak_ptr.h" +#include "base/optional.h" +#include "components/doodle/doodle_fetcher.h" +#include "components/doodle/doodle_types.h" +#include "net/url_request/url_fetcher_delegate.h" +#include "net/url_request/url_request_context_getter.h" +#include "url/gurl.h" + +class GoogleURLTracker; + +namespace base { +class Clock; +class DictionaryValue; +class Value; +} + +namespace doodle { + +// This class provides information about any recent doodle. +// It works asynchronously and calls a callback when finished fetching the +// information from the remote enpoint. +class DoodleFetcherImpl : public DoodleFetcher, public net::URLFetcherDelegate { + public: + // Callback for JSON parsing to allow injecting platform-dependent code. + using ParseJSONCallback = base::Callback<void( + const std::string& unsafe_json, + const base::Callback<void(std::unique_ptr<base::Value> json)>& success, + const base::Callback<void(const std::string&)>& error)>; + + DoodleFetcherImpl( + scoped_refptr<net::URLRequestContextGetter> download_context, + GoogleURLTracker* google_url_tracker, + const ParseJSONCallback& json_parsing_callback); + ~DoodleFetcherImpl() override; + + // Fetches a doodle asynchronously. The |callback| is called with a + // DoodleState indicating whether the request succeded in fetching a doodle. + // If a fetch is already running, the callback will be queued and invoked with + // result from the next completed request. + void FetchDoodle(FinishedCallback callback) override; + + // Overrides internal clock for testing purposes. + void SetClockForTesting(std::unique_ptr<base::Clock> clock) { + clock_ = std::move(clock); + } + + private: + // net::URLFetcherDelegate implementation. + void OnURLFetchComplete(const net::URLFetcher* source) override; + + // ParseJSONCallback Success callback + void OnJsonParsed(std::unique_ptr<base::Value> json); + // ParseJSONCallback Failure callback + void OnJsonParseFailed(const std::string& error_message); + + base::Optional<DoodleConfig> ParseDoodle( + const base::DictionaryValue& ddljson) const; + bool ParseImage(const base::DictionaryValue& image_dict, + const std::string& image_name, + DoodleImage* image) const; + void ParseBaseInformation(const base::DictionaryValue& ddljson, + DoodleConfig* config) const; + GURL ParseRelativeUrl(const base::DictionaryValue& dict_value, + const std::string& key) const; + + void RespondToAllCallbacks(DoodleState state, + const base::Optional<DoodleConfig>& config); + GURL GetGoogleBaseUrl() const; + + // Returns whether a fetch is still in progress. A fetch begins when a + // callback is added and ends when the last callback was called. + bool IsFetchInProgress() const { return !callbacks_.empty(); } + + // Parameters set from constructor. + scoped_refptr<net::URLRequestContextGetter> const download_context_; + ParseJSONCallback json_parsing_callback_; + GoogleURLTracker* google_url_tracker_; + + // Allow for an injectable tick clock for testing. + std::unique_ptr<base::Clock> clock_; + + std::vector<FinishedCallback> callbacks_; + std::unique_ptr<net::URLFetcher> fetcher_; + + base::WeakPtrFactory<DoodleFetcherImpl> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(DoodleFetcherImpl); +}; + +} // namespace doodle + +#endif // COMPONENTS_DOODLE_DOODLE_FETCHER_IMPL_H_
diff --git a/components/doodle/doodle_fetcher_unittest.cc b/components/doodle/doodle_fetcher_impl_unittest.cc similarity index 91% rename from components/doodle/doodle_fetcher_unittest.cc rename to components/doodle/doodle_fetcher_impl_unittest.cc index 5486b4c..5ab04de 100644 --- a/components/doodle/doodle_fetcher_unittest.cc +++ b/components/doodle/doodle_fetcher_impl_unittest.cc
@@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "components/doodle/doodle_fetcher.h" +#include "components/doodle/doodle_fetcher_impl.h" #include <string> #include <utility> @@ -68,9 +68,9 @@ } // namespace -class DoodleFetcherTest : public testing::Test { +class DoodleFetcherImplTest : public testing::Test { public: - DoodleFetcherTest() + DoodleFetcherImplTest() : url_(GURL(GoogleURLTracker::kDefaultGoogleHomepage)), google_url_tracker_(base::MakeUnique<GoogleURLTrackerClientStub>(), GoogleURLTracker::UNIT_TEST_MODE), @@ -113,7 +113,7 @@ return url_fetcher; } - DoodleFetcher::FinishedCallback CreateResponseSavingCallback( + DoodleFetcherImpl::FinishedCallback CreateResponseSavingCallback( DoodleState* state_out, base::Optional<DoodleConfig>* config_out) { return base::BindOnce( @@ -129,7 +129,7 @@ state_out, config_out); } - DoodleFetcher* doodle_fetcher() { return &doodle_fetcher_; } + DoodleFetcherImpl* doodle_fetcher() { return &doodle_fetcher_; } GURL GetGoogleBaseURL() { return google_url_tracker_.google_url(); } @@ -143,10 +143,10 @@ net::TestURLFetcherFactory url_fetcher_factory_; base::SimpleTestClock* clock_; // Owned by the doodle_fetcher. GoogleURLTracker google_url_tracker_; - DoodleFetcher doodle_fetcher_; + DoodleFetcherImpl doodle_fetcher_; }; -TEST_F(DoodleFetcherTest, ReturnsFromFetchWithoutError) { +TEST_F(DoodleFetcherImplTest, ReturnsFromFetchWithoutError) { DoodleState state(DoodleState::NO_DOODLE); base::Optional<DoodleConfig> response; @@ -161,7 +161,7 @@ EXPECT_TRUE(response.has_value()); } -TEST_F(DoodleFetcherTest, ReturnsFrom404FetchWithError) { +TEST_F(DoodleFetcherImplTest, ReturnsFrom404FetchWithError) { DoodleState state(DoodleState::NO_DOODLE); base::Optional<DoodleConfig> response; @@ -173,7 +173,7 @@ EXPECT_FALSE(response.has_value()); } -TEST_F(DoodleFetcherTest, ReturnsErrorForInvalidJson) { +TEST_F(DoodleFetcherImplTest, ReturnsErrorForInvalidJson) { DoodleState state(DoodleState::NO_DOODLE); base::Optional<DoodleConfig> response; @@ -185,7 +185,7 @@ EXPECT_FALSE(response.has_value()); } -TEST_F(DoodleFetcherTest, ReturnsErrorForIncompleteJson) { +TEST_F(DoodleFetcherImplTest, ReturnsErrorForIncompleteJson) { DoodleState state(DoodleState::NO_DOODLE); base::Optional<DoodleConfig> response; @@ -197,7 +197,7 @@ EXPECT_FALSE(response.has_value()); } -TEST_F(DoodleFetcherTest, ResponseContainsValidBaseInformation) { +TEST_F(DoodleFetcherImplTest, ResponseContainsValidBaseInformation) { DoodleState state(DoodleState::NO_DOODLE); base::Optional<DoodleConfig> response; @@ -235,7 +235,7 @@ EXPECT_THAT(config.expiry_date, Eq(TimeFromNow(55000))); } -TEST_F(DoodleFetcherTest, DoodleExpiresWithinThirtyDaysForTooLargeTTL) { +TEST_F(DoodleFetcherImplTest, DoodleExpiresWithinThirtyDaysForTooLargeTTL) { DoodleState state(DoodleState::NO_DOODLE); base::Optional<DoodleConfig> response; @@ -252,7 +252,7 @@ Eq(TimeFromNow(30ul * 24 * 60 * 60 * 1000 /* ms */))); // 30 days } -TEST_F(DoodleFetcherTest, DoodleExpiresNowWithNegativeTTL) { +TEST_F(DoodleFetcherImplTest, DoodleExpiresNowWithNegativeTTL) { DoodleState state(DoodleState::NO_DOODLE); base::Optional<DoodleConfig> response; @@ -268,7 +268,7 @@ EXPECT_THAT(response.value().expiry_date, Eq(TimeFromNow(0))); } -TEST_F(DoodleFetcherTest, DoodleExpiresNowWithoutValidTTL) { +TEST_F(DoodleFetcherImplTest, DoodleExpiresNowWithoutValidTTL) { DoodleState state(DoodleState::NO_DOODLE); base::Optional<DoodleConfig> response; @@ -283,7 +283,7 @@ EXPECT_THAT(response.value().expiry_date, Eq(TimeFromNow(0))); } -TEST_F(DoodleFetcherTest, ReturnsNoDoodleForMissingLargeImageUrl) { +TEST_F(DoodleFetcherImplTest, ReturnsNoDoodleForMissingLargeImageUrl) { DoodleState state(DoodleState::AVAILABLE); base::Optional<DoodleConfig> response; @@ -298,7 +298,7 @@ EXPECT_FALSE(response.has_value()); } -TEST_F(DoodleFetcherTest, EmptyResponsesCausesNoDoodleState) { +TEST_F(DoodleFetcherImplTest, EmptyResponsesCausesNoDoodleState) { DoodleState state(DoodleState::AVAILABLE); base::Optional<DoodleConfig> response; @@ -310,7 +310,7 @@ EXPECT_FALSE(response.has_value()); } -TEST_F(DoodleFetcherTest, ResponseContainsExactlyTheSampleImages) { +TEST_F(DoodleFetcherImplTest, ResponseContainsExactlyTheSampleImages) { DoodleState state(DoodleState::NO_DOODLE); base::Optional<DoodleConfig> response; @@ -374,7 +374,7 @@ EXPECT_TRUE(config.large_cta_image.is_cta); } -TEST_F(DoodleFetcherTest, RespondsToMultipleRequestsWithSameFetcher) { +TEST_F(DoodleFetcherImplTest, RespondsToMultipleRequestsWithSameFetcher) { DoodleState state1(DoodleState::NO_DOODLE); DoodleState state2(DoodleState::NO_DOODLE); base::Optional<DoodleConfig> response1; @@ -403,7 +403,7 @@ EXPECT_TRUE(response2.has_value()); } -TEST_F(DoodleFetcherTest, ReceivesBaseUrlFromTracker) { +TEST_F(DoodleFetcherImplTest, ReceivesBaseUrlFromTracker) { doodle_fetcher()->FetchDoodle( CreateResponseSavingCallback(/*state=*/nullptr, /*response=*/nullptr)); @@ -411,7 +411,7 @@ Eq(GetGoogleBaseURL().Resolve(kDoodleConfigPath))); } -TEST_F(DoodleFetcherTest, OverridesBaseUrlWithCommandLineArgument) { +TEST_F(DoodleFetcherImplTest, OverridesBaseUrlWithCommandLineArgument) { base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( switches::kGoogleBaseURL, "http://www.google.kz");
diff --git a/components/ntp_snippets/content_suggestions_metrics.cc b/components/ntp_snippets/content_suggestions_metrics.cc index 95a1381..a88d5d1a 100644 --- a/components/ntp_snippets/content_suggestions_metrics.cc +++ b/components/ntp_snippets/content_suggestions_metrics.cc
@@ -59,8 +59,8 @@ "NewTabPage.ContentSuggestions.MovedUpCategoryNewIndex"; const char kHistogramCategoryDismissed[] = "NewTabPage.ContentSuggestions.CategoryDismissed"; -const char kHistogramContentSuggestionsTimeSinceLastBackgroundFetch[] = - "NewTabPage.ContentSuggestions.TimeSinceLastBackgroundFetch"; +const char kHistogramTimeSinceSuggestionFetched[] = + "NewTabPage.ContentSuggestions.TimeSinceSuggestionFetched"; const char kPerCategoryHistogramFormat[] = "%s.%s"; @@ -235,6 +235,14 @@ LogCategoryHistogramScore(kHistogramShownScore, category, score); + if (category.IsKnownCategory(KnownCategories::ARTICLES)) { + // Records the time since the fetch time of the displayed snippet. + UMA_HISTOGRAM_CUSTOM_TIMES( + kHistogramTimeSinceSuggestionFetched, base::Time::Now() - fetch_date, + base::TimeDelta::FromSeconds(1), base::TimeDelta::FromDays(7), + /*bucket_count=*/100); + } + // TODO(markusheintz): Discuss whether the code below should be move into a // separate method called OnSuggestionsListShown. // When the first of the articles suggestions is shown, then we count this as @@ -242,14 +250,6 @@ if (category.IsKnownCategory(KnownCategories::ARTICLES) && position_in_category == 0) { RecordContentSuggestionsUsage(); - - // Records the time since the last background fetch of the remote content - // suggestions. - UMA_HISTOGRAM_CUSTOM_TIMES( - kHistogramContentSuggestionsTimeSinceLastBackgroundFetch, - base::Time::Now() - fetch_date, base::TimeDelta::FromSeconds(1), - base::TimeDelta::FromDays(7), - /*bucket_count=*/100); } }
diff --git a/content/browser/frame_host/render_widget_host_view_guest_unittest.cc b/content/browser/frame_host/render_widget_host_view_guest_unittest.cc index 9a7c231..bfba0e3 100644 --- a/content/browser/frame_host/render_widget_host_view_guest_unittest.cc +++ b/content/browser/frame_host/render_widget_host_view_guest_unittest.cc
@@ -114,7 +114,13 @@ } // namespace -TEST_F(RenderWidgetHostViewGuestTest, VisibilityTest) { +// Fails on Windows, crbug.com/695375. +#if defined(OS_WIN) +#define MAYBE_VisibilityTest DISABLED_VisibilityTest +#else +#define MAYBE_VisibilityTest VisibilityTest +#endif +TEST_F(RenderWidgetHostViewGuestTest, MAYBE_VisibilityTest) { view_->Show(); ASSERT_TRUE(view_->IsShowing());
diff --git a/docs/ios_build_instructions.md b/docs/ios_build_instructions.md index b23138d1..2d7a013 100644 --- a/docs/ios_build_instructions.md +++ b/docs/ios_build_instructions.md
@@ -1,6 +1,6 @@ # Checking out and building Chromium for iOS -There are instructions for other platforms linked from the +There are instructions for other platforms linked from the [get the code](get_the_code.md) page. ## Instructions for Google Employees @@ -16,10 +16,10 @@ * [Xcode](https://developer.apple.com/xcode) 8.0+. * The OS X 10.10 SDK. Run - ```shell + ```shell $ ls `xcode-select -p`/Platforms/MacOSX.platform/Developer/SDKs ``` - + to check whether you have it. Building with the 10.11 SDK works too, but the releases currently use the 10.10 SDK. * The current version of the JDK (required for the Closure compiler). @@ -81,7 +81,7 @@ Since the iOS build is a bit more complicated than a desktop build, we provide `ios/build/tools/setup-gn.py`, which will create four appropriately configured build directories under `out` for Release and Debug device and simulator -builds, and generates an appropriate Xcode workspace as well. +builds, and generates an appropriate Xcode workspace as well. This script is run automatically by fetch (as part of `gclient runhooks`). @@ -101,10 +101,129 @@ file is updated (either by you or after rebasing). If you forget to run it, the list of targets and files in the Xcode solution may be stale. -You can also follow the manual instructions on the +You can also follow the manual instructions on the [Mac page](mac_build_instructions.md), but make sure you set the GN arg `target_os="ios"`. +## Building for device + +To be able to build and run Chromium and the tests for devices, you need to +have an Apple developer account (a free one will work) and the appropriate +provisioning profiles, then configure the build to use them. + +### Code signing identity + +Please refer to the Apple documentation on how to get a code signing identity +and certificates. You can check that you have a code signing identity correctly +installed by running the following command. + +```shell +$ xcrun security find-identity -v -p codesigning + 1) 0123456789ABCDEF0123456789ABCDEF01234567 "iPhone Developer: someone@example.com (XXXXXXXXXX)" + 1 valid identities found +``` + +If the command output says you have zero valid identities, then you do not +have a code signing identity installed and need to get one from Apple. If +you have more than one identity, the build system may select the wrong one +automatically, and you can use the `ios_code_signing_identity` gn variable +to control which one to use by setting it to the identity hash, e.g. to +`"0123456789ABCDEF0123456789ABCDEF01234567"`. + +### Mobile provisioning profiles + +Once you have the code signing identity, you need to decide on a prefix +for the application bundle identifier. This is controlled by the gn variable +`ios_app_bundle_id_prefix` and usually corresponds to a reversed domain name +(the default value is `"org.chromium"`). + +You then need to request provisioning profiles from Apple for your devices +for the following bundle identifiers to build and run Chromium with these +application extensions: + +- `${prefix}.chrome.ios.herebedragons` +- `${prefix}.chrome.ios.herebedragons.ShareExtension` +- `${prefix}.chrome.ios.herebedragons.TodayExtension` + +All these certificates need to have the "App Groups" +(`com.apple.security.application-groups`) capability enabled for +the following groups: + +- `group.${prefix}.chrome` +- `group.${prefix}.common` + +The `group.${prefix}.chrome` is only shared by Chromium and its extensions +to share files and configurations while the `group.${prefix}.common` is shared +with Chromium and other applications from the same organisation and can be used +to send commands to Chromium. + +### Mobile provisioning profiles for tests + +In addition to that, you need provisioning profiles for the individual test +suites that you want to run. Their bundle identifier depends on whether the +gn variable `ios_automatically_manage_certs` is set to true (the default) +or false. + +If set to true, then you just need a provisioning profile for the bundle +identifier `${prefix}.gtest.generic-unit-test` but you can only have a +single test application installed on the device (all the test application +will share the same bundle identifier). + +If set to false, then you need a different provisioning profile for each +test application. Those provisioning profile will have a bundle identifier +matching the following pattern `${prefix}.gtest.${test-suite-name}` where +`${test-suite-name}` is the name of the test suite with underscores changed +to dashes (e.g. `base_unittests` app will use `${prefix}.gest.base-unittests` +as bundle identifier). + +To be able to run the EarlGrey tests on a device, you'll need two provisioning +profiles for EarlGrey and OCHamcrest frameworks: + +- `${prefix}.test.OCHamcrest` +- `${prefix}.test.EarlGrey` + +In addition to that, then you'll need one additional provisioning profile for +the XCTest module too. This module bundle identifier depends on whether the +gn variable `ios_automatically_manage_certs` is set to true or false. If set +to true, then `${prefix}.test.gtest.generic-unit-test.generic-unit-test-module` +will be used, otherwise it will match the following pattern: +`${prefix}.test.${test-suite-name}.${test-suite-name}-module`. + +### Other applications + +Other applications like `ios_web_shell` usually will require mobile provisioning +profiles with bundle identifiers that may usually match the following pattern +`${prefix}.${application-name}` and may require specific capabilities. + +Generally, if the mobile provisioning profile is missing then the code signing +step will fail and will print the bundle identifier of the bundle that could not +be signed on the command line, e.g.: + +```shell +$ ninja -C out/Debug-iphoneos ios_web_shell +ninja: Entering directory `out/Debug-iphoneos' +FAILED: ios_web_shell.app/ios_web_shell ios_web_shell.app/_CodeSignature/CodeResources ios_web_shell.app/embedded.mobileprovision +python ../../build/config/ios/codesign.py code-sign-bundle -t=iphoneos -i=0123456789ABCDEF0123456789ABCDEF01234567 -e=../../build/config/ios/entitlements.plist -b=obj/ios/web/shell/ios_web_shell ios_web_shell.app +Error: no mobile provisioning profile found for "org.chromium.ios-web-shell". +ninja: build stopped: subcommand failed. +``` + +Here, the build is failing because there are no mobile provisioning profiles +installed that could sign the `ios_web_shell.app` bundle with the identity +`0123456789ABCDEF0123456789ABCDEF01234567`. To fix the build, you'll need to +request such a mobile provisioning profile from Apple. + +You can inspect the file passed via the `-e` flag to the `codesign.py` script +to check which capabilites are required for the mobile provisioning profile +(e.g. `src/build/config/ios/entitlements.plist` for the above build error, +remember that the paths are relative to the build directory, not to the source +directory). + +If the required capabilities are not enabled on the mobile provisioning profile, +then it will be impossible to install the application on a device (Xcode will +display an error stating that "The application was signed with invalid +entitlements"). + ## Running apps from the commandline Any target that is built and runs on the bots (see [below](#Troubleshooting))
diff --git a/docs/ios_voiceover.md b/docs/ios_voiceover.md new file mode 100644 index 0000000..3f8a5cd --- /dev/null +++ b/docs/ios_voiceover.md
@@ -0,0 +1,48 @@ +# VoiceOver + +VoiceOver is an Accessibility feature available on Apple platforms. It provides +an interface between your app and your blind or low-vision users. In short, it's +a screen reader. + +## Overview + +[Activating VoiceOver](#Activate-VoiceOver) fundamentally changes the touch +interface of an iOS device, so when you activate it, you need to know these +basic commands to navigate: + +- VoiceOver focuses an item on the screen and speaks it to the user. +- To navigate between items, you can either flick left/right to go to the + previous/next item, or you can drag your finger around the screen. +- Double tapping anywhere on the screen activates the item (no need to double + tap exactly on the item on the screen). +- Items have a label (describing the item) and can have among others: + - traits (describing the item: a header, a button, selection state, etc.); + - a hint (describing how to interact with the item). + +The following video is a great summary. The content is succinct, high quality +and up-to-date, even though the video quality is low and the demo is on an +iPhone 3GS. + +[![Video screenshot]][Video link] + +## Activate VoiceOver + +Go to `Settings > General > Accessibility > VoiceOver` and toggle the top +switch. + +Alternatively, if you are activating/deactivating VoiceOver often, I'd advise +to set it as the Accessibility Shortcut. When enabled, a triple tap on the Home +button will toggle VoiceOver ON and OFF from anywhere. +``` +Settings > General > Accessibility > Accessibility Shortcut +``` + +## Additional Resources + +- [Apple presentation of VoiceOver] +- [AppleVis intro to VoiceOver] + +[Apple presentation of VoiceOver]: http://www.apple.com/accessibility/osx/voiceover/ +[AppleVis intro to VoiceOver]: http://www.applevis.com/guides/ios-miscellaneous-voiceover/intro-ios-accessibility-blind-and-low-vision-users +[Video screenshot]: https://img.youtube.com/vi/WxQ2qKShvmc/0.jpg +[Video link]: https://www.youtube.com/watch?v=WxQ2qKShvmc "iPhone VoiceOver Function For People With Disabilities"
diff --git a/headless/OWNERS b/headless/OWNERS index bfb2bee..af4e1d8 100644 --- a/headless/OWNERS +++ b/headless/OWNERS
@@ -1,5 +1,6 @@ skyostil@chromium.org alexclarke@chromium.org altimin@chromium.org +eseckler@chromium.org # TEAM: headless-dev@chromium.org
diff --git a/ios/chrome/browser/ui/bookmarks/BUILD.gn b/ios/chrome/browser/ui/bookmarks/BUILD.gn index 807c3fd..5887084 100644 --- a/ios/chrome/browser/ui/bookmarks/BUILD.gn +++ b/ios/chrome/browser/ui/bookmarks/BUILD.gn
@@ -143,6 +143,7 @@ } source_set("eg_tests") { + configs += [ "//build/config/compiler:enable_arc" ] testonly = true sources = [ "bookmarks_egtest.mm",
diff --git a/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm b/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm index 64d09643..05a248d0 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm
@@ -42,6 +42,10 @@ #include "ui/base/models/tree_node_iterator.h" #include "url/gurl.h" +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + using chrome_test_util::ButtonWithAccessibilityLabel; using chrome_test_util::ButtonWithAccessibilityLabelId; @@ -167,10 +171,10 @@ assertWithMatcher:grey_notNil()]; // Add the bookmark from the UI. - [[self class] bookmarkCurrentTabWithTitle:bookmarkTitle]; + [BookmarksTestCase bookmarkCurrentTabWithTitle:bookmarkTitle]; // Verify the bookmark is set. - [[self class] assertBookmarksWithTitle:bookmarkTitle expectedCount:1]; + [BookmarksTestCase assertBookmarksWithTitle:bookmarkTitle expectedCount:1]; NSString* const kStarLitLabel = !IsCompact() ? l10n_util::GetNSString(IDS_TOOLTIP_STAR) @@ -189,7 +193,7 @@ performAction:grey_tap()]; // Verify the bookmark is not in the BookmarkModel. - [[self class] assertBookmarksWithTitle:bookmarkTitle expectedCount:0]; + [BookmarksTestCase assertBookmarksWithTitle:bookmarkTitle expectedCount:0]; NSString* const kStarUnlitLabel = !IsCompact() ? l10n_util::GetNSString(IDS_TOOLTIP_STAR) @@ -210,8 +214,8 @@ } // Close the opened tab. - base::scoped_nsobject<GenericChromeCommand> command( - [[GenericChromeCommand alloc] initWithTag:IDC_CLOSE_TAB]); + GenericChromeCommand* command = + [[GenericChromeCommand alloc] initWithTag:IDC_CLOSE_TAB]; chrome_test_util::RunCommandWithActiveViewController(command); } @@ -222,10 +226,10 @@ NSString* bookmarkTitle = @"smokeTapBookmark"; // Load a bookmark into the bookmark model. - [[self class] addBookmark:bookmarkURL withTitle:bookmarkTitle]; + [BookmarksTestCase addBookmark:bookmarkURL withTitle:bookmarkTitle]; // Open the UI for Bookmarks. - [[self class] openMobileBookmarks]; + [BookmarksTestCase openMobileBookmarks]; // Verify bookmark is visible. [[EarlGrey @@ -252,14 +256,14 @@ chrome_test_util::OpenNewTab(); [ChromeEarlGrey loadURL:secondURL]; - [[self class] bookmarkCurrentTabWithTitle:@"my bookmark"]; - [[self class] assertBookmarksWithTitle:@"my bookmark" expectedCount:1]; + [BookmarksTestCase bookmarkCurrentTabWithTitle:@"my bookmark"]; + [BookmarksTestCase assertBookmarksWithTitle:@"my bookmark" expectedCount:1]; } // Try navigating to the bookmark screen, and selecting a bookmark. - (void)testSelectBookmark { - [[self class] setupStandardBookmarks]; - [[self class] openMobileBookmarks]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; // Tap on one of the standard bookmark. Verify that it loads. [[EarlGrey selectElementWithMatcher:grey_text(@"Second URL")] @@ -277,8 +281,8 @@ // Try deleting a bookmark, then undoing that delete. - (void)testUndoDeleteBookmark { - [[self class] setupStandardBookmarks]; - [[self class] openMobileBookmarks]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; // Load the menu for a bookmark. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Second URL Info")] @@ -289,7 +293,7 @@ performAction:grey_tap()]; // Wait until it's gone. - [[self class] waitForDeletionOfBookmarkWithTitle:@"Second URL"]; + [BookmarksTestCase waitForDeletionOfBookmarkWithTitle:@"Second URL"]; // Press undo [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Undo")] @@ -302,8 +306,8 @@ // Try deleting a bookmark from the edit screen, then undoing that delete. - (void)testUndoDeleteBookmarkFromEditScreen { - [[self class] setupStandardBookmarks]; - [[self class] openMobileBookmarks]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; // Load the menu for a bookmark. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Second URL Info")] @@ -339,11 +343,11 @@ // Try moving bookmarks, then undoing that move. - (void)testUndoMoveBookmark { - [[self class] setupStandardBookmarks]; - [[self class] openMobileBookmarks]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; // Verify that folder 2 only has 1 child. - [[self class] assertChildCount:1 ofFolderWithName:@"Folder 2"]; + [BookmarksTestCase assertChildCount:1 ofFolderWithName:@"Folder 2"]; // Load the menu for a bookmark. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Second URL Info")] @@ -372,7 +376,7 @@ // Verify that folder 2 has 3 children now, and that they are no longer // visible. - [[self class] assertChildCount:3 ofFolderWithName:@"Folder 2"]; + [BookmarksTestCase assertChildCount:3 ofFolderWithName:@"Folder 2"]; [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Second URL")] assertWithMatcher:grey_notVisible()]; @@ -387,12 +391,12 @@ assertWithMatcher:grey_notNil()]; // Verify that folder 2 is back to one child. - [[self class] assertChildCount:1 ofFolderWithName:@"Folder 2"]; + [BookmarksTestCase assertChildCount:1 ofFolderWithName:@"Folder 2"]; } - (void)testLabelUpdatedUponMove { - [[self class] setupStandardBookmarks]; - [[self class] openMobileBookmarks]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; // Load the menu for a bookmark. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"First URL Info")] @@ -407,7 +411,7 @@ performAction:grey_tap()]; // Create a new folder with default name. - [[self class] addFolderWithName:nil]; + [BookmarksTestCase addFolderWithName:nil]; // Verify that the editor is present. [[EarlGrey @@ -433,7 +437,7 @@ expectedURLContent)] assertWithMatcher:grey_notNil()]; - [[self class] starCurrentTab]; + [BookmarksTestCase starCurrentTab]; // Verify the snackbar title. [[EarlGrey selectElementWithMatcher:grey_accessibilityLabel(@"Bookmarked")] @@ -446,7 +450,7 @@ performAction:grey_tap()]; // Verify that the newly-created bookmark is in the BookmarkModel. - [[self class] + [BookmarksTestCase assertBookmarksWithTitle:base::SysUTF8ToNSString(expectedURLContent) expectedCount:1]; @@ -455,21 +459,21 @@ selectElementWithMatcher:grey_accessibilityID(@"Single Bookmark Editor")] assertWithMatcher:grey_notNil()]; - [[self class] assertFolderName:@"Mobile Bookmarks"]; + [BookmarksTestCase assertFolderName:@"Mobile Bookmarks"]; // Tap the Folder button. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Change Folder")] performAction:grey_tap()]; // Create a new folder with default name. - [[self class] addFolderWithName:nil]; + [BookmarksTestCase addFolderWithName:nil]; // Verify that the editor is present. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Single Bookmark Editor")] assertWithMatcher:grey_notNil()]; - [[self class] assertFolderExists:@"New Folder"]; + [BookmarksTestCase assertFolderExists:@"New Folder"]; } // Tests that changing a folder's title in edit mode works as expected. @@ -477,11 +481,11 @@ NSString* existingFolderTitle = @"Folder 1"; NSString* newFolderTitle = @"New Folder Title"; - [[self class] setupStandardBookmarks]; - [[self class] openMobileBookmarks]; - [[self class] openEditBookmarkFolderWithFolderTitle:existingFolderTitle]; - [[self class] renameBookmarkFolderWithFolderTitle:newFolderTitle]; - [[self class] closeEditBookmarkFolder]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; + [BookmarksTestCase openEditBookmarkFolderWithFolderTitle:existingFolderTitle]; + [BookmarksTestCase renameBookmarkFolderWithFolderTitle:newFolderTitle]; + [BookmarksTestCase closeEditBookmarkFolder]; if (IsCompact()) { // Exit from bookmarks modal. IPad shows bookmarks in tab. @@ -490,7 +494,7 @@ } // Verify that the change has been made. - [[self class] openMobileBookmarks]; + [BookmarksTestCase openMobileBookmarks]; [[EarlGrey selectElementWithMatcher:grey_accessibilityID(existingFolderTitle)] assertWithMatcher:grey_nil()]; [[EarlGrey selectElementWithMatcher:grey_accessibilityID(newFolderTitle)] @@ -500,8 +504,8 @@ // Tests that the default folder bookmarks are saved in is updated to the last // used folder. - (void)testStickyDefaultFolder { - [[self class] setupStandardBookmarks]; - [[self class] openMobileBookmarks]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; // Tap on the top-right button. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"First URL Info")] @@ -516,7 +520,7 @@ performAction:grey_tap()]; // Create a new folder. - [[self class] addFolderWithName:@"Sticky Folder"]; + [BookmarksTestCase addFolderWithName:@"Sticky Folder"]; // Verify that the editor is present. [[EarlGrey @@ -544,7 +548,8 @@ "http://ios/testing/data/http_server_files/fullscreen.html"); NSString* const bookmarkedURLString = base::SysUTF8ToNSString(bookmarkedURL.spec()); - [[self class] assertBookmarksWithTitle:bookmarkedURLString expectedCount:0]; + [BookmarksTestCase assertBookmarksWithTitle:bookmarkedURLString + expectedCount:0]; // Open the page. std::string expectedURLContent = bookmarkedURL.GetContent(); [ChromeEarlGrey loadURL:bookmarkedURL]; @@ -553,10 +558,10 @@ assertWithMatcher:grey_notNil()]; // Verify that the folder has only one element. - [[self class] assertChildCount:1 ofFolderWithName:@"Sticky Folder"]; + [BookmarksTestCase assertChildCount:1 ofFolderWithName:@"Sticky Folder"]; // Bookmark the page. - [[self class] starCurrentTab]; + [BookmarksTestCase starCurrentTab]; // Verify the snackbar title. [[EarlGrey selectElementWithMatcher:grey_accessibilityLabel( @@ -564,17 +569,18 @@ assertWithMatcher:grey_sufficientlyVisible()]; // Verify that the newly-created bookmark is in the BookmarkModel. - [[self class] assertBookmarksWithTitle:bookmarkedURLString expectedCount:1]; + [BookmarksTestCase assertBookmarksWithTitle:bookmarkedURLString + expectedCount:1]; // Verify that the folder has now two elements. - [[self class] assertChildCount:2 ofFolderWithName:@"Sticky Folder"]; + [BookmarksTestCase assertChildCount:2 ofFolderWithName:@"Sticky Folder"]; } // Tests that changes to the parent folder from the Single Bookmark Controller // are saved to the bookmark only when saving the results. - (void)testMoveDoesSaveOnSave { - [[self class] setupStandardBookmarks]; - [[self class] openTopLevelBookmarksFolder]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openTopLevelBookmarksFolder]; // Tap on the top-right button. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"First URL Info")] @@ -589,7 +595,7 @@ performAction:grey_tap()]; // Create a new folder. - [[self class] addFolderWithName:nil]; + [BookmarksTestCase addFolderWithName:nil]; // Verify that the editor is present. [[EarlGrey @@ -597,7 +603,7 @@ assertWithMatcher:grey_sufficientlyVisible()]; // Check that the new folder doesn't contain the bookmark. - [[self class] assertChildCount:0 ofFolderWithName:@"New Folder"]; + [BookmarksTestCase assertChildCount:0 ofFolderWithName:@"New Folder"]; // Tap the Done button. [[EarlGrey selectElementWithMatcher:BookmarksDoneButton()] @@ -607,7 +613,7 @@ assertWithMatcher:grey_notVisible()]; // Check that the new folder contains the bookmark. - [[self class] assertChildCount:1 ofFolderWithName:@"New Folder"]; + [BookmarksTestCase assertChildCount:1 ofFolderWithName:@"New Folder"]; // Dismiss the bookmarks screen. if (IsCompact()) { @@ -617,13 +623,13 @@ } // Check that the new folder still contains the bookmark. - [[self class] assertChildCount:1 ofFolderWithName:@"New Folder"]; + [BookmarksTestCase assertChildCount:1 ofFolderWithName:@"New Folder"]; } // Test thats editing a single bookmark correctly persists data. - (void)testSingleBookmarkEdit { - [[self class] setupStandardBookmarks]; - [[self class] openTopLevelBookmarksFolder]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openTopLevelBookmarksFolder]; // Load the menu for a bookmark. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"First URL Info")] @@ -665,14 +671,15 @@ // Verify that the bookmark was updated. [[EarlGrey selectElementWithMatcher:ButtonWithAccessibilityLabel(@"n5")] assertWithMatcher:grey_sufficientlyVisible()]; - [[self class] assertExistenceOfBookmarkWithURL:@"http://www.a.fr" name:@"n5"]; + [BookmarksTestCase assertExistenceOfBookmarkWithURL:@"http://www.a.fr" + name:@"n5"]; } // Tests that cancelling editing a single bookmark correctly doesn't persist // data. - (void)testSingleBookmarkCancelEdit { - [[self class] setupStandardBookmarks]; - [[self class] openTopLevelBookmarksFolder]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openTopLevelBookmarksFolder]; // Load the menu for a bookmark. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"First URL Info")] @@ -714,14 +721,14 @@ // Verify that the bookmark was not updated. [[EarlGrey selectElementWithMatcher:ButtonWithAccessibilityLabel(@"n5")] assertWithMatcher:grey_notVisible()]; - [[self class] assertAbsenceOfBookmarkWithURL:@"http://www.a.fr"]; + [BookmarksTestCase assertAbsenceOfBookmarkWithURL:@"http://www.a.fr"]; } // Tests that long pressing a bookmark selects it and gives access to editing, // as does the Info menu. - (void)testLongPressBookmark { - [[self class] setupStandardBookmarks]; - [[self class] openTopLevelBookmarksFolder]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openTopLevelBookmarksFolder]; // Long press the top-right button. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"First URL Info")] @@ -750,8 +757,8 @@ // Tests the editing of a folder. - (void)testEditFolder { - [[self class] setupStandardBookmarks]; - [[self class] openBookmarkFolder:@"Folder 1"]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openBookmarkFolder:@"Folder 1"]; // Tap the Edit button in the navigation bar. [[EarlGrey @@ -772,8 +779,8 @@ performAction:grey_tap()]; // Check that Folder 1 still exists at this name, and Renamed Folder doesn't. - [[self class] assertFolderExistsWithTitle:@"Folder 1"]; - [[self class] assertFolderDoesntExistWithTitle:@"Renamed Folder"]; + [BookmarksTestCase assertFolderExistsWithTitle:@"Folder 1"]; + [BookmarksTestCase assertFolderDoesntExistWithTitle:@"Renamed Folder"]; // Tap the Edit button in the navigation bar. [[EarlGrey @@ -794,56 +801,56 @@ performAction:grey_tap()]; // Check that Folder 1 doesn't exist and Renamed Folder does. - [[self class] assertFolderDoesntExistWithTitle:@"Folder 1"]; - [[self class] assertFolderExistsWithTitle:@"Renamed Folder"]; + [BookmarksTestCase assertFolderDoesntExistWithTitle:@"Folder 1"]; + [BookmarksTestCase assertFolderExistsWithTitle:@"Renamed Folder"]; } // Tests the deletion of a folder. - (void)testDeleteFolder { - [[self class] setupStandardBookmarks]; - [[self class] openBookmarkFolder:@"Folder 1"]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openBookmarkFolder:@"Folder 1"]; // Delete the folder. - [[self class] deleteSelectedFolder]; + [BookmarksTestCase deleteSelectedFolder]; // Check that the folder doesn't exist anymore. - [[self class] assertFolderDoesntExistWithTitle:@"Folder 1"]; + [BookmarksTestCase assertFolderDoesntExistWithTitle:@"Folder 1"]; } // Navigates to a deeply nested folder, deletes it and makes sure the UI is // consistent. - (void)testDeleteCurrentSubfolder { - [[self class] setupStandardBookmarks]; - [[self class] openBookmarkFolder:@"Folder 1"]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openBookmarkFolder:@"Folder 1"]; [[EarlGrey selectElementWithMatcher:ButtonWithAccessibilityLabel(@"Folder 2")] performAction:grey_tap()]; [[EarlGrey selectElementWithMatcher:ButtonWithAccessibilityLabel(@"Folder 3")] performAction:grey_tap()]; // Delete the folder. - [[self class] deleteSelectedFolder]; + [BookmarksTestCase deleteSelectedFolder]; // Folder 3 is now deleted, UI should have moved to Folder 2, and Folder 2 // should be empty. [[EarlGrey selectElementWithMatcher:grey_accessibilityLabel(@"Folder 2")] assertWithMatcher:grey_sufficientlyVisible()]; - [[self class] assertChildCount:0 ofFolderWithName:@"Folder 2"]; - [[self class] assertFolderDoesntExistWithTitle:@"Folder 3"]; - [[self class] waitForDeletionOfBookmarkWithTitle:@"Folder 3"]; + [BookmarksTestCase assertChildCount:0 ofFolderWithName:@"Folder 2"]; + [BookmarksTestCase assertFolderDoesntExistWithTitle:@"Folder 3"]; + [BookmarksTestCase waitForDeletionOfBookmarkWithTitle:@"Folder 3"]; } // Navigates to a deeply nested folder, delete its parent programatically. // Verifies that the UI is as expected. - (void)testDeleteParentFolder { - [[self class] setupStandardBookmarks]; - [[self class] openBookmarkFolder:@"Folder 1"]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openBookmarkFolder:@"Folder 1"]; [[EarlGrey selectElementWithMatcher:ButtonWithAccessibilityLabel(@"Folder 2")] performAction:grey_tap()]; [[EarlGrey selectElementWithMatcher:ButtonWithAccessibilityLabel(@"Folder 3")] performAction:grey_tap()]; // Remove the parent programmatically. - [[self class] removeBookmarkWithTitle:@"Folder 2"]; + [BookmarksTestCase removeBookmarkWithTitle:@"Folder 2"]; // Folder 2 and 3 are now deleted, UI should have moved to Folder1, and // Folder 1 should be empty. @@ -854,11 +861,11 @@ grey_descendant(grey_text(@"Folder 1")), nil)] assertWithMatcher:grey_sufficientlyVisible()]; - [[self class] assertChildCount:0 ofFolderWithName:@"Folder 1"]; + [BookmarksTestCase assertChildCount:0 ofFolderWithName:@"Folder 1"]; [[EarlGrey selectElementWithMatcher:grey_accessibilityLabel(@"Folder 2")] assertWithMatcher:grey_notVisible()]; - [[self class] assertFolderDoesntExistWithTitle:@"Folder 2"]; - [[self class] assertFolderDoesntExistWithTitle:@"Folder 3"]; + [BookmarksTestCase assertFolderDoesntExistWithTitle:@"Folder 2"]; + [BookmarksTestCase assertFolderDoesntExistWithTitle:@"Folder 3"]; // Check that the selected folder in the menu is Folder 1. if (IsCompact()) { @@ -878,8 +885,8 @@ // Tests that the menu button changes to a back button as expected when browsing // nested folders. - (void)testBrowseNestedFolders { - [[self class] setupStandardBookmarks]; - [[self class] openMobileBookmarks]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; // Navigate down the nested folders. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Folder 1")] @@ -918,8 +925,8 @@ // Tests moving a bookmark into a new folder created in the moving process. - (void)testCreateNewFolderWhileMovingBookmark { - [[self class] setupStandardBookmarks]; - [[self class] openMobileBookmarks]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; // Tap the info disclosure indicator for the bookmark we want to move. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"First URL Info")] @@ -935,7 +942,8 @@ performAction:grey_tap()]; // Enter custom new folder name. - [[self class] renameBookmarkFolderWithFolderTitle:@"Title For New Folder"]; + [BookmarksTestCase + renameBookmarkFolderWithFolderTitle:@"Title For New Folder"]; // Verify current parent folder (Change Folder) is Bookmarks folder. [[EarlGrey @@ -954,7 +962,7 @@ assertWithMatcher:grey_sufficientlyVisible()]; // Verify Folder 2 only has one item. - [[self class] assertChildCount:1 ofFolderWithName:@"Folder 2"]; + [BookmarksTestCase assertChildCount:1 ofFolderWithName:@"Folder 2"]; // Select Folder 2 as new Change Folder. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Folder 2")] @@ -986,23 +994,24 @@ assertWithMatcher:grey_notVisible()]; // Verify new folder has been created under Folder 2. - [[self class] assertChildCount:2 ofFolderWithName:@"Folder 2"]; + [BookmarksTestCase assertChildCount:2 ofFolderWithName:@"Folder 2"]; // Verify new folder has one bookmark. - [[self class] assertChildCount:1 ofFolderWithName:@"Title For New Folder"]; + [BookmarksTestCase assertChildCount:1 + ofFolderWithName:@"Title For New Folder"]; } // Navigates to a deeply nested folder, deletes its root ancestor and checks // that the UI is on the top level folder. - (void)testDeleteRootFolder { - [[self class] setupStandardBookmarks]; - [[self class] openBookmarkFolder:@"Folder 1"]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openBookmarkFolder:@"Folder 1"]; [[EarlGrey selectElementWithMatcher:grey_text(@"Folder 2")] performAction:grey_tap()]; [[EarlGrey selectElementWithMatcher:grey_text(@"Folder 3")] performAction:grey_tap()]; - [[self class] removeBookmarkWithTitle:@"Folder 1"]; + [BookmarksTestCase removeBookmarkWithTitle:@"Folder 1"]; NSString* rootFolderTitle = nil; rootFolderTitle = @"Mobile Bookmarks"; @@ -1049,7 +1058,7 @@ // new bookmark UI as it shows only a snackbar. - (void)testKeyboardCommandsRegistered_AddBookmark { // Add the bookmark. - [[self class] starCurrentTab]; + [BookmarksTestCase starCurrentTab]; GREYAssertTrue(chrome_test_util::GetRegisteredKeyCommandsCount() > 0, @"Some keyboard commands are registered."); } @@ -1057,8 +1066,8 @@ // Tests that keyboard commands are not registered when a bookmark is edited, as // the edit screen is presented modally. - (void)testKeyboardCommandsNotRegistered_EditBookmark { - [[self class] setupStandardBookmarks]; - [[self class] openMobileBookmarks]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; // Go to a bookmarked page. Tap on one of the standard bookmark. [[EarlGrey selectElementWithMatcher:grey_text(@"Second URL")] @@ -1078,16 +1087,16 @@ // Tests that tapping No thanks on the promo make it disappear. - (void)testPromoNoThanksMakeItDisappear { - [[self class] setupStandardBookmarks]; - [[self class] openTopLevelBookmarksFolder]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openTopLevelBookmarksFolder]; // We are going to set the PromoAlreadySeen preference. Set a teardown handler // to reset it. [self setTearDownHandler:^{ - [[self class] setPromoAlreadySeen:NO]; + [BookmarksTestCase setPromoAlreadySeen:NO]; }]; // Check that promo is visible. - [[self class] verifyPromoAlreadySeen:NO]; + [BookmarksTestCase verifyPromoAlreadySeen:NO]; [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"promo_view")] assertWithMatcher:grey_notNil()]; @@ -1101,14 +1110,14 @@ assertWithMatcher:grey_notVisible()]; // Check that the promo already seen state is updated. - [[self class] verifyPromoAlreadySeen:YES]; + [BookmarksTestCase verifyPromoAlreadySeen:YES]; } // Tests that tapping Sign in on the promo make the Sign in sheet appear and // the promo still appears after dismissing the Sign in sheet. - (void)testUIPromoSignIn { - [[self class] setupStandardBookmarks]; - [[self class] openTopLevelBookmarksFolder]; + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openTopLevelBookmarksFolder]; // Set up a fake identity. ChromeIdentity* identity = [FakeChromeIdentity identityWithEmail:@"fakefoo@egmail.com" @@ -1118,7 +1127,7 @@ identity); // Check that promo is visible. - [[self class] verifyPromoAlreadySeen:NO]; + [BookmarksTestCase verifyPromoAlreadySeen:NO]; [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"promo_view")] assertWithMatcher:grey_notNil()]; @@ -1137,12 +1146,12 @@ [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"promo_view")] assertWithMatcher:grey_notNil()]; - [[self class] verifyPromoAlreadySeen:NO]; + [BookmarksTestCase verifyPromoAlreadySeen:NO]; } // Tests that all elements on the bookmarks landing page are accessible. - (void)testAccessibilityOnBookmarksLandingPage { - [[self class] openMobileBookmarksPrepopulatedWithOneBookmark]; + [BookmarksTestCase openMobileBookmarksPrepopulatedWithOneBookmark]; chrome_test_util::VerifyAccessibilityForCurrentScreen(); if (IsCompact()) { @@ -1154,7 +1163,7 @@ // Tests that all elements on the bookmarks Edit page are accessible. - (void)testAccessibilityOnBookmarksEditPage { - [[self class] openMobileBookmarksPrepopulatedWithOneBookmark]; + [BookmarksTestCase openMobileBookmarksPrepopulatedWithOneBookmark]; // Load the menu for a bookmark. [[EarlGrey selectElementWithMatcher:MoreMenuButton()] @@ -1175,7 +1184,7 @@ // Tests that all elements on the bookmarks Move page are accessible. - (void)testAccessibilityOnBookmarksMovePage { - [[self class] openMobileBookmarksPrepopulatedWithOneBookmark]; + [BookmarksTestCase openMobileBookmarksPrepopulatedWithOneBookmark]; // Load the menu for a bookmark. [[EarlGrey selectElementWithMatcher:MoreMenuButton()] @@ -1197,7 +1206,7 @@ // Tests that all elements on the bookmarks Move to New Folder page are // accessible. - (void)testAccessibilityOnBookmarksMoveToNewFolderPage { - [[self class] openMobileBookmarksPrepopulatedWithOneBookmark]; + [BookmarksTestCase openMobileBookmarksPrepopulatedWithOneBookmark]; // Load the menu for a bookmark. [[EarlGrey selectElementWithMatcher:MoreMenuButton()] @@ -1222,7 +1231,7 @@ // Tests that all elements on bookmarks Delete and Undo are accessible. - (void)testAccessibilityOnBookmarksDeleteUndo { - [[self class] openMobileBookmarksPrepopulatedWithOneBookmark]; + [BookmarksTestCase openMobileBookmarksPrepopulatedWithOneBookmark]; // Load the menu for a bookmark. [[EarlGrey selectElementWithMatcher:MoreMenuButton()] @@ -1241,7 +1250,7 @@ // Tests that all elements on the bookmarks Select page are accessible. - (void)testAccessibilityOnBookmarksSelect { - [[self class] openMobileBookmarksPrepopulatedWithOneBookmark]; + [BookmarksTestCase openMobileBookmarksPrepopulatedWithOneBookmark]; // Load the menu for a bookmark. [[EarlGrey selectElementWithMatcher:MoreMenuButton()] @@ -1308,10 +1317,10 @@ "http://ios/testing/data/http_server_files/destination.html"); NSString* bookmarkTitle = @"smokeTapBookmark"; // Load a bookmark into the bookmark model. - [[self class] addBookmark:bookmarkURL withTitle:bookmarkTitle]; + [BookmarksTestCase addBookmark:bookmarkURL withTitle:bookmarkTitle]; // Open the UI for Bookmarks. - [[self class] openMobileBookmarks]; + [BookmarksTestCase openMobileBookmarks]; // Verify bookmark is visible. [[EarlGrey @@ -1356,9 +1365,9 @@ // Tap on the star to bookmark a page, then edit the bookmark to change the // title to |title|. + (void)bookmarkCurrentTabWithTitle:(NSString*)title { - [[self class] waitForBookmarkModelLoaded:YES]; + [BookmarksTestCase waitForBookmarkModelLoaded:YES]; // Add the bookmark from the UI. - [[self class] starCurrentTab]; + [BookmarksTestCase starCurrentTab]; // Set the bookmark name. [[EarlGrey selectElementWithMatcher:EditBookmarkButton()] @@ -1473,7 +1482,7 @@ // Adds a bookmark with the given |url| and |title| into the Mobile Bookmarks // folder. + (void)addBookmark:(const GURL)url withTitle:(NSString*)title { - [[self class] waitForBookmarkModelLoaded:YES]; + [BookmarksTestCase waitForBookmarkModelLoaded:YES]; bookmarks::BookmarkModel* bookmark_model = ios::BookmarkModelFactory::GetForBrowserState( chrome_test_util::GetOriginalBrowserState()); @@ -1517,7 +1526,7 @@ // Loads a set of default bookmarks in the model for the tests to use. + (void)setupStandardBookmarks { - [[self class] waitForBookmarkModelLoaded:YES]; + [BookmarksTestCase waitForBookmarkModelLoaded:YES]; bookmarks::BookmarkModel* bookmark_model = ios::BookmarkModelFactory::GetForBrowserState( @@ -1620,13 +1629,13 @@ // Asserts that there is a bookmark folder with the given title. + (void)assertFolderExistsWithTitle:(NSString*)folderTitle { - GREYAssert([[self class] folderExistsWithTitle:folderTitle], + GREYAssert([BookmarksTestCase folderExistsWithTitle:folderTitle], @"There is no folder named %@", folderTitle); } // Asserts that there is no bookmark folder with the given title. + (void)assertFolderDoesntExistWithTitle:(NSString*)folderTitle { - GREYAssert(![[self class] folderExistsWithTitle:folderTitle], + GREYAssert(![BookmarksTestCase folderExistsWithTitle:folderTitle], @"There is a folder named %@", folderTitle); }
diff --git a/services/ui/ws/BUILD.gn b/services/ui/ws/BUILD.gn index 76368748..56bcc46 100644 --- a/services/ui/ws/BUILD.gn +++ b/services/ui/ws/BUILD.gn
@@ -213,7 +213,6 @@ "event_dispatcher_unittest.cc", "event_matcher_unittest.cc", "focus_controller_unittest.cc", - "frame_generator_unittest.cc", "server_window_compositor_frame_sink_manager_test_api.cc", "server_window_compositor_frame_sink_manager_test_api.h", "server_window_drawn_tracker_unittest.cc",
diff --git a/services/ui/ws/frame_generator.cc b/services/ui/ws/frame_generator.cc index 9dc197d..505ad37 100644 --- a/services/ui/ws/frame_generator.cc +++ b/services/ui/ws/frame_generator.cc
@@ -23,27 +23,10 @@ namespace ws { FrameGenerator::FrameGenerator(FrameGeneratorDelegate* delegate, - ServerWindow* root_window) - : delegate_(delegate), - root_window_(root_window), - binding_(this) { + ServerWindow* root_window, + gfx::AcceleratedWidget widget) + : delegate_(delegate), root_window_(root_window), binding_(this) { DCHECK(delegate_); -} - -void FrameGenerator::SetDeviceScaleFactor(float device_scale_factor) { - if (device_scale_factor_ == device_scale_factor) - return; - device_scale_factor_ = device_scale_factor; - if (compositor_frame_sink_) - compositor_frame_sink_->SetNeedsBeginFrame(true); -} - -FrameGenerator::~FrameGenerator() { - compositor_frame_sink_.reset(); -} - -void FrameGenerator::OnAcceleratedWidgetAvailable( - gfx::AcceleratedWidget widget) { DCHECK_NE(gfx::kNullAcceleratedWidget, widget); cc::mojom::MojoCompositorFrameSinkAssociatedRequest sink_request = mojo::MakeRequest(&compositor_frame_sink_); @@ -54,6 +37,15 @@ std::move(display_request)); } +FrameGenerator::~FrameGenerator() = default; + +void FrameGenerator::SetDeviceScaleFactor(float device_scale_factor) { + if (device_scale_factor_ == device_scale_factor) + return; + device_scale_factor_ = device_scale_factor; + compositor_frame_sink_->SetNeedsBeginFrame(true); +} + void FrameGenerator::OnSurfaceCreated(const cc::SurfaceInfo& surface_info) { DCHECK(surface_info.id().is_valid()); @@ -79,24 +71,21 @@ // TODO(fsamuel): We should add a trace for generating a top level frame. cc::CompositorFrame frame(GenerateCompositorFrame(root_window_->bounds())); - if (compositor_frame_sink_) { - gfx::Size frame_size = last_submitted_frame_size_; - if (!frame.render_pass_list.empty()) - frame_size = frame.render_pass_list[0]->output_rect.size(); + gfx::Size frame_size = last_submitted_frame_size_; + if (!frame.render_pass_list.empty()) + frame_size = frame.render_pass_list.back()->output_rect.size(); - if (!local_surface_id_.is_valid() || - frame_size != last_submitted_frame_size_) { - local_surface_id_ = id_allocator_.GenerateId(); - display_private_->ResizeDisplay(frame_size); - } - - display_private_->SetLocalSurfaceId(local_surface_id_, - device_scale_factor_); - compositor_frame_sink_->SubmitCompositorFrame(local_surface_id_, - std::move(frame)); - compositor_frame_sink_->SetNeedsBeginFrame(false); - last_submitted_frame_size_ = frame_size; + if (!local_surface_id_.is_valid() || + frame_size != last_submitted_frame_size_) { + local_surface_id_ = id_allocator_.GenerateId(); + display_private_->ResizeDisplay(frame_size); } + + display_private_->SetLocalSurfaceId(local_surface_id_, device_scale_factor_); + compositor_frame_sink_->SubmitCompositorFrame(local_surface_id_, + std::move(frame)); + compositor_frame_sink_->SetNeedsBeginFrame(false); + last_submitted_frame_size_ = frame_size; } void FrameGenerator::ReclaimResources(
diff --git a/services/ui/ws/frame_generator.h b/services/ui/ws/frame_generator.h index bfad392a..e8ac31c 100644 --- a/services/ui/ws/frame_generator.h +++ b/services/ui/ws/frame_generator.h
@@ -39,14 +39,13 @@ // submitting CompositorFrames to the owned CompositorFrameSink. class FrameGenerator : public cc::mojom::MojoCompositorFrameSinkClient { public: - FrameGenerator(FrameGeneratorDelegate* delegate, ServerWindow* root_window); + FrameGenerator(FrameGeneratorDelegate* delegate, + ServerWindow* root_window, + gfx::AcceleratedWidget widget); ~FrameGenerator() override; void SetDeviceScaleFactor(float device_scale_factor); - // Schedules a redraw for the provided region. - void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget); - // Updates the WindowManager's SurfaceInfo. void OnSurfaceCreated(const cc::SurfaceInfo& surface_info);
diff --git a/services/ui/ws/frame_generator_unittest.cc b/services/ui/ws/frame_generator_unittest.cc deleted file mode 100644 index 114924c..0000000 --- a/services/ui/ws/frame_generator_unittest.cc +++ /dev/null
@@ -1,119 +0,0 @@ -// Copyright 2016 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 "services/ui/ws/frame_generator.h" - -#include <memory> - -#include "base/memory/ptr_util.h" -#include "base/test/test_message_loop.h" -#include "cc/quads/render_pass.h" -#include "cc/quads/shared_quad_state.h" -#include "services/ui/ws/ids.h" -#include "services/ui/ws/platform_display_init_params.h" -#include "services/ui/ws/server_window.h" -#include "services/ui/ws/server_window_compositor_frame_sink_manager.h" -#include "services/ui/ws/test_server_window_delegate.h" -#include "services/ui/ws/test_utils.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace ui { -namespace ws { -namespace test { -namespace { - -// Typical id for the display root ServerWindow. -constexpr WindowId kRootDisplayId(0, 2); -const base::UnguessableToken kArbitraryToken = base::UnguessableToken::Create(); - -// Makes the window visible and creates the default surface for it. -void InitWindow(ServerWindow* window) { - window->SetVisible(true); - ServerWindowCompositorFrameSinkManager* compositor_frame_sink_manager = - window->GetOrCreateCompositorFrameSinkManager(); - compositor_frame_sink_manager->SetLatestSurfaceInfo(cc::SurfaceInfo( - cc::SurfaceId(cc::FrameSinkId(WindowIdToTransportId(window->id()), 0), - cc::LocalSurfaceId(1u, kArbitraryToken)), - 1.0f, gfx::Size(100, 100))); -} - -} // namespace - -class FrameGeneratorTest : public testing::Test { - public: - FrameGeneratorTest() - : root_window_(base::MakeUnique<ServerWindow>(&window_delegate_, - kRootDisplayId)) {} - ~FrameGeneratorTest() override {} - - // Calls DrawWindow() on |frame_generator_| - void DrawWindow(cc::RenderPass* pass); - - ServerWindow* root_window() { return root_window_.get(); } - - TestServerWindowDelegate* test_window_delegate() { return &window_delegate_; } - - private: - // testing::Test: - void SetUp() override; - void TearDown() override; - - std::unique_ptr<FrameGenerator> frame_generator_; - std::unique_ptr<TestFrameGeneratorDelegate> frame_generator_delegate_; - TestServerWindowDelegate window_delegate_; - std::unique_ptr<ServerWindow> root_window_; - - // Needed so that Mojo classes can be initialized for ServerWindow use. - base::TestMessageLoop message_loop_; - - DISALLOW_COPY_AND_ASSIGN(FrameGeneratorTest); -}; - -void FrameGeneratorTest::DrawWindow(cc::RenderPass* pass) { - cc::SurfaceId surface_id(cc::FrameSinkId(5, 5), - cc::LocalSurfaceId(1u, kArbitraryToken)); - frame_generator_->window_manager_surface_info_ = - cc::SurfaceInfo(surface_id, 2, gfx::Size(2, 2)); - frame_generator_->DrawWindow(pass); -} - -void FrameGeneratorTest::SetUp() { - testing::Test::SetUp(); - frame_generator_delegate_ = base::MakeUnique<TestFrameGeneratorDelegate>(); - PlatformDisplayInitParams init_params; - frame_generator_ = base::MakeUnique<FrameGenerator>( - frame_generator_delegate_.get(), root_window_.get()); - frame_generator_->SetDeviceScaleFactor( - init_params.metrics.device_scale_factor); - InitWindow(root_window()); -} - -void FrameGeneratorTest::TearDown() { - frame_generator_.reset(); - frame_generator_delegate_.reset(); -} - -// Tests correctness of the SharedQuadStateList generated by -// FrameGenerator::DrawWindow(). -TEST_F(FrameGeneratorTest, DrawWindow) { - ServerWindow child_window(test_window_delegate(), WindowId(1, 1)); - root_window()->Add(&child_window); - InitWindow(&child_window); - const float child_opacity = .4f; - child_window.SetOpacity(child_opacity); - - std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create(); - DrawWindow(render_pass.get()); - cc::SharedQuadStateList* quad_state_list = - &render_pass->shared_quad_state_list; - - EXPECT_EQ(1u, quad_state_list->size()); - cc::SharedQuadState* root_sqs = quad_state_list->back(); - // Opacity should always be 1. - EXPECT_EQ(1.0f, root_sqs->opacity); -} - -} // namespace test -} // namespace ws -} // namespace ui
diff --git a/services/ui/ws/platform_display_default.cc b/services/ui/ws/platform_display_default.cc index 6f8c8d6..6d32827f0 100644 --- a/services/ui/ws/platform_display_default.cc +++ b/services/ui/ws/platform_display_default.cc
@@ -35,11 +35,10 @@ #if !defined(OS_ANDROID) image_cursors_(new ImageCursors), #endif - frame_generator_(new FrameGenerator(this, init_params.root_window)), metrics_(init_params.metrics), - widget_(gfx::kNullAcceleratedWidget) { - frame_generator_->SetDeviceScaleFactor( - init_params.metrics.device_scale_factor); + widget_(gfx::kNullAcceleratedWidget), + root_window_(init_params.root_window), + init_device_scale_factor_(init_params.metrics.device_scale_factor) { } PlatformDisplayDefault::~PlatformDisplayDefault() { @@ -148,7 +147,8 @@ } metrics_ = metrics; - frame_generator_->SetDeviceScaleFactor(metrics_.device_scale_factor); + if (frame_generator_) + frame_generator_->SetDeviceScaleFactor(metrics_.device_scale_factor); return true; } @@ -178,7 +178,8 @@ } void PlatformDisplayDefault::OnDamageRect(const gfx::Rect& damaged_region) { - frame_generator_->OnWindowDamaged(); + if (frame_generator_) + frame_generator_->OnWindowDamaged(); } void PlatformDisplayDefault::DispatchEvent(ui::Event* event) { @@ -247,7 +248,9 @@ DCHECK_EQ(gfx::kNullAcceleratedWidget, widget_); widget_ = widget; delegate_->OnAcceleratedWidgetAvailable(); - frame_generator_->OnAcceleratedWidgetAvailable(widget); + frame_generator_ = + base::MakeUnique<FrameGenerator>(this, root_window_, widget_); + frame_generator_->SetDeviceScaleFactor(init_device_scale_factor_); } void PlatformDisplayDefault::OnAcceleratedWidgetDestroyed() {
diff --git a/services/ui/ws/platform_display_default.h b/services/ui/ws/platform_display_default.h index ca1b7d5..54d36d8 100644 --- a/services/ui/ws/platform_display_default.h +++ b/services/ui/ws/platform_display_default.h
@@ -26,7 +26,7 @@ // FrameGenerator for Chrome OS. class PlatformDisplayDefault : public PlatformDisplay, public ui::PlatformWindowDelegate, - private FrameGeneratorDelegate { + public FrameGeneratorDelegate { public: explicit PlatformDisplayDefault(const PlatformDisplayInitParams& init_params); ~PlatformDisplayDefault() override; @@ -83,6 +83,8 @@ display::ViewportMetrics metrics_; std::unique_ptr<ui::PlatformWindow> platform_window_; gfx::AcceleratedWidget widget_; + ServerWindow* root_window_; + float init_device_scale_factor_; DISALLOW_COPY_AND_ASSIGN(PlatformDisplayDefault); };
diff --git a/third_party/WebKit/LayoutTests/fast/css/sticky/sticky-style-change-expected.html b/third_party/WebKit/LayoutTests/fast/css/sticky/sticky-style-change-expected.html new file mode 100644 index 0000000..6bd42d28 --- /dev/null +++ b/third_party/WebKit/LayoutTests/fast/css/sticky/sticky-style-change-expected.html
@@ -0,0 +1,21 @@ +<!DOCTYPE html> +<html> +<head> +<style> + body { + margin: 0; + } + + .absolute-box { + position: absolute; + top: 50px; + height: 50px; + width: 100%; + background-color: green; + } +</style> +</head> +<body> + <div class="absolute-box"></div> +</body> +</html>
diff --git a/third_party/WebKit/LayoutTests/fast/css/sticky/sticky-style-change.html b/third_party/WebKit/LayoutTests/fast/css/sticky/sticky-style-change.html new file mode 100644 index 0000000..0524b75 --- /dev/null +++ b/third_party/WebKit/LayoutTests/fast/css/sticky/sticky-style-change.html
@@ -0,0 +1,69 @@ +<!DOCTYPE html> +<script> +if (window.internals) { + internals.settings.setCSSStickyPositionEnabled(true); +} +</script> + +<html> +<head> +<style> + html { + overflow: hidden; /* hide scrollbars */ + } + + body { + margin: 0; + } + + .container { + height: 2000px; + } + + .box { + height: 50px; + } + + .sticky { + position: sticky; + top: 50px; + background-color: green; + } +</style> +<script> + if (window.testRunner) + testRunner.waitUntilDone() + + function doTest() + { + const sticky = document.querySelector('.sticky'); + sticky.style.position = 'relative'; + + // Force layout. + sticky.offsetTop; + + sticky.style.position = 'sticky'; + + window.requestAnimationFrame(function() { + window.scrollTo(0, 100); + if (window.testRunner) + testRunner.notifyDone(); + }); + } + + window.addEventListener('load', function() { + // We require the compositings inputs to be clean (and thus the initial + // sticky position to be calculated) before we stat changing things. + // Force this by waiting for a double rAF. + window.requestAnimationFrame(function() { + window.requestAnimationFrame(doTest); + }); + }); +</script> +</head> +<body> + <div class="container"> + <div class="sticky box"></div> + </div> +</body> +</html>
diff --git a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLength-expected.txt b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLength-expected.txt deleted file mode 100644 index fa21a69..0000000 --- a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLength-expected.txt +++ /dev/null
@@ -1,27 +0,0 @@ -This test checks the SVGAnimatedLength API - utilizing the width property of SVGRectElement - -On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". - - - -Check initial width value -PASS rectElement.width.toString() is "[object SVGAnimatedLength]" -PASS rectElement.width.baseVal.toString() is "[object SVGLength]" -PASS rectElement.width.baseVal.value is 0 - -Check that lengths are dynamic, caching value in a local variable and modifying it, should take effect -PASS numRef.value is 100 -PASS rectElement.width.baseVal.value is 100 - -Check that assigning to baseVal has no effect, as no setter is defined -PASS rectElement.width.baseVal = -1 is -1 -PASS rectElement.width.baseVal = 'aString' is "aString" -PASS rectElement.width.baseVal = rectElement is rectElement - -Check that the width value remained 100, and the baseVal type has not been changed -PASS rectElement.width.baseVal.toString() is "[object SVGLength]" -PASS rectElement.width.baseVal.value is 100 -PASS successfullyParsed is true - -TEST COMPLETE -
diff --git a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLength.html b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLength.html index 54ae2e9..f5388a8 100644 --- a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLength.html +++ b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLength.html
@@ -1,11 +1,33 @@ -<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> -<html> -<head> -<script src="../../resources/js-test.js"></script> -</head> -<body> -<p id="description"></p> -<div id="console"></div> -<script src="script-tests/SVGAnimatedLength.js"></script> -</body> -</html> +<!DOCTYPE HTML> +<title>SVGAnimatedLength interface - utilizing the width property of SVGRectElement</title> +<script src="../../resources/testharness.js"></script> +<script src="../../resources/testharnessreport.js"></script> +<script> +test(function() { + // This test checks the SVGAnimatedLength API - utilizing the width property of SVGRectElement. + + var rectElement = document.createElementNS("http://www.w3.org/2000/svg", "rect"); + + // Check initial width value. + assert_true(rectElement.width instanceof SVGAnimatedLength); + assert_true(rectElement.width.baseVal instanceof SVGLength); + assert_equals(rectElement.width.baseVal.value, 0); + + // Check that lengths are dynamic, caching value in a local variable and modifying it, should take effect. + var numRef = rectElement.width.baseVal; + numRef.value = 100; + assert_equals(numRef.value, 100); + assert_equals(rectElement.width.baseVal.value, 100); + + // Check that assigning to baseVal has no effect, as no setter is defined. + rectElement.width.baseVal = -1; + assert_equals(rectElement.width.baseVal.value, 100); + rectElement.width.baseVal = 'aString'; + assert_equals(rectElement.width.baseVal.value, 100); + rectElement.width.baseVal = rectElement; + assert_equals(rectElement.width.baseVal.value, 100); + + // Check that the width baseVal type has not been changed. + assert_true(rectElement.width.baseVal instanceof SVGLength); +}); +</script> \ No newline at end of file
diff --git a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLengthList-expected.txt b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLengthList-expected.txt deleted file mode 100644 index ab46020..0000000 --- a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLengthList-expected.txt +++ /dev/null
@@ -1,27 +0,0 @@ -This test checks the SVGAnimatedLengthList API - utilizing the dx property of SVGTextElement - -On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". - - - -Check initial dx value -PASS textElement.dx.toString() is "[object SVGAnimatedLengthList]" -PASS textElement.dx.baseVal.toString() is "[object SVGLengthList]" -PASS textElement.dx.baseVal.getItem(0).value is 50 - -Check that length lists are dynamic, caching value in a local variable and modifying it, should take effect -PASS numRef.getItem(0).value is 100 -PASS textElement.dx.baseVal.getItem(0).value is 100 - -Check that assigning to baseVal has no effect, as no setter is defined -PASS textElement.dx.baseVal = -1 is -1 -PASS textElement.dx.baseVal = 'aString' is "aString" -PASS textElement.dx.baseVal = textElement is textElement - -Check that the dx value remained 100, and the baseVal type has not been changed -PASS textElement.dx.baseVal.toString() is "[object SVGLengthList]" -PASS textElement.dx.baseVal.getItem(0).value is 100 -PASS successfullyParsed is true - -TEST COMPLETE -
diff --git a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLengthList.html b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLengthList.html index 82dce0ced..15251f4e 100644 --- a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLengthList.html +++ b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedLengthList.html
@@ -1,11 +1,34 @@ -<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> -<html> -<head> -<script src="../../resources/js-test.js"></script> -</head> -<body> -<p id="description"></p> -<div id="console"></div> -<script src="script-tests/SVGAnimatedLengthList.js"></script> -</body> -</html> +<!DOCTYPE HTML> +<title>SVGAnimatedLengthList interface - utilizing the dx property of SVGTextElement</title> +<script src="../../resources/testharness.js"></script> +<script src="../../resources/testharnessreport.js"></script> +<script> +test(function() { + // This test checks the SVGAnimatedLengthList API - utilizing the dx property of SVGTextElement. + + var textElement = document.createElementNS("http://www.w3.org/2000/svg", "text"); + textElement.setAttribute("dx", "50"); + + // Check initial dx value. + assert_true(textElement.dx instanceof SVGAnimatedLengthList); + assert_true(textElement.dx.baseVal instanceof SVGLengthList); + assert_equals(textElement.dx.baseVal.getItem(0).value, 50); + + // Check that length lists are dynamic, caching value in a local variable and modifying it, should take effect. + var numRef = textElement.dx.baseVal; + numRef.getItem(0).value = 100; + assert_equals(numRef.getItem(0).value, 100); + assert_equals(textElement.dx.baseVal.getItem(0).value, 100); + + // Check that assigning to baseVal has no effect, as no setter is defined. + textElement.dx.baseVal = -1; + assert_equals(textElement.dx.baseVal.getItem(0).value, 100); + textElement.dx.baseVal = 'aString'; + assert_equals(textElement.dx.baseVal.getItem(0).value, 100); + textElement.dx.baseVal = textElement; + assert_equals(textElement.dx.baseVal.getItem(0).value, 100); + + // Check that the dx baseVal type has not been changed. + assert_true(textElement.dx.baseVal instanceof SVGLengthList); +}); +</script>
diff --git a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumber-expected.txt b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumber-expected.txt deleted file mode 100644 index 8add6f1e..0000000 --- a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumber-expected.txt +++ /dev/null
@@ -1,35 +0,0 @@ -This test checks the SVGAnimatedNumber API - utilizing the surfaceScale property of SVGFESpecularLightingElement - -On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". - - - -Check initial surfaceScale value -PASS feSpecularLightingElement.surfaceScale.toString() is "[object SVGAnimatedNumber]" -PASS typeof(feSpecularLightingElement.surfaceScale.baseVal) is "number" -PASS feSpecularLightingElement.surfaceScale.baseVal is 1 - -Check that integers are static, caching value in a local variable and modifying it, should have no effect -PASS numRef is 100 -PASS feSpecularLightingElement.surfaceScale.baseVal is 1 - -Check assigning various valid and invalid values -PASS feSpecularLightingElement.surfaceScale.baseVal = -1 is -1 -PASS feSpecularLightingElement.surfaceScale.baseVal = 300 is 300 -PASS feSpecularLightingElement.surfaceScale.baseVal = 'aString' threw exception TypeError: Failed to set the 'baseVal' property on 'SVGAnimatedNumber': The provided float value is non-finite.. -PASS feSpecularLightingElement.surfaceScale.baseVal is 300 -PASS feSpecularLightingElement.surfaceScale.baseVal = 0 is 0 -PASS feSpecularLightingElement.surfaceScale.baseVal = NaN threw exception TypeError: Failed to set the 'baseVal' property on 'SVGAnimatedNumber': The provided float value is non-finite.. -PASS feSpecularLightingElement.surfaceScale.baseVal is 0 -PASS feSpecularLightingElement.surfaceScale.baseVal = Infinity threw exception TypeError: Failed to set the 'baseVal' property on 'SVGAnimatedNumber': The provided float value is non-finite.. -PASS feSpecularLightingElement.surfaceScale.baseVal is 0 -PASS feSpecularLightingElement.surfaceScale.baseVal = feSpecularLightingElement threw exception TypeError: Failed to set the 'baseVal' property on 'SVGAnimatedNumber': The provided float value is non-finite.. -PASS feSpecularLightingElement.surfaceScale.baseVal is 0 -PASS feSpecularLightingElement.surfaceScale.baseVal = 300 is 300 - -Check that the surfaceScale value remained 300 -PASS feSpecularLightingElement.surfaceScale.baseVal is 300 -PASS successfullyParsed is true - -TEST COMPLETE -
diff --git a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumber.html b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumber.html index 346ecd6..10330a33 100644 --- a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumber.html +++ b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumber.html
@@ -1,11 +1,42 @@ -<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> -<html> -<head> -<script src="../../resources/js-test.js"></script> -</head> -<body> -<p id="description"></p> -<div id="console"></div> -<script src="script-tests/SVGAnimatedNumber.js"></script> -</body> -</html> +<!DOCTYPE HTML> +<title>SVGAnimatedNumber interface - utilizing the surfaceScale property of SVGFESpecularLightingElement</title> +<script src="../../resources/testharness.js"></script> +<script src="../../resources/testharnessreport.js"></script> +<script> +test(function() { + // This test checks the SVGAnimatedNumber API - utilizing the surfaceScale property of SVGFESpecularLightingElement. + + var feSpecularLightingElement = document.createElementNS("http://www.w3.org/2000/svg", "feSpecularLighting"); + + // Check initial surfaceScale value. + assert_true(feSpecularLightingElement.surfaceScale instanceof SVGAnimatedNumber); + assert_equals(typeof(feSpecularLightingElement.surfaceScale.baseVal), "number"); + assert_equals(feSpecularLightingElement.surfaceScale.baseVal, 1); + + // Check that integers are static, caching value in a local variable and modifying it, should have no effect. + var numRef = feSpecularLightingElement.surfaceScale.baseVal; + numRef = 100; + assert_equals(numRef, 100); + assert_equals(feSpecularLightingElement.surfaceScale.baseVal, 1); + + // Check assigning various valid and invalid values. + feSpecularLightingElement.surfaceScale.baseVal = -1; // Negative values are allowed from SVG DOM, but should lead to an error when rendering (disable the filter) + assert_equals(feSpecularLightingElement.surfaceScale.baseVal, -1); + feSpecularLightingElement.surfaceScale.baseVal = 300; + assert_equals(feSpecularLightingElement.surfaceScale.baseVal, 300); + + // ECMA-262, 9.3, "ToNumber" + assert_throws(new TypeError(), function() { feSpecularLightingElement.surfaceScale.baseVal = 'aString'; }); + assert_equals(feSpecularLightingElement.surfaceScale.baseVal, 300); + feSpecularLightingElement.surfaceScale.baseVal = 0; + assert_equals(feSpecularLightingElement.surfaceScale.baseVal, 0); + assert_throws(new TypeError(), function() { feSpecularLightingElement.surfaceScale.baseVal = NaN; }); + assert_equals(feSpecularLightingElement.surfaceScale.baseVal, 0); + assert_throws(new TypeError(), function() { feSpecularLightingElement.surfaceScale.baseVal = Infinity; }); + assert_equals(feSpecularLightingElement.surfaceScale.baseVal, 0); + assert_throws(new TypeError(), function() { feSpecularLightingElement.surfaceScale.baseVal = feSpecularLightingElement; }); + assert_equals(feSpecularLightingElement.surfaceScale.baseVal, 0); + feSpecularLightingElement.surfaceScale.baseVal = 300; + assert_equals(feSpecularLightingElement.surfaceScale.baseVal, 300); +}); +</script>
diff --git a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumberList-expected.txt b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumberList-expected.txt deleted file mode 100644 index bff3bd97f..0000000 --- a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumberList-expected.txt +++ /dev/null
@@ -1,27 +0,0 @@ -This test checks the SVGAnimatedNumberList API - utilizing the rotate property of SVGTextElement - -On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". - - - -Check initial rotate value -PASS textElement.rotate.toString() is "[object SVGAnimatedNumberList]" -PASS textElement.rotate.baseVal.toString() is "[object SVGNumberList]" -PASS textElement.rotate.baseVal.getItem(0).value is 50 - -Check that number lists are dynamic, caching value in a local variable and modifying it, should take effect -PASS numRef.getItem(0).value is 100 -PASS textElement.rotate.baseVal.getItem(0).value is 100 - -Check that assigning to baseVal has no effect, as no setter is defined -PASS textElement.rotate.baseVal = -1 is -1 -PASS textElement.rotate.baseVal = 'aString' is "aString" -PASS textElement.rotate.baseVal = textElement is textElement - -Check that the rotate value remained 100, and the baseVal type has not been changed -PASS textElement.rotate.baseVal.toString() is "[object SVGNumberList]" -PASS textElement.rotate.baseVal.getItem(0).value is 100 -PASS successfullyParsed is true - -TEST COMPLETE -
diff --git a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumberList.html b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumberList.html index dd42e3c..3aa30e4 100644 --- a/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumberList.html +++ b/third_party/WebKit/LayoutTests/svg/dom/SVGAnimatedNumberList.html
@@ -1,11 +1,34 @@ -<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> -<html> -<head> -<script src="../../resources/js-test.js"></script> -</head> -<body> -<p id="description"></p> -<div id="console"></div> -<script src="script-tests/SVGAnimatedNumberList.js"></script> -</body> -</html> +<!DOCTYPE HTML> +<title>SVGAnimatedNumberList interface - utilizing the rotate property of SVGTextElement</title> +<script src="../../resources/testharness.js"></script> +<script src="../../resources/testharnessreport.js"></script> +<script> +test(function() { + // This test checks the SVGAnimatedNumberList API - utilizing the rotate property of SVGTextElement. + + var textElement = document.createElementNS("http://www.w3.org/2000/svg", "text"); + textElement.setAttribute("rotate", "50"); + + // Check initial rotate value. + assert_true(textElement.rotate instanceof SVGAnimatedNumberList); + assert_true(textElement.rotate.baseVal instanceof SVGNumberList); + assert_equals(textElement.rotate.baseVal.getItem(0).value, 50); + + // Check that number lists are dynamic, caching value in a local variable and modifying it, should take effect. + var numRef = textElement.rotate.baseVal; + numRef.getItem(0).value = 100; + assert_equals(numRef.getItem(0).value, 100); + assert_equals(textElement.rotate.baseVal.getItem(0).value, 100); + + // Check that assigning to baseVal has no effect, as no setter is defined. + textElement.rotate.baseVal = -1; + assert_equals(textElement.rotate.baseVal.getItem(0).value, 100); + textElement.rotate.baseVal = 'aString'; + assert_equals(textElement.rotate.baseVal.getItem(0).value, 100); + textElement.rotate.baseVal = textElement; + assert_equals(textElement.rotate.baseVal.getItem(0).value, 100); + + // Check that the rotate baseVal type has not been changed. + assert_true(textElement.rotate.baseVal instanceof SVGNumberList); +}); +</script> \ No newline at end of file
diff --git a/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedLength.js b/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedLength.js deleted file mode 100644 index 7f499a2..0000000 --- a/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedLength.js +++ /dev/null
@@ -1,29 +0,0 @@ -description("This test checks the SVGAnimatedLength API - utilizing the width property of SVGRectElement"); - -var rectElement = document.createElementNS("http://www.w3.org/2000/svg", "rect"); - -debug(""); -debug("Check initial width value"); -shouldBeEqualToString("rectElement.width.toString()", "[object SVGAnimatedLength]"); -shouldBeEqualToString("rectElement.width.baseVal.toString()", "[object SVGLength]"); -shouldBe("rectElement.width.baseVal.value", "0"); - -debug(""); -debug("Check that lengths are dynamic, caching value in a local variable and modifying it, should take effect"); -var numRef = rectElement.width.baseVal; -numRef.value = 100; -shouldBe("numRef.value", "100"); -shouldBe("rectElement.width.baseVal.value", "100"); - -debug(""); -debug("Check that assigning to baseVal has no effect, as no setter is defined"); -shouldBe("rectElement.width.baseVal = -1", "-1"); -shouldBeEqualToString("rectElement.width.baseVal = 'aString'", "aString"); -shouldBe("rectElement.width.baseVal = rectElement", "rectElement"); - -debug(""); -debug("Check that the width value remained 100, and the baseVal type has not been changed"); -shouldBeEqualToString("rectElement.width.baseVal.toString()", "[object SVGLength]"); -shouldBe("rectElement.width.baseVal.value", "100"); - -successfullyParsed = true;
diff --git a/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedLengthList.js b/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedLengthList.js deleted file mode 100644 index 1cc76685..0000000 --- a/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedLengthList.js +++ /dev/null
@@ -1,30 +0,0 @@ -description("This test checks the SVGAnimatedLengthList API - utilizing the dx property of SVGTextElement"); - -var textElement = document.createElementNS("http://www.w3.org/2000/svg", "text"); -textElement.setAttribute("dx", "50"); - -debug(""); -debug("Check initial dx value"); -shouldBeEqualToString("textElement.dx.toString()", "[object SVGAnimatedLengthList]"); -shouldBeEqualToString("textElement.dx.baseVal.toString()", "[object SVGLengthList]"); -shouldBe("textElement.dx.baseVal.getItem(0).value", "50"); - -debug(""); -debug("Check that length lists are dynamic, caching value in a local variable and modifying it, should take effect"); -var numRef = textElement.dx.baseVal; -numRef.getItem(0).value = 100; -shouldBe("numRef.getItem(0).value", "100"); -shouldBe("textElement.dx.baseVal.getItem(0).value", "100"); - -debug(""); -debug("Check that assigning to baseVal has no effect, as no setter is defined"); -shouldBe("textElement.dx.baseVal = -1", "-1"); -shouldBeEqualToString("textElement.dx.baseVal = 'aString'", "aString"); -shouldBe("textElement.dx.baseVal = textElement", "textElement"); - -debug(""); -debug("Check that the dx value remained 100, and the baseVal type has not been changed"); -shouldBeEqualToString("textElement.dx.baseVal.toString()", "[object SVGLengthList]"); -shouldBe("textElement.dx.baseVal.getItem(0).value", "100"); - -successfullyParsed = true;
diff --git a/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedNumber.js b/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedNumber.js deleted file mode 100644 index df19566..0000000 --- a/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedNumber.js +++ /dev/null
@@ -1,38 +0,0 @@ -description("This test checks the SVGAnimatedNumber API - utilizing the surfaceScale property of SVGFESpecularLightingElement"); - -var feSpecularLightingElement = document.createElementNS("http://www.w3.org/2000/svg", "feSpecularLighting"); - -debug(""); -debug("Check initial surfaceScale value"); -shouldBeEqualToString("feSpecularLightingElement.surfaceScale.toString()", "[object SVGAnimatedNumber]"); -shouldBeEqualToString("typeof(feSpecularLightingElement.surfaceScale.baseVal)", "number"); -shouldBe("feSpecularLightingElement.surfaceScale.baseVal", "1"); - -debug(""); -debug("Check that integers are static, caching value in a local variable and modifying it, should have no effect"); -var numRef = feSpecularLightingElement.surfaceScale.baseVal; -numRef = 100; -shouldBe("numRef", "100"); -shouldBe("feSpecularLightingElement.surfaceScale.baseVal", "1"); - -debug(""); -debug("Check assigning various valid and invalid values"); -shouldBe("feSpecularLightingElement.surfaceScale.baseVal = -1", "-1"); // Negative values are allowed from SVG DOM, but should lead to an error when rendering (disable the filter) -shouldBe("feSpecularLightingElement.surfaceScale.baseVal = 300", "300"); -// ECMA-262, 9.3, "ToNumber" -shouldThrow("feSpecularLightingElement.surfaceScale.baseVal = 'aString'"); -shouldBe("feSpecularLightingElement.surfaceScale.baseVal", "300"); -shouldBe("feSpecularLightingElement.surfaceScale.baseVal = 0", "0"); -shouldThrow("feSpecularLightingElement.surfaceScale.baseVal = NaN"); -shouldBe("feSpecularLightingElement.surfaceScale.baseVal", "0"); -shouldThrow("feSpecularLightingElement.surfaceScale.baseVal = Infinity"); -shouldBe("feSpecularLightingElement.surfaceScale.baseVal", "0"); -shouldThrow("feSpecularLightingElement.surfaceScale.baseVal = feSpecularLightingElement"); -shouldBe("feSpecularLightingElement.surfaceScale.baseVal", "0"); -shouldBe("feSpecularLightingElement.surfaceScale.baseVal = 300", "300"); - -debug(""); -debug("Check that the surfaceScale value remained 300"); -shouldBe("feSpecularLightingElement.surfaceScale.baseVal", "300"); - -successfullyParsed = true;
diff --git a/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedNumberList.js b/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedNumberList.js deleted file mode 100644 index c4516b4..0000000 --- a/third_party/WebKit/LayoutTests/svg/dom/script-tests/SVGAnimatedNumberList.js +++ /dev/null
@@ -1,30 +0,0 @@ -description("This test checks the SVGAnimatedNumberList API - utilizing the rotate property of SVGTextElement"); - -var textElement = document.createElementNS("http://www.w3.org/2000/svg", "text"); -textElement.setAttribute("rotate", "50"); - -debug(""); -debug("Check initial rotate value"); -shouldBeEqualToString("textElement.rotate.toString()", "[object SVGAnimatedNumberList]"); -shouldBeEqualToString("textElement.rotate.baseVal.toString()", "[object SVGNumberList]"); -shouldBe("textElement.rotate.baseVal.getItem(0).value", "50"); - -debug(""); -debug("Check that number lists are dynamic, caching value in a local variable and modifying it, should take effect"); -var numRef = textElement.rotate.baseVal; -numRef.getItem(0).value = 100; -shouldBe("numRef.getItem(0).value", "100"); -shouldBe("textElement.rotate.baseVal.getItem(0).value", "100"); - -debug(""); -debug("Check that assigning to baseVal has no effect, as no setter is defined"); -shouldBe("textElement.rotate.baseVal = -1", "-1"); -shouldBeEqualToString("textElement.rotate.baseVal = 'aString'", "aString"); -shouldBe("textElement.rotate.baseVal = textElement", "textElement"); - -debug(""); -debug("Check that the rotate value remained 100, and the baseVal type has not been changed"); -shouldBeEqualToString("textElement.rotate.baseVal.toString()", "[object SVGNumberList]"); -shouldBe("textElement.rotate.baseVal.getItem(0).value", "100"); - -successfullyParsed = true;
diff --git a/third_party/WebKit/Source/core/frame/FrameView.cpp b/third_party/WebKit/Source/core/frame/FrameView.cpp index 552536d..333d17d 100644 --- a/third_party/WebKit/Source/core/frame/FrameView.cpp +++ b/third_party/WebKit/Source/core/frame/FrameView.cpp
@@ -4748,7 +4748,11 @@ // Don't throttle display:none frames (see updateRenderThrottlingStatus). HTMLFrameOwnerElement* ownerElement = m_frame->deprecatedLocalOwner(); if (m_hiddenForThrottling && ownerElement && !ownerElement->layoutObject()) { - updateRenderThrottlingStatus(m_hiddenForThrottling, m_subtreeThrottled); + // No need to notify children because descendants of display:none frames + // should remain throttled. + updateRenderThrottlingStatus(m_hiddenForThrottling, m_subtreeThrottled, + DontForceThrottlingInvalidation, + DontNotifyChildren); DCHECK(!canThrottleRendering()); } @@ -4769,12 +4773,15 @@ // Cross-domain status is not stored as a dirty bit within FrameView, // so force-invalidate throttling status when it changes regardless of // previous or new value. - updateRenderThrottlingStatus(m_hiddenForThrottling, m_subtreeThrottled, true); + updateRenderThrottlingStatus(m_hiddenForThrottling, m_subtreeThrottled, + ForceThrottlingInvalidation); } -void FrameView::updateRenderThrottlingStatus(bool hidden, - bool subtreeThrottled, - bool forceThrottlingInvalidation) { +void FrameView::updateRenderThrottlingStatus( + bool hidden, + bool subtreeThrottled, + ForceThrottlingInvalidationBehavior forceThrottlingInvalidationBehavior, + NotifyChildrenBehavior notifyChildrenBehavior) { TRACE_EVENT0("blink", "FrameView::updateRenderThrottlingStatus"); DCHECK(!isInPerformLayout()); DCHECK(!m_frame->document() || !m_frame->document()->inStyleRecalc()); @@ -4795,7 +4802,9 @@ // paint one of the children with an out-of-date layout before // |updateRenderThrottlingStatus| has made it throttled or 2) fail to // unthrottle a child whose parent is unthrottled by a later notification. - if (wasThrottled != isThrottled || forceThrottlingInvalidation) { + if (notifyChildrenBehavior == NotifyChildren && + (wasThrottled != isThrottled || + forceThrottlingInvalidationBehavior == ForceThrottlingInvalidation)) { for (const Member<Widget>& child : *children()) { if (child->isFrameView()) { FrameView* childView = toFrameView(child); @@ -4806,7 +4815,8 @@ } ScrollingCoordinator* scrollingCoordinator = this->scrollingCoordinator(); - if (becameUnthrottled || forceThrottlingInvalidation) { + if (becameUnthrottled || + forceThrottlingInvalidationBehavior == ForceThrottlingInvalidation) { // ScrollingCoordinator needs to update according to the new throttling // status. if (scrollingCoordinator)
diff --git a/third_party/WebKit/Source/core/frame/FrameView.h b/third_party/WebKit/Source/core/frame/FrameView.h index c042f64..196d737e 100644 --- a/third_party/WebKit/Source/core/frame/FrameView.h +++ b/third_party/WebKit/Source/core/frame/FrameView.h
@@ -1028,9 +1028,17 @@ void updateViewportIntersectionsForSubtree( DocumentLifecycle::LifecycleState targetState); - void updateRenderThrottlingStatus(bool hidden, - bool subtreeThrottled, - bool forceThrottlingInvalidation = false); + + enum ForceThrottlingInvalidationBehavior { + DontForceThrottlingInvalidation, + ForceThrottlingInvalidation + }; + enum NotifyChildrenBehavior { DontNotifyChildren, NotifyChildren }; + void updateRenderThrottlingStatus( + bool hidden, + bool subtreeThrottled, + ForceThrottlingInvalidationBehavior = DontForceThrottlingInvalidation, + NotifyChildrenBehavior = NotifyChildren); void notifyResizeObservers(); // PaintInvalidationCapableScrollableArea
diff --git a/third_party/WebKit/Source/core/frame/FrameViewTest.cpp b/third_party/WebKit/Source/core/frame/FrameViewTest.cpp index a3f1cd19..a182ea5 100644 --- a/third_party/WebKit/Source/core/frame/FrameViewTest.cpp +++ b/third_party/WebKit/Source/core/frame/FrameViewTest.cpp
@@ -4,9 +4,12 @@ #include "core/frame/FrameView.h" +#include <memory> + #include "bindings/core/v8/ExceptionState.h" #include "core/frame/Settings.h" #include "core/html/HTMLElement.h" +#include "core/layout/LayoutBoxModelObject.h" #include "core/layout/LayoutObject.h" #include "core/loader/EmptyClients.h" #include "core/page/Page.h" @@ -18,7 +21,6 @@ #include "platform/testing/RuntimeEnabledFeaturesTestHelpers.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" -#include <memory> using testing::_; using testing::AnyNumber; @@ -137,5 +139,40 @@ EXPECT_TRUE(document().view()->isVisuallyNonEmpty()); } +TEST_P(FrameViewTest, StyleChangeUpdatesViewportConstrainedObjects) { + // When using root layer scrolling there is no concept of viewport constrained + // objects, so skip this test. + if (RuntimeEnabledFeatures::rootLayerScrollingEnabled()) + return; + + document().body()->setInnerHTML( + "<style>.container { height: 200%; }" + "#sticky { position: sticky; top: 0; height: 50px; }</style>" + "<div class='container'><div id='sticky'></div></div>"); + document().view()->updateAllLifecyclePhases(); + + LayoutBoxModelObject* sticky = toLayoutBoxModelObject( + document().getElementById("sticky")->layoutObject()); + + EXPECT_TRUE( + document().view()->viewportConstrainedObjects()->contains(sticky)); + + // Making the element non-sticky should remove it from the set of + // viewport-constrained objects. + document().getElementById("sticky")->setAttribute(HTMLNames::styleAttr, + "position: relative"); + document().view()->updateAllLifecyclePhases(); + + EXPECT_FALSE( + document().view()->viewportConstrainedObjects()->contains(sticky)); + + // And making it sticky again should put it back in that list. + document().getElementById("sticky")->setAttribute(HTMLNames::styleAttr, ""); + document().view()->updateAllLifecyclePhases(); + + EXPECT_TRUE( + document().view()->viewportConstrainedObjects()->contains(sticky)); +} + } // namespace } // namespace blink
diff --git a/third_party/WebKit/Source/core/layout/compositing/CompositingInputsUpdater.cpp b/third_party/WebKit/Source/core/layout/compositing/CompositingInputsUpdater.cpp index 808736d..ae0dcc6 100644 --- a/third_party/WebKit/Source/core/layout/compositing/CompositingInputsUpdater.cpp +++ b/third_party/WebKit/Source/core/layout/compositing/CompositingInputsUpdater.cpp
@@ -102,23 +102,27 @@ layer->updateAncestorOverflowLayer(info.lastOverflowClipLayer); if (info.lastOverflowClipLayer && layer->needsCompositingInputsUpdate() && layer->layoutObject().style()->position() == EPosition::kSticky) { - if (info.lastOverflowClipLayer != previousOverflowLayer && - !RuntimeEnabledFeatures::rootLayerScrollingEnabled()) { - // Old ancestor scroller should no longer have these constraints. - ASSERT(!previousOverflowLayer || - !previousOverflowLayer->getScrollableArea() - ->stickyConstraintsMap() - .contains(layer)); + if (!RuntimeEnabledFeatures::rootLayerScrollingEnabled()) { + if (info.lastOverflowClipLayer != previousOverflowLayer) { + // Old ancestor scroller should no longer have these constraints. + DCHECK(!previousOverflowLayer || + !previousOverflowLayer->getScrollableArea() + ->stickyConstraintsMap() + .contains(layer)); + + // If our ancestor scroller has changed and the previous one was the + // root layer, we are no longer viewport constrained. + if (previousOverflowLayer && previousOverflowLayer->isRootLayer()) { + layer->layoutObject() + .view() + ->frameView() + ->removeViewportConstrainedObject(layer->layoutObject()); + } + } if (info.lastOverflowClipLayer->isRootLayer()) { layer->layoutObject().view()->frameView()->addViewportConstrainedObject( layer->layoutObject()); - } else if (previousOverflowLayer && - previousOverflowLayer->isRootLayer()) { - layer->layoutObject() - .view() - ->frameView() - ->removeViewportConstrainedObject(layer->layoutObject()); } } layer->layoutObject().updateStickyPositionConstraints();
diff --git a/third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp b/third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp index fdaaf3f3..6f23acd 100644 --- a/third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp +++ b/third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp
@@ -4,7 +4,6 @@ #include "core/loader/ThreadableLoader.h" -#include "core/dom/ExecutionContextTask.h" #include "core/loader/DocumentThreadableLoader.h" #include "core/loader/ThreadableLoaderClient.h" #include "core/loader/WorkerThreadableLoader.h" @@ -325,16 +324,13 @@ } // WorkerLoaderProxyProvider methods. - void postTaskToLoader(const WebTraceLocation& location, - std::unique_ptr<ExecutionContextTask> task) override { + void postTaskToLoader( + const WebTraceLocation& location, + std::unique_ptr<WTF::CrossThreadClosure> task) override { DCHECK(m_workerThread); DCHECK(m_workerThread->isCurrentThread()); m_parentFrameTaskRunners->get(TaskType::Networking) - ->postTask( - BLINK_FROM_HERE, - crossThreadBind(&ExecutionContextTask::performTaskIfContextIsValid, - WTF::passed(std::move(task)), - wrapCrossThreadWeakPersistent(&document()))); + ->postTask(BLINK_FROM_HERE, std::move(task)); } void postTaskToWorkerGlobalScope( @@ -344,6 +340,8 @@ m_workerThread->postTask(location, std::move(task)); } + ExecutionContext* getLoaderExecutionContext() override { return &document(); } + RefPtr<SecurityOrigin> m_securityOrigin; std::unique_ptr<MockWorkerReportingProxy> m_mockWorkerReportingProxy; std::unique_ptr<WorkerThreadForTest> m_workerThread;
diff --git a/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp b/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp index 70d417f..c3ed20de 100644 --- a/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp +++ b/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp
@@ -31,7 +31,6 @@ #include "core/loader/WorkerThreadableLoader.h" #include "core/dom/Document.h" -#include "core/dom/ExecutionContextTask.h" #include "core/loader/DocumentThreadableLoader.h" #include "core/timing/WorkerGlobalScopePerformance.h" #include "core/workers/WorkerGlobalScope.h" @@ -236,7 +235,7 @@ m_workerLoaderProxy->postTaskToLoader( BLINK_FROM_HERE, - createCrossThreadTask( + crossThreadBind( &MainThreadLoaderHolder::createAndStart, wrapCrossThreadPersistent(this), m_workerLoaderProxy, wrapCrossThreadPersistent( @@ -272,16 +271,17 @@ return; m_workerLoaderProxy->postTaskToLoader( BLINK_FROM_HERE, - createCrossThreadTask(&MainThreadLoaderHolder::overrideTimeout, - m_mainThreadLoaderHolder, timeoutMilliseconds)); + crossThreadBind(&MainThreadLoaderHolder::overrideTimeout, + m_mainThreadLoaderHolder, timeoutMilliseconds)); } void WorkerThreadableLoader::cancel() { DCHECK(!isMainThread()); if (m_mainThreadLoaderHolder) { m_workerLoaderProxy->postTaskToLoader( - BLINK_FROM_HERE, createCrossThreadTask(&MainThreadLoaderHolder::cancel, - m_mainThreadLoaderHolder)); + BLINK_FROM_HERE, + crossThreadBind(&MainThreadLoaderHolder::cancel, + m_mainThreadLoaderHolder)); m_mainThreadLoaderHolder = nullptr; } @@ -306,9 +306,9 @@ if (!m_client) { // The thread is terminating. m_workerLoaderProxy->postTaskToLoader( - BLINK_FROM_HERE, createCrossThreadTask(&MainThreadLoaderHolder::cancel, - wrapCrossThreadPersistent( - mainThreadLoaderHolder))); + BLINK_FROM_HERE, + crossThreadBind(&MainThreadLoaderHolder::cancel, + wrapCrossThreadPersistent(mainThreadLoaderHolder))); return; } @@ -432,11 +432,13 @@ std::unique_ptr<CrossThreadResourceRequestData> request, const ThreadableLoaderOptions& options, const ResourceLoaderOptions& resourceLoaderOptions, - PassRefPtr<WaitableEventWithTasks> eventWithTasks, - ExecutionContext* executionContext) { + PassRefPtr<WaitableEventWithTasks> eventWithTasks) { DCHECK(isMainThread()); TaskForwarder* forwarder; RefPtr<WorkerLoaderProxy> loaderProxy = passLoaderProxy; + ExecutionContext* loaderContext = loaderProxy->getLoaderExecutionContext(); + if (!loaderContext) + return; if (eventWithTasks) forwarder = new SyncTaskForwarder(std::move(eventWithTasks)); else @@ -456,9 +458,8 @@ crossThreadBind(&WorkerThreadableLoader::didStart, wrapCrossThreadPersistent(workerLoader), wrapCrossThreadPersistent(mainThreadLoaderHolder))); - mainThreadLoaderHolder->start(*toDocument(executionContext), - std::move(request), options, - resourceLoaderOptions); + mainThreadLoaderHolder->start(*toDocument(loaderContext), std::move(request), + options, resourceLoaderOptions); } WorkerThreadableLoader::MainThreadLoaderHolder::~MainThreadLoaderHolder() {
diff --git a/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.h b/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.h index 5f3d2b0e..6eabba2 100644 --- a/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.h +++ b/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.h
@@ -145,8 +145,7 @@ std::unique_ptr<CrossThreadResourceRequestData>, const ThreadableLoaderOptions&, const ResourceLoaderOptions&, - PassRefPtr<WaitableEventWithTasks>, - ExecutionContext*); + PassRefPtr<WaitableEventWithTasks>); ~MainThreadLoaderHolder() override; void overrideTimeout(unsigned long timeoutMillisecond);
diff --git a/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.cpp b/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.cpp index d60433a..a3fe6be 100644 --- a/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.cpp +++ b/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.cpp
@@ -72,15 +72,14 @@ void ThreadedMessagingProxyBase::postTaskToLoader( const WebTraceLocation& location, - std::unique_ptr<ExecutionContextTask> task) { - DCHECK(getExecutionContext()->isDocument()); - getParentFrameTaskRunners() - ->get(TaskType::Networking) - ->postTask(BLINK_FROM_HERE, - crossThreadBind( - &ExecutionContextTask::performTaskIfContextIsValid, - WTF::passed(std::move(task)), - wrapCrossThreadWeakPersistent(getExecutionContext()))); + std::unique_ptr<WTF::CrossThreadClosure> task) { + m_parentFrameTaskRunners->get(TaskType::Networking) + ->postTask(BLINK_FROM_HERE, std::move(task)); +} + +ExecutionContext* ThreadedMessagingProxyBase::getLoaderExecutionContext() { + DCHECK(isParentContextThread()); + return getExecutionContext(); } void ThreadedMessagingProxyBase::countFeature(UseCounter::Feature feature) { @@ -168,7 +167,7 @@ bool ThreadedMessagingProxyBase::isParentContextThread() const { // TODO(nhiroki): Nested worker is not supported yet, so the parent context // thread should be equal to the main thread (http://crbug.com/31666). - DCHECK(getExecutionContext()->isDocument()); + DCHECK(m_executionContext->isDocument()); return isMainThread(); }
diff --git a/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.h b/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.h index 308cf11..978f8059 100644 --- a/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.h +++ b/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.h
@@ -44,7 +44,7 @@ // 'virtual' for testing. virtual void workerThreadTerminated(); - // Accessed from both the parent thread and the worker. + // Accessed only from the parent thread. ExecutionContext* getExecutionContext() const { return m_executionContext.get(); } @@ -81,10 +81,11 @@ // These methods are called on different threads to schedule loading // requests and to send callbacks back to WorkerGlobalScope. void postTaskToLoader(const WebTraceLocation&, - std::unique_ptr<ExecutionContextTask>) override; + std::unique_ptr<WTF::CrossThreadClosure>) override; void postTaskToWorkerGlobalScope( const WebTraceLocation&, std::unique_ptr<WTF::CrossThreadClosure>) override; + ExecutionContext* getLoaderExecutionContext() override; private: friend class InProcessWorkerMessagingProxyForTest; @@ -92,11 +93,7 @@ void parentObjectDestroyedInternal(); - // Accessed cross-thread when worker thread posts tasks to the parent; - // it is not ideal to have ExecutionContext be cross-thread accessible. - // - // TODO: try to drop the need for the CrossThreadPersistent<>. - CrossThreadPersistent<ExecutionContext> m_executionContext; + Persistent<ExecutionContext> m_executionContext; Persistent<WorkerInspectorProxy> m_workerInspectorProxy; // Accessed cross-thread when worker thread posts tasks to the parent. CrossThreadPersistent<ParentFrameTaskRunners> m_parentFrameTaskRunners;
diff --git a/third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp b/third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp index 243a09d..3ed77a5 100644 --- a/third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp +++ b/third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp
@@ -33,7 +33,6 @@ #include "bindings/core/v8/V8AbstractEventListener.h" #include "core/dom/ContextLifecycleNotifier.h" #include "core/dom/ExceptionCode.h" -#include "core/dom/ExecutionContextTask.h" #include "core/dom/SuspendableObject.h" #include "core/events/ErrorEvent.h" #include "core/events/Event.h" @@ -50,6 +49,7 @@ #include "core/workers/WorkerReportingProxy.h" #include "core/workers/WorkerScriptLoader.h" #include "core/workers/WorkerThread.h" +#include "platform/CrossThreadFunctional.h" #include "platform/InstanceCounters.h" #include "platform/loader/fetch/MemoryCache.h" #include "platform/network/ContentSecurityPolicyParsers.h" @@ -350,9 +350,10 @@ } void WorkerGlobalScope::removeURLFromMemoryCache(const KURL& url) { - m_thread->workerLoaderProxy()->postTaskToLoader( - BLINK_FROM_HERE, - createCrossThreadTask(&removeURLFromMemoryCacheInternal, url)); + m_thread->getParentFrameTaskRunners() + ->get(TaskType::Networking) + ->postTask(BLINK_FROM_HERE, + crossThreadBind(&removeURLFromMemoryCacheInternal, url)); } KURL WorkerGlobalScope::virtualCompleteURL(const String& url) const {
diff --git a/third_party/WebKit/Source/core/workers/WorkerLoaderProxy.cpp b/third_party/WebKit/Source/core/workers/WorkerLoaderProxy.cpp index dd8655f5..01e0c417 100644 --- a/third_party/WebKit/Source/core/workers/WorkerLoaderProxy.cpp +++ b/third_party/WebKit/Source/core/workers/WorkerLoaderProxy.cpp
@@ -4,7 +4,7 @@ #include "core/workers/WorkerLoaderProxy.h" -#include "core/dom/ExecutionContextTask.h" +#include "core/dom/ExecutionContext.h" namespace blink { @@ -26,7 +26,7 @@ void WorkerLoaderProxy::postTaskToLoader( const WebTraceLocation& location, - std::unique_ptr<ExecutionContextTask> task) { + std::unique_ptr<WTF::CrossThreadClosure> task) { MutexLocker locker(m_lock); DCHECK(!isMainThread()); if (!m_loaderProxyProvider) @@ -37,11 +37,20 @@ void WorkerLoaderProxy::postTaskToWorkerGlobalScope( const WebTraceLocation& location, std::unique_ptr<WTF::CrossThreadClosure> task) { - MutexLocker locker(m_lock); DCHECK(isMainThread()); + // Note: No locking needed for the access from the main thread. if (!m_loaderProxyProvider) return; m_loaderProxyProvider->postTaskToWorkerGlobalScope(location, std::move(task)); } +ExecutionContext* WorkerLoaderProxy::getLoaderExecutionContext() { + DCHECK(isMainThread()); + // Note: No locking needed for the access from the main thread. + if (!m_loaderProxyProvider) + return nullptr; + DCHECK(m_loaderProxyProvider->getLoaderExecutionContext()->isContextThread()); + return m_loaderProxyProvider->getLoaderExecutionContext(); +} + } // namespace blink
diff --git a/third_party/WebKit/Source/core/workers/WorkerLoaderProxy.h b/third_party/WebKit/Source/core/workers/WorkerLoaderProxy.h index 770719f..c2235f02 100644 --- a/third_party/WebKit/Source/core/workers/WorkerLoaderProxy.h +++ b/third_party/WebKit/Source/core/workers/WorkerLoaderProxy.h
@@ -32,13 +32,17 @@ #define WorkerLoaderProxy_h #include "core/CoreExport.h" -#include "core/dom/ExecutionContext.h" #include "public/platform/WebTraceLocation.h" #include "wtf/Forward.h" +#include "wtf/Functional.h" +#include "wtf/PassRefPtr.h" #include "wtf/ThreadSafeRefCounted.h" +#include "wtf/ThreadingPrimitives.h" namespace blink { +class ExecutionContext; + // The WorkerLoaderProxy is a proxy to the loader context. Normally, the // document on the main thread provides loading services for the subordinate // workers. WorkerLoaderProxy provides 2-way communications to the Document @@ -64,13 +68,17 @@ // Posts a task to the thread which runs the loading code (normally, the main // thread). This must be called from a worker thread. virtual void postTaskToLoader(const WebTraceLocation&, - std::unique_ptr<ExecutionContextTask>) = 0; + std::unique_ptr<WTF::CrossThreadClosure>) = 0; // Posts callbacks from loading code to the WorkerGlobalScope. This must be // called from the main thread. virtual void postTaskToWorkerGlobalScope( const WebTraceLocation&, std::unique_ptr<WTF::CrossThreadClosure>) = 0; + + // It is guaranteed that this gets accessed only on the thread where + // the ExecutionContext is bound. + virtual ExecutionContext* getLoaderExecutionContext() = 0; }; class CORE_EXPORT WorkerLoaderProxy final @@ -85,12 +93,17 @@ // This must be called from a worker thread. void postTaskToLoader(const WebTraceLocation&, - std::unique_ptr<ExecutionContextTask>); + std::unique_ptr<WTF::CrossThreadClosure>); // This must be called from the main thread. void postTaskToWorkerGlobalScope(const WebTraceLocation&, std::unique_ptr<WTF::CrossThreadClosure>); + // This may return nullptr. + // This must be called from the main thread (== the thread of the + // loader execution context). + ExecutionContext* getLoaderExecutionContext(); + // Notification from the provider that it can no longer be accessed. An // implementation of WorkerLoaderProxyProvider is required to call // detachProvider() when finalizing. This must be called from the main thread. @@ -99,7 +112,7 @@ private: explicit WorkerLoaderProxy(WorkerLoaderProxyProvider*); - Mutex m_lock; + WTF::Mutex m_lock; WorkerLoaderProxyProvider* m_loaderProxyProvider; };
diff --git a/third_party/WebKit/Source/core/workers/WorkerThreadTestHelper.h b/third_party/WebKit/Source/core/workers/WorkerThreadTestHelper.h index 380e6266..a5f69a82 100644 --- a/third_party/WebKit/Source/core/workers/WorkerThreadTestHelper.h +++ b/third_party/WebKit/Source/core/workers/WorkerThreadTestHelper.h
@@ -41,7 +41,7 @@ ~MockWorkerLoaderProxyProvider() override {} void postTaskToLoader(const WebTraceLocation&, - std::unique_ptr<ExecutionContextTask>) override { + std::unique_ptr<WTF::CrossThreadClosure>) override { NOTIMPLEMENTED(); } @@ -50,6 +50,11 @@ std::unique_ptr<WTF::CrossThreadClosure>) override { NOTIMPLEMENTED(); } + + ExecutionContext* getLoaderExecutionContext() override { + NOTIMPLEMENTED(); + return nullptr; + } }; class MockWorkerReportingProxy : public WorkerReportingProxy {
diff --git a/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp b/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp index ecddcb2..79cf91f7 100644 --- a/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp +++ b/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp
@@ -30,15 +30,16 @@ #include "modules/websockets/WorkerWebSocketChannel.h" +#include <memory> #include "core/dom/DOMArrayBuffer.h" #include "core/dom/Document.h" #include "core/dom/ExecutionContext.h" -#include "core/dom/ExecutionContextTask.h" #include "core/fileapi/Blob.h" #include "core/workers/WorkerGlobalScope.h" #include "core/workers/WorkerLoaderProxy.h" #include "core/workers/WorkerThread.h" #include "modules/websockets/DocumentWebSocketChannel.h" +#include "platform/CrossThreadFunctional.h" #include "platform/WaitableEvent.h" #include "platform/heap/SafePoint.h" #include "public/platform/Platform.h" @@ -47,7 +48,6 @@ #include "wtf/PtrUtil.h" #include "wtf/text/CString.h" #include "wtf/text/WTFString.h" -#include <memory> namespace blink { @@ -364,15 +364,18 @@ void Bridge::connectOnMainThread( std::unique_ptr<SourceLocation> location, + RefPtr<WorkerLoaderProxy> loaderProxy, WorkerThreadLifecycleContext* workerThreadLifecycleContext, const KURL& url, const String& protocol, - WebSocketChannelSyncHelper* syncHelper, - ExecutionContext* context) { + WebSocketChannelSyncHelper* syncHelper) { DCHECK(isMainThread()); DCHECK(!m_peer); + ExecutionContext* loaderContext = loaderProxy->getLoaderExecutionContext(); + if (!loaderContext) + return; Peer* peer = new Peer(this, m_loaderProxy, workerThreadLifecycleContext); - if (peer->initialize(std::move(location), context)) { + if (peer->initialize(std::move(location), loaderContext)) { m_peer = peer; syncHelper->setConnectRequestResult(m_peer->connect(url, protocol)); } @@ -387,9 +390,9 @@ WebSocketChannelSyncHelper syncHelper; m_loaderProxy->postTaskToLoader( BLINK_FROM_HERE, - createCrossThreadTask( + crossThreadBind( &Bridge::connectOnMainThread, wrapCrossThreadPersistent(this), - WTF::passed(location->clone()), + WTF::passed(location->clone()), m_loaderProxy, wrapCrossThreadPersistent( m_workerGlobalScope->thread()->getWorkerThreadLifecycleContext()), url, protocol, crossThreadUnretained(&syncHelper))); @@ -407,8 +410,8 @@ m_loaderProxy->postTaskToLoader( BLINK_FROM_HERE, - createCrossThreadTask(&Peer::sendTextAsCharVector, m_peer, - WTF::passed(std::move(data)))); + crossThreadBind(&Peer::sendTextAsCharVector, m_peer, + WTF::passed(std::move(data)))); } void Bridge::send(const DOMArrayBuffer& binaryData, @@ -426,22 +429,21 @@ m_loaderProxy->postTaskToLoader( BLINK_FROM_HERE, - createCrossThreadTask(&Peer::sendBinaryAsCharVector, m_peer, - WTF::passed(std::move(data)))); + crossThreadBind(&Peer::sendBinaryAsCharVector, m_peer, + WTF::passed(std::move(data)))); } void Bridge::send(PassRefPtr<BlobDataHandle> data) { DCHECK(m_peer); m_loaderProxy->postTaskToLoader( BLINK_FROM_HERE, - createCrossThreadTask(&Peer::sendBlob, m_peer, std::move(data))); + crossThreadBind(&Peer::sendBlob, m_peer, std::move(data))); } void Bridge::close(int code, const String& reason) { DCHECK(m_peer); m_loaderProxy->postTaskToLoader( - BLINK_FROM_HERE, - createCrossThreadTask(&Peer::close, m_peer, code, reason)); + BLINK_FROM_HERE, crossThreadBind(&Peer::close, m_peer, code, reason)); } void Bridge::fail(const String& reason, @@ -449,16 +451,17 @@ std::unique_ptr<SourceLocation> location) { DCHECK(m_peer); m_loaderProxy->postTaskToLoader( - BLINK_FROM_HERE, createCrossThreadTask(&Peer::fail, m_peer, reason, level, - WTF::passed(location->clone()))); + BLINK_FROM_HERE, + crossThreadBind(&Peer::fail, m_peer, reason, level, + WTF::passed(location->clone()))); } void Bridge::disconnect() { if (!m_peer) return; - m_loaderProxy->postTaskToLoader( - BLINK_FROM_HERE, createCrossThreadTask(&Peer::disconnect, m_peer)); + m_loaderProxy->postTaskToLoader(BLINK_FROM_HERE, + crossThreadBind(&Peer::disconnect, m_peer)); m_client = nullptr; m_peer = nullptr;
diff --git a/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.h b/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.h index 01ea434..883b12d6 100644 --- a/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.h +++ b/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.h
@@ -160,11 +160,11 @@ void disconnect(); void connectOnMainThread(std::unique_ptr<SourceLocation>, + RefPtr<WorkerLoaderProxy>, WorkerThreadLifecycleContext*, const KURL&, const String& protocol, - WebSocketChannelSyncHelper*, - ExecutionContext*); + WebSocketChannelSyncHelper*); // Returns null when |disconnect| has already been called. WebSocketChannelClient* client() { return m_client; }
diff --git a/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.cpp b/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.cpp index e0a9d55..ebae8080 100644 --- a/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.cpp +++ b/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.cpp
@@ -262,16 +262,9 @@ void WebEmbeddedWorkerImpl::postTaskToLoader( const WebTraceLocation& location, - std::unique_ptr<ExecutionContextTask> task) { - // This cross-thread operation is brittle wrt per-thread heaps, - // posting a task to main-thread owned objects. + std::unique_ptr<WTF::CrossThreadClosure> task) { m_mainThreadTaskRunners->get(TaskType::Networking) - ->postTask( - BLINK_FROM_HERE, - crossThreadBind( - &ExecutionContextTask::performTaskIfContextIsValid, - WTF::passed(std::move(task)), - wrapCrossThreadWeakPersistent(m_mainFrame->frame()->document()))); + ->postTask(BLINK_FROM_HERE, std::move(task)); } void WebEmbeddedWorkerImpl::postTaskToWorkerGlobalScope( @@ -282,6 +275,10 @@ m_workerThread->postTask(location, std::move(task)); } +ExecutionContext* WebEmbeddedWorkerImpl::getLoaderExecutionContext() { + return m_mainFrame->frame()->document(); +} + void WebEmbeddedWorkerImpl::prepareShadowPageForLoader() { // Create 'shadow page', which is never displayed and is used mainly to // provide a context for loading on the main thread.
diff --git a/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.h b/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.h index 2a8c516..78856bc 100644 --- a/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.h +++ b/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.h
@@ -102,10 +102,11 @@ // WorkerLoaderProxyProvider void postTaskToLoader(const WebTraceLocation&, - std::unique_ptr<ExecutionContextTask>) override; + std::unique_ptr<WTF::CrossThreadClosure>) override; void postTaskToWorkerGlobalScope( const WebTraceLocation&, std::unique_ptr<WTF::CrossThreadClosure>) override; + ExecutionContext* getLoaderExecutionContext() override; WebEmbeddedWorkerStartData m_workerStartData; @@ -137,10 +138,7 @@ // are guaranteed to exist while this object is around. WebView* m_webView; - // Accessed cross-thread when worker thread posts tasks to the parent. - // - // TODO: avoid reaching into the local frame object when posting. - CrossThreadPersistent<WebLocalFrameImpl> m_mainFrame; + Persistent<WebLocalFrameImpl> m_mainFrame; bool m_loadingShadowPage; bool m_askedToTerminate;
diff --git a/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp b/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp index 47e1ed2..6b25a1d 100644 --- a/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp +++ b/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp
@@ -31,7 +31,6 @@ #include "web/WebSharedWorkerImpl.h" #include "core/dom/Document.h" -#include "core/dom/ExecutionContextTask.h" #include "core/events/MessageEvent.h" #include "core/html/HTMLFormElement.h" #include "core/inspector/ConsoleMessage.h" @@ -303,13 +302,9 @@ void WebSharedWorkerImpl::postTaskToLoader( const WebTraceLocation& location, - std::unique_ptr<ExecutionContextTask> task) { + std::unique_ptr<WTF::CrossThreadClosure> task) { m_parentFrameTaskRunners->get(TaskType::Networking) - ->postTask(FROM_HERE, - crossThreadBind( - &ExecutionContextTask::performTaskIfContextIsValid, - WTF::passed(std::move(task)), - wrapCrossThreadWeakPersistent(m_loadingDocument.get()))); + ->postTask(FROM_HERE, std::move(task)); } void WebSharedWorkerImpl::postTaskToWorkerGlobalScope( @@ -318,6 +313,10 @@ m_workerThread->postTask(location, std::move(task)); } +ExecutionContext* WebSharedWorkerImpl::getLoaderExecutionContext() { + return m_loadingDocument.get(); +} + void WebSharedWorkerImpl::connect(WebMessagePortChannel* webChannel) { DCHECK(isMainThread()); workerThread()->postTask(
diff --git a/third_party/WebKit/Source/web/WebSharedWorkerImpl.h b/third_party/WebKit/Source/web/WebSharedWorkerImpl.h index ea4bb1b..d32b805 100644 --- a/third_party/WebKit/Source/web/WebSharedWorkerImpl.h +++ b/third_party/WebKit/Source/web/WebSharedWorkerImpl.h
@@ -151,17 +151,15 @@ // WorkerLoaderProxyProvider void postTaskToLoader(const WebTraceLocation&, - std::unique_ptr<ExecutionContextTask>) override; + std::unique_ptr<WTF::CrossThreadClosure>) override; void postTaskToWorkerGlobalScope( const WebTraceLocation&, std::unique_ptr<WTF::CrossThreadClosure>) override; + ExecutionContext* getLoaderExecutionContext() override; // 'shadow page' - created to proxy loading requests from the worker. // Will be accessed by worker thread when posting tasks. - // - // TODO: it is undesirable to keep a cross-thread reference to this - // document; avoid reaching into the document when posting. - CrossThreadPersistent<ExecutionContext> m_loadingDocument; + Persistent<ExecutionContext> m_loadingDocument; WebView* m_webView; Persistent<WebLocalFrameImpl> m_mainFrame; bool m_askedToTerminate;
diff --git a/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp b/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp index 74d6575a..bbc356f 100644 --- a/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp +++ b/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp
@@ -1051,4 +1051,36 @@ EXPECT_FALSE(frameDocument->view()->canThrottleRendering()); } +TEST_P(FrameThrottlingTest, DisplayNoneChildrenRemainThrottled) { + // Create two nested frames which are throttled. + SimRequest mainResource("https://example.com/", "text/html"); + SimRequest frameResource("https://example.com/iframe.html", "text/html"); + SimRequest childFrameResource("https://example.com/child-iframe.html", + "text/html"); + + loadURL("https://example.com/"); + mainResource.complete("<iframe id=frame sandbox src=iframe.html></iframe>"); + frameResource.complete( + "<iframe id=child-frame sandbox src=child-iframe.html></iframe>"); + childFrameResource.complete(""); + + // Move both frames offscreen to make them throttled. + auto* frameElement = toHTMLIFrameElement(document().getElementById("frame")); + auto* childFrameElement = toHTMLIFrameElement( + frameElement->contentDocument()->getElementById("child-frame")); + frameElement->setAttribute(styleAttr, "transform: translateY(480px)"); + compositeFrame(); + EXPECT_TRUE(frameElement->contentDocument()->view()->canThrottleRendering()); + EXPECT_TRUE( + childFrameElement->contentDocument()->view()->canThrottleRendering()); + + // Setting display:none for the parent frame unthrottles the parent but not + // the child. This behavior matches Safari. + frameElement->setAttribute(styleAttr, "display: none"); + compositeFrame(); + EXPECT_FALSE(frameElement->contentDocument()->view()->canThrottleRendering()); + EXPECT_TRUE( + childFrameElement->contentDocument()->view()->canThrottleRendering()); +} + } // namespace blink
diff --git a/third_party/WebKit/Source/wtf/RefCounted.h b/third_party/WebKit/Source/wtf/RefCounted.h index a4847ef..a2820ea 100644 --- a/third_party/WebKit/Source/wtf/RefCounted.h +++ b/third_party/WebKit/Source/wtf/RefCounted.h
@@ -23,10 +23,13 @@ #include "wtf/Allocator.h" #include "wtf/Assertions.h" -#include "wtf/InstanceCounter.h" #include "wtf/Noncopyable.h" #include "wtf/WTFExport.h" +#if ENABLE(INSTANCE_COUNTER) +#include "wtf/InstanceCounter.h" +#endif + #if DCHECK_IS_ON() #define CHECK_REF_COUNTED_LIFECYCLE 1 #include "wtf/ThreadRestrictionVerifier.h" @@ -152,7 +155,7 @@ } protected: -#ifdef ENABLE_INSTANCE_COUNTER +#if ENABLE(INSTANCE_COUNTER) RefCounted() { incrementInstanceCount<T>(static_cast<T*>(this)); } ~RefCounted() { decrementInstanceCount<T>(static_cast<T*>(this)); }
diff --git a/third_party/re2/README.chromium b/third_party/re2/README.chromium index a941175a..730859a 100644 --- a/third_party/re2/README.chromium +++ b/third_party/re2/README.chromium
@@ -1,8 +1,8 @@ Name: re2 - an efficient, principled regular expression library Short Name: re2 URL: https://github.com/google/re2 -Version: 193486d7164a40ce28d8ec64552df129c0558bad -Date: 2017-02-17 +Version: db20d46c05900a12539225fe800dd860b14e0061 +Date: 2017-02-22 License: BSD 3-Clause License File: LICENSE Security Critical: yes
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 6bf2c91..4d01462 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -39856,6 +39856,9 @@ <histogram name="NewTabPage.ContentSuggestions.TimeSinceLastBackgroundFetch" units="ms"> + <obsolete> + Replaced by NewTabPage.ContentSuggestions.TimeSinceSuggestionFetched. + </obsolete> <owner>markusheintz@chromium.org</owner> <summary> Android: The time since the last successful background fetch of remote @@ -39864,6 +39867,16 @@ </summary> </histogram> +<histogram name="NewTabPage.ContentSuggestions.TimeSinceSuggestionFetched" + units="ms"> + <owner>markusheintz@chromium.org</owner> + <summary> + Android: The time since the displayed content suggestions was fetched. + Recorded when the user looks at content suggestions on the NTP. The metric + is only recorded for suggestions of the Articles for you section. + </summary> +</histogram> + <histogram name="NewTabPage.ContentSuggestions.UIUpdateResult" enum="ContentSuggestionsUIUpdateResult"> <obsolete> @@ -46390,6 +46403,23 @@ </summary> </histogram> +<histogram name="Permissions.AutoBlocker.EmbargoPromptSuppression" + enum="PermissionEmbargoStatus"> + <owner>dominickn@chromium.org</owner> + <owner>kcarattini@chromium.org</owner> + <summary> + For every permission request that would show a prompt to the user, this + metric tracks whether the request is suppressed by embargo (and the source + of that embargo), or whether there is no embargo and the prompt is shown. + This is the proportion of permission prompts that the user would have seen + that have been blocked due to embargo. + + If the (origin, permission) pair was previously placed under embargo, no + prompt is displayed and the reason for embargo is recorded. Otherwise, the + prompt is shown and a no embargo reason is recorded. + </summary> +</histogram> + <histogram name="Permissions.AutoBlocker.EmbargoStatus" enum="PermissionEmbargoStatus"> <owner>dominickn@chromium.org</owner> @@ -97561,6 +97591,7 @@ <int value="-55944747" label="disable-child-account-detection"/> <int value="-52483823" label="disable-new-video-renderer"/> <int value="-52241456" label="enable-single-click-autofill"/> + <int value="-50021298" label="ash-adjustable-large-cursor"/> <int value="-48920737" label="enable-smooth-scrolling"/> <int value="-45532639" label="enable-default-media-session"/> <int value="-45074716" label="SystemDownloadManager:disabled"/>
diff --git a/ui/webui/resources/cr_elements/cr_slider/cr_slider.html b/ui/webui/resources/cr_elements/cr_slider/cr_slider.html index 71710d47..c398fc5e 100644 --- a/ui/webui/resources/cr_elements/cr_slider/cr_slider.html +++ b/ui/webui/resources/cr_elements/cr_slider/cr_slider.html
@@ -43,7 +43,8 @@ </style> <div class="outer"> <paper-slider id="slider" disabled="[[disabled]]" snaps - on-immediate-value-changed="onSliderChanged_"> + on-immediate-value-changed="onSliderChanged_" max="[[max]]" + min="[[min]]"> </paper-slider> <div id="labels"> <div id="label-begin">[[labelMin]]</div>
diff --git a/ui/webui/resources/cr_elements/cr_slider/cr_slider.js b/ui/webui/resources/cr_elements/cr_slider/cr_slider.js index 128ddfa..c95743a1 100644 --- a/ui/webui/resources/cr_elements/cr_slider/cr_slider.js +++ b/ui/webui/resources/cr_elements/cr_slider/cr_slider.js
@@ -22,7 +22,7 @@ }, /** @type {!Array<number>} Values corresponding to each tick. */ - tickValues: Array, + tickValues: {type: Array, value: []}, disabled: { type: Boolean, @@ -30,6 +30,10 @@ reflectToAttribute: true, }, + min: Number, + + max: Number, + labelMin: String, labelMax: String, @@ -47,6 +51,8 @@ onSliderChanged_: function() { if (this.tickValues && this.tickValues.length > 0) this.value = this.tickValues[this.$.slider.immediateValue]; + else + this.value = this.$.slider.immediateValue; }, /** @@ -55,6 +61,12 @@ * @private */ valueChanged_: function() { + // If |tickValues| is empty, simply set current value to the slider. + if (this.tickValues.length == 0) { + this.$.slider.value = this.value; + return; + } + // First update the slider settings if |tickValues| was set. var numTicks = Math.max(1, this.tickValues.length); this.$.slider.max = numTicks - 1;
diff --git a/ui/wm/BUILD.gn b/ui/wm/BUILD.gn index 5a90c19..1fb7cf1 100644 --- a/ui/wm/BUILD.gn +++ b/ui/wm/BUILD.gn
@@ -114,6 +114,7 @@ sources = [ "core/capture_controller_unittest.cc", "core/compound_event_filter_unittest.cc", + "core/coordinate_conversion_unittest.cc", "core/cursor_manager_unittest.cc", "core/focus_controller_unittest.cc", "core/image_grid_unittest.cc",
diff --git a/ui/wm/core/coordinate_conversion.cc b/ui/wm/core/coordinate_conversion.cc index 1d97f04..17104b35 100644 --- a/ui/wm/core/coordinate_conversion.cc +++ b/ui/wm/core/coordinate_conversion.cc
@@ -6,6 +6,10 @@ #include "ui/aura/client/screen_position_client.h" #include "ui/gfx/geometry/point.h" +#include "ui/gfx/geometry/rect.h" +#include "ui/gfx/geometry/rect_conversions.h" +#include "ui/gfx/geometry/rect_f.h" +#include "ui/gfx/transform.h" namespace wm { @@ -26,4 +30,17 @@ ConvertPointFromScreen(window, point_in_screen); } +void ConvertRectToScreen(const aura::Window* window, gfx::Rect* rect) { + gfx::Point origin = rect->origin(); + ConvertPointToScreen(window, &origin); + rect->set_origin(origin); +} + +void ConvertRectFromScreen(const aura::Window* window, + gfx::Rect* rect_in_screen) { + gfx::Point origin = rect_in_screen->origin(); + ConvertPointFromScreen(window, &origin); + rect_in_screen->set_origin(origin); +} + } // namespace wm
diff --git a/ui/wm/core/coordinate_conversion.h b/ui/wm/core/coordinate_conversion.h index acc6a46..4b987fcb 100644 --- a/ui/wm/core/coordinate_conversion.h +++ b/ui/wm/core/coordinate_conversion.h
@@ -13,6 +13,7 @@ namespace gfx { class Point; +class Rect; } // namespace gfx namespace wm { @@ -27,6 +28,15 @@ WM_EXPORT void ConvertPointFromScreen(const aura::Window* window, gfx::Point* point_in_screen); +// Converts |rect| from |window|'s coordinates to the virtual screen +// coordinates. +WM_EXPORT void ConvertRectToScreen(const aura::Window* window, gfx::Rect* rect); + +// Converts |rect| from virtual screen coordinates to the |window|'s +// coordinates. +WM_EXPORT void ConvertRectFromScreen(const aura::Window* window, + gfx::Rect* rect_in_screen); + } // namespace wm #endif // UI_WM_CORE_COORDINATE_CONVERSION_H_
diff --git a/ui/wm/core/coordinate_conversion_unittest.cc b/ui/wm/core/coordinate_conversion_unittest.cc new file mode 100644 index 0000000..13d077a --- /dev/null +++ b/ui/wm/core/coordinate_conversion_unittest.cc
@@ -0,0 +1,42 @@ +// Copyright 2017 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 "ui/wm/core/coordinate_conversion.h" + +#include "testing/gtest/include/gtest/gtest.h" +#include "ui/aura/client/screen_position_client.h" +#include "ui/aura/test/aura_test_base.h" +#include "ui/aura/test/test_windows.h" +#include "ui/wm/core/default_screen_position_client.h" + +namespace wm { + +typedef aura::test::AuraTestBase CoordinateConversionTest; + +TEST_F(CoordinateConversionTest, ConvertRect) { + DefaultScreenPositionClient screen_position_client; + aura::client::SetScreenPositionClient(root_window(), &screen_position_client); + aura::Window* w = aura::test::CreateTestWindowWithBounds( + gfx::Rect(10, 20, 100, 200), root_window()); + + gfx::Rect r1(10, 20, 100, 120); + ConvertRectFromScreen(w, &r1); + EXPECT_EQ("0,0 100x120", r1.ToString()); + + gfx::Rect r2(0, 0, 100, 200); + ConvertRectFromScreen(w, &r2); + EXPECT_EQ("-10,-20 100x200", r2.ToString()); + + gfx::Rect r3(30, 30, 100, 200); + ConvertRectToScreen(w, &r3); + EXPECT_EQ("40,50 100x200", r3.ToString()); + + gfx::Rect r4(-10, -20, 100, 200); + ConvertRectToScreen(w, &r4); + EXPECT_EQ("0,0 100x200", r4.ToString()); + + aura::client::SetScreenPositionClient(root_window(), nullptr); +} + +} // namespace wm