diff --git a/DEPS b/DEPS index dc2c855..ffe3b05 100644 --- a/DEPS +++ b/DEPS
@@ -40,7 +40,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling Skia # and whatever else without interference from each other. - 'skia_revision': 'e14349a7545e7bd1e93f4c3095db8f481939b053', + 'skia_revision': '7fc4a2d2f0eb57f94f5ef9a0f759a099a4320a54', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other.
diff --git a/base/mac/sdk_forward_declarations.h b/base/mac/sdk_forward_declarations.h index 31ae383f..c636b34 100644 --- a/base/mac/sdk_forward_declarations.h +++ b/base/mac/sdk_forward_declarations.h
@@ -122,6 +122,7 @@ @interface NSLayoutConstraint (YosemiteSDK) @property(getter=isActive) BOOL active; ++ (void)activateConstraints:(NSArray*)constraints; @end @interface NSVisualEffectView (YosemiteSDK) @@ -163,13 +164,19 @@ #if !defined(MAC_OS_X_VERSION_10_11) || \ MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_11 +@class NSLayoutDimension; @class NSLayoutXAxisAnchor; @class NSLayoutYAxisAnchor; +@interface NSObject (ElCapitanSDK) +- (NSLayoutConstraint*)constraintEqualToConstant:(CGFloat)c; +@end + @interface NSView (ElCapitanSDK) @property(readonly, strong) NSLayoutXAxisAnchor* leftAnchor; @property(readonly, strong) NSLayoutXAxisAnchor* rightAnchor; @property(readonly, strong) NSLayoutYAxisAnchor* bottomAnchor; +@property(readonly, strong) NSLayoutDimension* widthAnchor; @end @interface NSWindow (ElCapitanSDK) @@ -196,9 +203,24 @@ @interface NSButton (SierraPointOneSDK) @property(copy) NSColor* bezelColor; +@property BOOL imageHugsTitle; + (instancetype)buttonWithTitle:(NSString*)title target:(id)target action:(SEL)action; ++ (instancetype)buttonWithImage:(NSImage*)image + target:(id)target + action:(SEL)action; ++ (instancetype)buttonWithTitle:(NSString*)title + image:(NSImage*)image + target:(id)target + action:(SEL)action; +@end + +@interface NSSegmentedControl (SierraPointOneSDK) ++ (instancetype)segmentedControlWithImages:(NSArray*)images + trackingMode:(NSSegmentSwitchTracking)trackingMode + target:(id)target + action:(SEL)action; @end #endif // MAC_OS_X_VERSION_10_12_1
diff --git a/build/secondary/third_party/crashpad/crashpad/handler/BUILD.gn b/build/secondary/third_party/crashpad/crashpad/handler/BUILD.gn index 6cd8b9f..d2ce8b9 100644 --- a/build/secondary/third_party/crashpad/crashpad/handler/BUILD.gn +++ b/build/secondary/third_party/crashpad/crashpad/handler/BUILD.gn
@@ -48,11 +48,11 @@ ] if (is_mac && is_component_build) { - # The handler is in Chromium.app/Contents/Versions/X/Chromium Framework.framework/Helpers/ + # The handler is in Chromium.app/Contents/Versions/X/Chromium Framework.framework/Versions/A/Helpers/ # so set rpath up to the base. ldflags = [ "-rpath", - "@loader_path/../../../../../..", + "@loader_path/../../../../../../../..", ] }
diff --git a/cc/output/color_lut_cache.cc b/cc/output/color_lut_cache.cc index aa22d50..4cc7220 100644 --- a/cc/output/color_lut_cache.cc +++ b/cc/output/color_lut_cache.cc
@@ -51,16 +51,8 @@ } // namespace template <typename T> -unsigned int ColorLUTCache::MakeLUT(const gfx::ColorSpace& from, - gfx::ColorSpace to, +unsigned int ColorLUTCache::MakeLUT(const gfx::ColorTransform* transform, int lut_samples) { - if (to == gfx::ColorSpace()) { - to = gfx::ColorSpace::CreateSRGB(); - } - std::unique_ptr<gfx::ColorTransform> transform( - gfx::ColorTransform::NewColorTransform( - from, to, gfx::ColorTransform::Intent::INTENT_PERCEPTUAL)); - int lut_entries = lut_samples * lut_samples * lut_samples; float inverse = 1.0f / (lut_samples - 1); std::vector<T> lut(lut_entries * 4); @@ -103,10 +95,8 @@ return lut_texture; } -ColorLUTCache::LUT ColorLUTCache::GetLUT(const gfx::ColorSpace& from, - const gfx::ColorSpace& to) { - CacheKey key(from, to); - auto iter = lut_cache_.Get(key); +ColorLUTCache::LUT ColorLUTCache::GetLUT(const gfx::ColorTransform* transform) { + auto iter = lut_cache_.Get(transform); if (iter != lut_cache_.end()) { iter->second.last_used_frame = current_frame_; return iter->second.lut; @@ -117,15 +107,15 @@ // to produce values outside of 0-1, so we'll need to make a half-float // LUT. Also, we'll need to build a larger lut to maintain accuracy. // All LUT sizes should be odd a some transforms hav a knee at 0.5. - if (to == gfx::ColorSpace::CreateSCRGBLinear() && from.IsHDR() && - texture_half_float_linear_) { + if (transform->GetDstColorSpace() == gfx::ColorSpace::CreateSCRGBLinear() && + transform->GetSrcColorSpace().IsHDR() && texture_half_float_linear_) { lut.size = 37; - lut.texture = MakeLUT<uint16_t>(from, to, lut.size); + lut.texture = MakeLUT<uint16_t>(transform, lut.size); } else { lut.size = 17; - lut.texture = MakeLUT<unsigned char>(from, to, lut.size); + lut.texture = MakeLUT<unsigned char>(transform, lut.size); } - lut_cache_.Put(key, CacheVal(lut, current_frame_)); + lut_cache_.Put(transform, CacheVal(lut, current_frame_)); return lut; }
diff --git a/cc/output/color_lut_cache.h b/cc/output/color_lut_cache.h index 12a1db5e..b298b6b 100644 --- a/cc/output/color_lut_cache.h +++ b/cc/output/color_lut_cache.h
@@ -11,6 +11,10 @@ #include "base/macros.h" #include "ui/gfx/color_space.h" +namespace gfx { +class ColorTransform; +} + namespace gpu { namespace gles2 { class GLES2Interface; @@ -28,18 +32,16 @@ int size; }; - LUT GetLUT(const gfx::ColorSpace& from, const gfx::ColorSpace& to); + LUT GetLUT(const gfx::ColorTransform* transform); // End of frame, assume all LUTs handed out are no longer used. void Swap(); private: template <typename T> - unsigned int MakeLUT(const gfx::ColorSpace& from, - gfx::ColorSpace to, - int lut_samples); + unsigned int MakeLUT(const gfx::ColorTransform* transform, int lut_samples); - typedef std::pair<gfx::ColorSpace, gfx::ColorSpace> CacheKey; + typedef const gfx::ColorTransform* CacheKey; struct CacheVal { CacheVal(LUT lut, uint32_t last_used_frame)
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc index e03abea9..477a88a 100644 --- a/cc/output/gl_renderer.cc +++ b/cc/output/gl_renderer.cc
@@ -63,6 +63,7 @@ #include "third_party/skia/include/gpu/gl/GrGLInterface.h" #include "third_party/skia/include/gpu/gl/GrGLTypes.h" #include "ui/gfx/color_space.h" +#include "ui/gfx/color_transform.h" #include "ui/gfx/geometry/quad_f.h" #include "ui/gfx/geometry/rect_conversions.h" #include "ui/gfx/skia_util.h" @@ -2029,50 +2030,6 @@ gl_->DrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_SHORT, 0); } -// TODO(ccameron): This has been replicated in ui/gfx/color_transform.cc. Delete -// one of the instances. -void ComputeYUVToRGBMatrices(const gfx::ColorSpace& src_color_space, - const gfx::ColorSpace& dst_color_space, - uint32_t bits_per_channel, - float resource_multiplier, - float resource_offset, - float* yuv_to_rgb_matrix) { - // Compute the matrix |full_transform| which converts input YUV values to RGB - // values. - SkMatrix44 full_transform; - - // Start with the resource adjust. - full_transform.setScale(resource_multiplier, resource_multiplier, - resource_multiplier); - full_transform.preTranslate(-resource_offset, -resource_offset, - -resource_offset); - - // If we're using a LUT for conversion, we only need the resource adjust, - // so just return this matrix. - if (dst_color_space.IsValid()) { - full_transform.asColMajorf(yuv_to_rgb_matrix); - return; - } - - // Then apply the range adjust. - { - SkMatrix44 range_adjust; - src_color_space.GetRangeAdjustMatrix(&range_adjust); - full_transform.postConcat(range_adjust); - } - - // Then apply the YUV to RGB full_transform. - { - SkMatrix44 rgb_to_yuv; - src_color_space.GetTransferMatrix(&rgb_to_yuv); - SkMatrix44 yuv_to_rgb; - rgb_to_yuv.invert(&yuv_to_rgb); - full_transform.postConcat(yuv_to_rgb); - } - - full_transform.asColMajorf(yuv_to_rgb_matrix); -} - void GLRenderer::DrawYUVVideoQuad(const YUVVideoDrawQuad* quad, const gfx::QuadF* clip_region) { SetBlendEnabled(quad->ShouldDrawWithBlending()); @@ -2205,12 +2162,10 @@ if (alpha_texture_mode == YUV_HAS_ALPHA_TEXTURE) gl_->Uniform1i(current_program_->a_texture_location(), 4); - float yuv_to_rgb_matrix[16] = {0}; - ComputeYUVToRGBMatrices(src_color_space, dst_color_space, - quad->bits_per_channel, quad->resource_multiplier, - quad->resource_offset, yuv_to_rgb_matrix); - gl_->UniformMatrix4fv(current_program_->yuv_and_resource_matrix_location(), 1, - 0, yuv_to_rgb_matrix); + gl_->Uniform1f(current_program_->resource_multiplier_location(), + quad->resource_multiplier); + gl_->Uniform1f(current_program_->resource_offset_location(), + quad->resource_offset); // The transform and vertex data are used to figure out the extents that the // un-antialiased quad should have and which vertex this is and the float @@ -3032,8 +2987,9 @@ const gfx::ColorSpace& src_color_space, const gfx::ColorSpace& dst_color_space) { ProgramKey program_key = program_key_no_color; - if (src_color_space.IsValid() && dst_color_space.IsValid()) - program_key.SetColorConversionMode(COLOR_CONVERSION_MODE_LUT); + const gfx::ColorTransform* color_transform = + GetColorTransform(src_color_space, dst_color_space); + program_key.SetColorTransform(color_transform); // Create and set the program if needed. std::unique_ptr<Program>& program = program_cache_[program_key]; @@ -3064,8 +3020,7 @@ gl_->Uniform4fv(current_program_->viewport_location(), 1, viewport); } if (current_program_->lut_texture_location() != -1) { - ColorLUTCache::LUT lut = - color_lut_cache_.GetLUT(src_color_space, dst_color_space); + ColorLUTCache::LUT lut = color_lut_cache_.GetLUT(color_transform); gl_->ActiveTexture(GL_TEXTURE5); gl_->BindTexture(GL_TEXTURE_2D, lut.texture); gl_->Uniform1i(current_program_->lut_texture_location(), 5); @@ -3082,12 +3037,25 @@ return found->second.get(); } +const gfx::ColorTransform* GLRenderer::GetColorTransform( + const gfx::ColorSpace& src, + const gfx::ColorSpace& dst) { + std::unique_ptr<gfx::ColorTransform>& transform = + color_transform_cache_[dst][src]; + if (!transform) { + transform = gfx::ColorTransform::NewColorTransform( + src, dst, gfx::ColorTransform::Intent::INTENT_PERCEPTUAL); + } + return transform.get(); +} + void GLRenderer::CleanupSharedObjects() { shared_geometry_ = nullptr; for (auto& iter : program_cache_) iter.second->Cleanup(gl_); program_cache_.clear(); + color_transform_cache_.clear(); if (offscreen_framebuffer_id_) gl_->DeleteFramebuffers(1, &offscreen_framebuffer_id_);
diff --git a/cc/output/gl_renderer.h b/cc/output/gl_renderer.h index ea73b6f8..2a0ce1c 100644 --- a/cc/output/gl_renderer.h +++ b/cc/output/gl_renderer.h
@@ -298,6 +298,12 @@ std::unordered_map<ProgramKey, std::unique_ptr<Program>, ProgramKeyHash> program_cache_; + const gfx::ColorTransform* GetColorTransform(const gfx::ColorSpace& src, + const gfx::ColorSpace& dst); + std::map<gfx::ColorSpace, + std::map<gfx::ColorSpace, std::unique_ptr<gfx::ColorTransform>>> + color_transform_cache_; + gpu::gles2::GLES2Interface* gl_; gpu::ContextSupport* context_support_; std::unique_ptr<ContextCacheController::ScopedVisibility> context_visibility_;
diff --git a/cc/output/program_binding.cc b/cc/output/program_binding.cc index dc8fe84..a8f4b6a 100644 --- a/cc/output/program_binding.cc +++ b/cc/output/program_binding.cc
@@ -8,6 +8,7 @@ #include "cc/output/geometry_binding.h" #include "gpu/GLES2/gl2extchromium.h" #include "gpu/command_buffer/client/gles2_interface.h" +#include "ui/gfx/color_transform.h" using gpu::gles2::GLES2Interface; @@ -29,7 +30,8 @@ has_color_matrix_ == other.has_color_matrix_ && yuv_alpha_texture_mode_ == other.yuv_alpha_texture_mode_ && uv_texture_mode_ == other.uv_texture_mode_ && - color_conversion_mode_ == other.color_conversion_mode_; + color_conversion_mode_ == other.color_conversion_mode_ && + color_transform_ == other.color_transform_; } bool ProgramKey::operator!=(const ProgramKey& other) const { @@ -128,6 +130,18 @@ return result; } +void ProgramKey::SetColorTransform(const gfx::ColorTransform* transform) { + color_transform_ = nullptr; + if (transform->IsIdentity()) { + color_conversion_mode_ = COLOR_CONVERSION_MODE_NONE; + } else if (transform->CanGetShaderSource()) { + color_conversion_mode_ = COLOR_CONVERSION_MODE_SHADER; + color_transform_ = transform; + } else { + color_conversion_mode_ = COLOR_CONVERSION_MODE_LUT; + } +} + ProgramBindingBase::ProgramBindingBase() : program_(0), vertex_shader_id_(0),
diff --git a/cc/output/program_binding.h b/cc/output/program_binding.h index 3b9a382b..6f1bc91 100644 --- a/cc/output/program_binding.h +++ b/cc/output/program_binding.h
@@ -12,6 +12,10 @@ #include "cc/output/context_provider.h" #include "cc/output/shader.h" +namespace gfx { +class ColorTransform; +} + namespace gpu { namespace gles2 { class GLES2Interface; @@ -99,9 +103,7 @@ bool operator==(const ProgramKey& other) const; bool operator!=(const ProgramKey& other) const; - void SetColorConversionMode(ColorConversionMode color_conversion_mode) { - color_conversion_mode_ = color_conversion_mode; - } + void SetColorTransform(const gfx::ColorTransform* transform); private: friend struct ProgramKeyHash; @@ -126,6 +128,7 @@ UVTextureMode uv_texture_mode_ = UV_TEXTURE_MODE_NA; ColorConversionMode color_conversion_mode_ = COLOR_CONVERSION_MODE_NONE; + const gfx::ColorTransform* color_transform_ = nullptr; }; struct ProgramKeyHash { @@ -164,6 +167,7 @@ fragment_shader_.mask_mode_ = key.mask_mode_; fragment_shader_.mask_for_background_ = key.mask_for_background_; fragment_shader_.color_conversion_mode_ = key.color_conversion_mode_; + fragment_shader_.color_transform_ = key.color_transform_; switch (key.type_) { case PROGRAM_TYPE_DEBUG_BORDER: @@ -271,8 +275,11 @@ return fragment_shader_.lut_texture_location_; } int lut_size_location() const { return fragment_shader_.lut_size_location_; } - int yuv_and_resource_matrix_location() const { - return fragment_shader_.yuv_and_resource_matrix_location_; + int resource_multiplier_location() const { + return fragment_shader_.resource_multiplier_location_; + } + int resource_offset_location() const { + return fragment_shader_.resource_offset_location_; } int ya_clamp_rect_location() const { return fragment_shader_.ya_clamp_rect_location_;
diff --git a/cc/output/shader.cc b/cc/output/shader.cc index 2972183..7b46db25 100644 --- a/cc/output/shader.cc +++ b/cc/output/shader.cc
@@ -13,6 +13,7 @@ #include "base/strings/stringprintf.h" #include "cc/output/static_geometry_binding.h" #include "gpu/command_buffer/client/gles2_interface.h" +#include "ui/gfx/color_transform.h" #include "ui/gfx/geometry/point.h" #include "ui/gfx/geometry/size.h" @@ -439,7 +440,8 @@ uniforms.push_back("a_texture"); uniforms.push_back("ya_clamp_rect"); uniforms.push_back("uv_clamp_rect"); - uniforms.push_back("yuv_and_resource_matrix"); + uniforms.push_back("resource_multiplier"); + uniforms.push_back("resource_offset"); break; case INPUT_COLOR_SOURCE_UNIFORM: uniforms.push_back("color"); @@ -492,7 +494,8 @@ a_texture_location_ = locations[index++]; ya_clamp_rect_location_ = locations[index++]; uv_clamp_rect_location_ = locations[index++]; - yuv_and_resource_matrix_location_ = locations[index++]; + resource_multiplier_location_ = locations[index++]; + resource_offset_location_ = locations[index++]; break; case INPUT_COLOR_SOURCE_UNIFORM: color_location_ = locations[index++]; @@ -862,10 +865,12 @@ HDR("uniform SamplerType a_texture;"); HDR("uniform vec4 ya_clamp_rect;"); HDR("uniform vec4 uv_clamp_rect;"); - HDR("uniform mat4 yuv_and_resource_matrix;"); + HDR("uniform float resource_multiplier;"); + HDR("uniform float resource_offset;"); HDR("varying TexCoordPrecision vec2 v_yaTexCoord;"); HDR("varying TexCoordPrecision vec2 v_uvTexCoord;"); - SRC("texColor = yuv_and_resource_matrix * texColor;"); + SRC("texColor.xyz -= vec3(resource_offset);"); + SRC("texColor.xyz *= resource_multiplier;"); break; case INPUT_COLOR_SOURCE_UNIFORM: DCHECK(!ignore_sampler_type_); @@ -877,22 +882,30 @@ } // Apply LUT based color conversion. - if (color_conversion_mode_ == COLOR_CONVERSION_MODE_LUT) { - HDR("uniform sampler2D lut_texture;"); - HDR("uniform float lut_size;"); - HDR("vec4 LUT(sampler2D sampler, vec3 pos, float size) {"); - HDR(" pos *= size - 1.0;"); - HDR(" // Select layer"); - HDR(" float layer = min(floor(pos.z), size - 2.0);"); - HDR(" // Compress the xy coordinates so they stay within"); - HDR(" // [0.5 .. 31.5] / N (assuming a LUT size of 17^3)"); - HDR(" pos.xy = (pos.xy + vec2(0.5)) / size;"); - HDR(" pos.y = (pos.y + layer) / size;"); - HDR(" return mix(LutLookup(sampler, pos.xy),"); - HDR(" LutLookup(sampler, pos.xy + vec2(0, 1.0 / size)),"); - HDR(" pos.z - layer);"); - HDR("}"); - SRC("texColor.xyz = LUT(lut_texture, texColor.xyz, lut_size).xyz;"); + switch (color_conversion_mode_) { + case COLOR_CONVERSION_MODE_LUT: + HDR("uniform sampler2D lut_texture;"); + HDR("uniform float lut_size;"); + HDR("vec4 LUT(sampler2D sampler, vec3 pos, float size) {"); + HDR(" pos *= size - 1.0;"); + HDR(" // Select layer"); + HDR(" float layer = min(floor(pos.z), size - 2.0);"); + HDR(" // Compress the xy coordinates so they stay within"); + HDR(" // [0.5 .. 31.5] / N (assuming a LUT size of 17^3)"); + HDR(" pos.xy = (pos.xy + vec2(0.5)) / size;"); + HDR(" pos.y = (pos.y + layer) / size;"); + HDR(" return mix(LutLookup(sampler, pos.xy),"); + HDR(" LutLookup(sampler, pos.xy + vec2(0, 1.0 / size)),"); + HDR(" pos.z - layer);"); + HDR("}"); + SRC("texColor.rgb = LUT(lut_texture, texColor.xyz, lut_size).xyz;"); + break; + case COLOR_CONVERSION_MODE_SHADER: + header += color_transform_->GetShaderSource(); + SRC("texColor.rgb = DoColorConversion(texColor.xyz);"); + break; + case COLOR_CONVERSION_MODE_NONE: + break; } // Apply the color matrix to texColor.
diff --git a/cc/output/shader.h b/cc/output/shader.h index 0d3899a..0f65a595 100644 --- a/cc/output/shader.h +++ b/cc/output/shader.h
@@ -12,6 +12,7 @@ #include "cc/base/cc_export.h" namespace gfx { +class ColorTransform; class Point; class Size; } @@ -137,6 +138,8 @@ // applicable) to output RGB space, via a 3D texture represented as a 2D // texture. COLOR_CONVERSION_MODE_LUT, + // Conversion is done analytically in the shader. + COLOR_CONVERSION_MODE_SHADER, }; // TODO(ccameron): Merge this with BlendMode. @@ -285,6 +288,7 @@ UVTextureMode uv_texture_mode_ = UV_TEXTURE_MODE_UV; ColorConversionMode color_conversion_mode_ = COLOR_CONVERSION_MODE_NONE; + const gfx::ColorTransform* color_transform_ = nullptr; // YUV uniform locations. int y_texture_location_ = -1; @@ -295,10 +299,9 @@ int ya_clamp_rect_location_ = -1; int uv_clamp_rect_location_ = -1; - // This matrix will convert from the values read in the YUV texture to - // RGB (including resource offset and scale). If we are using LUT based - // color conversion, then this will only perform resource offset and scale. - int yuv_and_resource_matrix_location_ = -1; + // The resource offset and multiplier to adjust for bit depth. + int resource_multiplier_location_ = -1; + int resource_offset_location_ = -1; // LUT YUV to color-converted RGB. int lut_texture_location_ = -1;
diff --git a/chrome/app/chromeos_strings.grdp b/chrome/app/chromeos_strings.grdp index f12a5de0..67547e5 100644 --- a/chrome/app/chromeos_strings.grdp +++ b/chrome/app/chromeos_strings.grdp
@@ -1788,6 +1788,9 @@ <message name="IDS_OOBE_EULA_SECTION_TITLE" desc="Title of Terms of Service screen"> Google Chrome OS terms </message> + <message name="IDS_OOBE_EULA_IFRAME_LABEL" desc="Accessibility label on an iframe with full Chrome OS Terms text"> + Google Chrome OS Terms contents + </message> <message name="IDS_OOBE_EULA_ACCEPT_AND_CONTINUE_BUTTON_TEXT" desc="Label on a button on the Title of Terms of Service OOBE screen to accept EULA and continue."> Accept and continue </message>
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 850b9ede..89bde79 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd
@@ -1927,6 +1927,15 @@ </if> </if> + <!-- Touch Bar--> + <if expr="is_macosx"> + <message name="IDS_TOUCH_BAR_SEARCH" + desc="Text for the search button in the touch bar. Pressing the button will bring focus to the omnibox."> + Search or type URL + </message> + + </if> + <!-- Remove in-progress downloads confirmation dialog --> <if expr="not is_macosx"> <message name="IDS_DOWNLOAD_REMOVE_CONFIRM_TITLE" desc="Title of the dialog asking for user confirmation to close the browser or the last incognito window when one or more downloads are in progress. If necessary, add '#' for plural cases. [ICU Syntax]">
diff --git a/chrome/app/vector_icons/BUILD.gn b/chrome/app/vector_icons/BUILD.gn index 5c70490e..1311436 100644 --- a/chrome/app/vector_icons/BUILD.gn +++ b/chrome/app/vector_icons/BUILD.gn
@@ -63,6 +63,10 @@ "zoom_plus.icon", "${branding_path_component}/product.icon", ] + + if (is_mac) { + icons += [ "new_tab_mac_touchbar.icon" ] + } } source_set("vector_icons") {
diff --git a/chrome/app/vector_icons/new_tab_mac_touchbar.icon b/chrome/app/vector_icons/new_tab_mac_touchbar.icon new file mode 100644 index 0000000..51bc0f4 --- /dev/null +++ b/chrome/app/vector_icons/new_tab_mac_touchbar.icon
@@ -0,0 +1,26 @@ +// 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. + +CANVAS_DIMENSIONS, 36, +MOVE_TO, 18, 0, +R_CUBIC_TO, -9.94f, 0, -18, 8.06f, -18, 18, +R_CUBIC_TO, 0, 9.94f, 8.06f, 18, 18, 18, +R_CUBIC_TO, 9.94f, 0, 18, -8.06f, 18, -18, +R_CUBIC_TO, 0, -9.94f, -8.06f, -18, -18, -18, +CLOSE, +R_MOVE_TO, 9, 19.8f, +R_H_LINE_TO, -7.2f, +V_LINE_TO, 27, +R_H_LINE_TO, -3.6f, +R_V_LINE_TO, -7.2f, +H_LINE_TO, 9, +R_V_LINE_TO, -3.6f, +R_H_LINE_TO, 7.2f, +V_LINE_TO, 9, +R_H_LINE_TO, 3.6f, +R_V_LINE_TO, 7.2f, +H_LINE_TO, 27, +R_V_LINE_TO, 3.6f, +CLOSE, +END \ No newline at end of file
diff --git a/chrome/browser/chromeos/arc/arc_session_manager.cc b/chrome/browser/chromeos/arc/arc_session_manager.cc index 6ae5cb9..7eab36a 100644 --- a/chrome/browser/chromeos/arc/arc_session_manager.cc +++ b/chrome/browser/chromeos/arc/arc_session_manager.cc
@@ -118,8 +118,12 @@ registry->RegisterBooleanPref(prefs::kArcEnabled, false); registry->RegisterBooleanPref(prefs::kArcSignedIn, false); registry->RegisterBooleanPref(prefs::kArcTermsAccepted, false); - registry->RegisterBooleanPref(prefs::kArcBackupRestoreEnabled, true); - registry->RegisterBooleanPref(prefs::kArcLocationServiceEnabled, true); + // Note that ArcBackupRestoreEnabled and ArcLocationServiceEnabled prefs have + // to be off by default, until an explicit gesture from the user to enable + // them is received. This is crucial in the cases when these prefs transition + // from a previous managed state to the unmanaged. + registry->RegisterBooleanPref(prefs::kArcBackupRestoreEnabled, false); + registry->RegisterBooleanPref(prefs::kArcLocationServiceEnabled, false); } // static @@ -706,18 +710,30 @@ return; } + PrefService* const prefs = profile_->GetPrefs(); + // For ARC Kiosk we skip ToS because it is very likely that near the device // there will be no one who is eligible to accept them. // TODO(poromov): Move to more Kiosk dedicated set-up phase. if (IsArcKioskMode()) - profile_->GetPrefs()->SetBoolean(prefs::kArcTermsAccepted, true); + prefs->SetBoolean(prefs::kArcTermsAccepted, true); + + // Skip to show UI asking users to enable/disable their preference for + // backup-restore and location-service, if both are managed by the admin + // policy. Note that the ToS agreement is anyway not shown in the case of the + // managed ARC. + if (IsArcManaged() && + prefs->IsManagedPreference(prefs::kArcBackupRestoreEnabled) && + prefs->IsManagedPreference(prefs::kArcLocationServiceEnabled)) { + prefs->SetBoolean(prefs::kArcTermsAccepted, true); + } // If it is marked that sign in has been successfully done, then directly // start ARC. - // For testing, and for Kisok mode, we also skip ToS negotiation procedure. + // For testing, and for Kiosk mode, we also skip ToS negotiation procedure. // For backward compatibility, this check needs to be prior to the // kArcTermsAccepted check below. - if (profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn) || + if (prefs->GetBoolean(prefs::kArcSignedIn) || IsArcOptInVerificationDisabled() || IsArcKioskMode()) { StartArc(); @@ -751,10 +767,12 @@ // 1) User accepted the Terms of service on OOBE flow. // 2) User accepted the Terms of service on Opt-in flow, but logged out // before ARC sign in procedure was done. Then, logs in again. - if (profile_->GetPrefs()->GetBoolean(prefs::kArcTermsAccepted)) { + if (prefs->GetBoolean(prefs::kArcTermsAccepted)) { // Don't show UI for this progress if it was not shown. - if (support_host_->ui_page() != ArcSupportHost::UIPage::NO_PAGE) + if (support_host_ && + support_host_->ui_page() != ArcSupportHost::UIPage::NO_PAGE) { support_host_->ShowArcLoading(); + } StartArcAndroidManagementCheck(); return; }
diff --git a/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc b/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc index a2ae503..98d8e147 100644 --- a/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc +++ b/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc
@@ -4,6 +4,7 @@ #include <memory> #include <string> +#include <tuple> #include <vector> #include "base/bind.h" @@ -11,10 +12,12 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" +#include "base/logging.h" #include "base/macros.h" #include "base/memory/ptr_util.h" #include "base/observer_list.h" #include "base/run_loop.h" +#include "base/values.h" #include "chrome/browser/chromeos/arc/arc_optin_uma.h" #include "chrome/browser/chromeos/arc/arc_session_manager.h" #include "chrome/browser/chromeos/arc/optin/arc_terms_of_service_oobe_negotiator.h" @@ -42,6 +45,7 @@ #include "components/arc/arc_util.h" #include "components/arc/test/fake_arc_session.h" #include "components/prefs/pref_service.h" +#include "components/prefs/testing_pref_service.h" #include "components/signin/core/account_id/account_id.h" #include "components/sync/model/fake_sync_change_processor.h" #include "components/sync/model/sync_error_factory_mock.h" @@ -567,6 +571,75 @@ arc_session_manager()->Shutdown(); } +class ArcSessionManagerPolicyTest + : public ArcSessionManagerTest, + public testing::WithParamInterface<std::tuple<base::Value, base::Value>> { + public: + const base::Value& backup_restore_pref_value() const { + return std::get<0>(GetParam()); + } + + const base::Value& location_service_pref_value() const { + return std::get<1>(GetParam()); + } +}; + +TEST_P(ArcSessionManagerPolicyTest, SkippingTerms) { + sync_preferences::TestingPrefServiceSyncable* const prefs = + profile()->GetTestingPrefService(); + + // Backup-restore and location-service prefs are off by default. + EXPECT_FALSE(prefs->GetBoolean(prefs::kArcSignedIn)); + EXPECT_FALSE(prefs->GetBoolean(prefs::kArcTermsAccepted)); + + // Set ARC to be managed. + prefs->SetManagedPref(prefs::kArcEnabled, new base::Value(true)); + + // Assign test values to the prefs. + if (backup_restore_pref_value().is_bool()) { + prefs->SetManagedPref(prefs::kArcBackupRestoreEnabled, + backup_restore_pref_value().DeepCopy()); + } + if (location_service_pref_value().is_bool()) { + prefs->SetManagedPref(prefs::kArcLocationServiceEnabled, + location_service_pref_value().DeepCopy()); + } + + arc_session_manager()->OnPrimaryUserProfilePrepared(profile()); + EXPECT_TRUE(arc_session_manager()->IsArcPlayStoreEnabled()); + EXPECT_TRUE(arc_session_manager()->IsArcManaged()); + + // Terms of Service should be skipped if both ArcBackupRestoreEnabled and + // ArcLocationServiceEnabled are managed. + const ArcSessionManager::State expected_state = + backup_restore_pref_value().is_bool() && + location_service_pref_value().is_bool() + ? ArcSessionManager::State::ACTIVE + : ArcSessionManager::State::SHOWING_TERMS_OF_SERVICE; + EXPECT_EQ(expected_state, arc_session_manager()->state()); + + // Managed values for the prefs are unset. + prefs->RemoveManagedPref(prefs::kArcBackupRestoreEnabled); + prefs->RemoveManagedPref(prefs::kArcLocationServiceEnabled); + + // The ARC state is preserved. The prefs return to the default false values. + EXPECT_EQ(expected_state, arc_session_manager()->state()); + EXPECT_FALSE(prefs->GetBoolean(prefs::kArcBackupRestoreEnabled)); + EXPECT_FALSE(prefs->GetBoolean(prefs::kArcLocationServiceEnabled)); + + // Stop ARC and shutdown the service. + prefs->RemoveManagedPref(prefs::kArcEnabled); + WaitForDataRemoved(ArcSessionManager::State::STOPPED); + arc_session_manager()->Shutdown(); +} + +INSTANTIATE_TEST_CASE_P( + ArcSessionManagerPolicyTest, + ArcSessionManagerPolicyTest, + testing::Combine( + testing::Values(base::Value(), base::Value(false), base::Value(true)), + testing::Values(base::Value(), base::Value(false), base::Value(true)))); + class ArcSessionManagerKioskTest : public ArcSessionManagerTestBase { public: ArcSessionManagerKioskTest() = default;
diff --git a/chrome/browser/chromeos/arc/extensions/fake_arc_support.h b/chrome/browser/chromeos/arc/extensions/fake_arc_support.h index a0af1b5..f34b0ee0 100644 --- a/chrome/browser/chromeos/arc/extensions/fake_arc_support.h +++ b/chrome/browser/chromeos/arc/extensions/fake_arc_support.h
@@ -32,6 +32,10 @@ // Emulates clicking Agree button. void ClickAgreeButton(); + bool metrics_mode() const { return metrics_mode_; } + bool backup_and_restore_mode() const { return backup_and_restore_mode_; } + bool location_service_mode() const { return location_service_mode_; } + // Emulates checking preference box. void set_metrics_mode(bool mode) { metrics_mode_ = mode; } void set_backup_and_restore_mode(bool mode) {
diff --git a/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc b/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc index b52c9ca..008a1c4 100644 --- a/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc +++ b/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc
@@ -65,12 +65,15 @@ return !host->empty() && *port; } +PrefService* GetPrefs() { + return ProfileManager::GetActiveUserProfile()->GetPrefs(); +} + // Returns whether kProxy pref proxy config is applied. bool IsPrefProxyConfigApplied() { net::ProxyConfig config; - Profile* profile = ProfileManager::GetActiveUserProfile(); return PrefProxyConfigTrackerImpl::PrefPrecedes( - PrefProxyConfigTrackerImpl::ReadPrefConfig(profile->GetPrefs(), &config)); + PrefProxyConfigTrackerImpl::ReadPrefConfig(GetPrefs(), &config)); } } // namespace @@ -112,18 +115,23 @@ // Stops listening for Chrome settings changes. void StopObservingSettingsChanges(); + // Retrieves Chrome's state for the settings that need to be synced on the + // initial Android boot and send it to Android. + void SyncInitialSettings() const; // Retrieves Chrome's state for the settings that need to be synced on each // Android boot and send it to Android. void SyncRuntimeSettings() const; - // Send settings that need to be synced only on Android first start to - // Android. + // Determine whether a particular setting needs to be synced to Android. + // Keep these lines ordered lexicographically. + bool ShouldSyncBackupEnabled() const; + bool ShouldSyncLocationServiceEnabled() const; + // Send particular settings to Android. // Keep these lines ordered lexicographically. void SyncAccessibilityLargeMouseCursorEnabled() const; void SyncAccessibilityVirtualKeyboardEnabled() const; void SyncBackupEnabled() const; void SyncFocusHighlightEnabled() const; void SyncFontSize() const; - void SyncInitialSettings() const; void SyncLocale() const; void SyncLocationServiceEnabled() const; void SyncProxySettings() const; @@ -204,10 +212,11 @@ SyncSpokenFeedbackEnabled(); } else if (pref_name == prefs::kAccessibilityVirtualKeyboardEnabled) { SyncAccessibilityVirtualKeyboardEnabled(); + } else if (pref_name == prefs::kArcBackupRestoreEnabled) { + if (ShouldSyncBackupEnabled()) + SyncBackupEnabled(); } else if (pref_name == prefs::kArcLocationServiceEnabled) { - const PrefService* const prefs = - ProfileManager::GetActiveUserProfile()->GetPrefs(); - if (prefs->IsManagedPreference(prefs::kArcLocationServiceEnabled)) + if (ShouldSyncLocationServiceEnabled()) SyncLocationServiceEnabled(); } else if (pref_name == prefs::kUse24HourClock) { SyncUse24HourClock(); @@ -249,8 +258,7 @@ } void ArcSettingsServiceImpl::StartObservingSettingsChanges() { - Profile* profile = ProfileManager::GetActiveUserProfile(); - registrar_.Init(profile->GetPrefs()); + registrar_.Init(GetPrefs()); // Keep these lines ordered lexicographically. AddPrefToObserve(prefs::kAccessibilityFocusHighlightEnabled); @@ -293,6 +301,12 @@ this, FROM_HERE); } +void ArcSettingsServiceImpl::SyncInitialSettings() const { + // Keep these lines ordered lexicographically. + SyncBackupEnabled(); + SyncLocationServiceEnabled(); +} + void ArcSettingsServiceImpl::SyncRuntimeSettings() const { // Keep these lines ordered lexicographically. SyncAccessibilityLargeMouseCursorEnabled(); @@ -306,14 +320,28 @@ SyncTimeZone(); SyncUse24HourClock(); - const PrefService* const prefs = - ProfileManager::GetActiveUserProfile()->GetPrefs(); - if (prefs->IsManagedPreference(prefs::kArcBackupRestoreEnabled)) + if (ShouldSyncBackupEnabled()) SyncBackupEnabled(); - if (prefs->IsManagedPreference(prefs::kArcLocationServiceEnabled)) + if (ShouldSyncLocationServiceEnabled()) SyncLocationServiceEnabled(); } +bool ArcSettingsServiceImpl::ShouldSyncBackupEnabled() const { + // Always sync the managed setting. Also sync when the pref is unset, which + // normally happens once after the pref changes from the managed state to + // unmanaged. + return GetPrefs()->IsManagedPreference(prefs::kArcBackupRestoreEnabled) || + !GetPrefs()->HasPrefPath(prefs::kArcBackupRestoreEnabled); +} + +bool ArcSettingsServiceImpl::ShouldSyncLocationServiceEnabled() const { + // Always sync the managed setting. Also sync when the pref is unset, which + // normally happens once after the pref changes from the managed state to + // unmanaged. + return GetPrefs()->IsManagedPreference(prefs::kArcLocationServiceEnabled) || + !GetPrefs()->HasPrefPath(prefs::kArcLocationServiceEnabled); +} + void ArcSettingsServiceImpl::SyncAccessibilityLargeMouseCursorEnabled() const { SendBoolPrefSettingsBroadcast( prefs::kAccessibilityLargeCursorEnabled, @@ -330,6 +358,15 @@ SendBoolPrefSettingsBroadcast( prefs::kArcBackupRestoreEnabled, "org.chromium.arc.intent_helper.SET_BACKUP_ENABLED"); + if (GetPrefs()->IsManagedPreference(prefs::kArcBackupRestoreEnabled)) { + // Unset the user pref so that if the pref becomes unmanaged at some point, + // this change will be synced. + GetPrefs()->ClearPref(prefs::kArcBackupRestoreEnabled); + } else if (!GetPrefs()->HasPrefPath(prefs::kArcBackupRestoreEnabled)) { + // Set the pref value in order to prevent the subsequent syncing. The + // "false" value is a safe default from the legal/privacy perspective. + GetPrefs()->SetBoolean(prefs::kArcBackupRestoreEnabled, false); + } } void ArcSettingsServiceImpl::SyncFocusHighlightEnabled() const { @@ -352,11 +389,6 @@ extras); } -void ArcSettingsServiceImpl::SyncInitialSettings() const { - SyncBackupEnabled(); - SyncLocationServiceEnabled(); -} - void ArcSettingsServiceImpl::SyncLocale() const { const PrefService::Preference* pref = registrar_.prefs()->FindPreference(prefs::kApplicationLocale); @@ -373,13 +405,21 @@ SendBoolPrefSettingsBroadcast( prefs::kArcLocationServiceEnabled, "org.chromium.arc.intent_helper.SET_LOCATION_SERVICE_ENABLED"); + if (GetPrefs()->IsManagedPreference(prefs::kArcLocationServiceEnabled)) { + // Unset the user pref so that if the pref becomes unmanaged at some point, + // this change will be synced. + GetPrefs()->ClearPref(prefs::kArcLocationServiceEnabled); + } else if (!GetPrefs()->HasPrefPath(prefs::kArcLocationServiceEnabled)) { + // Set the pref value in order to prevent the subsequent syncing. The + // "false" value is a safe default from the legal/privacy perspective. + GetPrefs()->SetBoolean(prefs::kArcLocationServiceEnabled, false); + } } void ArcSettingsServiceImpl::SyncProxySettings() const { std::unique_ptr<ProxyConfigDictionary> proxy_config_dict = chromeos::ProxyConfigServiceImpl::GetActiveProxyConfigDictionary( - ProfileManager::GetActiveUserProfile()->GetPrefs(), - g_browser_process->local_state()); + GetPrefs(), g_browser_process->local_state()); if (!proxy_config_dict) return;
diff --git a/chrome/browser/chromeos/arc/intent_helper/arc_settings_service_browsertest.cc b/chrome/browser/chromeos/arc/intent_helper/arc_settings_service_browsertest.cc index bdc2589..b6af8900 100644 --- a/chrome/browser/chromeos/arc/intent_helper/arc_settings_service_browsertest.cc +++ b/chrome/browser/chromeos/arc/intent_helper/arc_settings_service_browsertest.cc
@@ -15,6 +15,7 @@ #include "chrome/browser/chromeos/arc/intent_helper/arc_settings_service.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" +#include "chrome/common/pref_names.h" #include "chrome/test/base/in_process_browser_test.h" #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/shill_profile_client.h" @@ -174,6 +175,8 @@ constexpr char kONCPacUrl[] = "http://domain.com/x"; +constexpr char kBackupBroadcastAction[] = + "org.chromium.arc.intent_helper.SET_BACKUP_ENABLED"; constexpr char kLocationServiceBroadcastAction[] = "org.chromium.arc.intent_helper.SET_LOCATION_SERVICE_ENABLED"; constexpr char kSetProxyBroadcastAction[] = @@ -317,7 +320,76 @@ DISALLOW_COPY_AND_ASSIGN(ArcSettingsServiceTest); }; +IN_PROC_BROWSER_TEST_F(ArcSettingsServiceTest, BackupRestorePolicyTest) { + PrefService* const prefs = browser()->profile()->GetPrefs(); + + // Set the user pref as initially enabled. + prefs->SetBoolean(prefs::kArcBackupRestoreEnabled, true); + EXPECT_TRUE(prefs->GetBoolean(prefs::kArcBackupRestoreEnabled)); + + fake_intent_helper_instance_->clear_broadcasts(); + + // The policy is set to false. + policy::PolicyMap policy; + policy.Set(policy::key::kArcBackupRestoreEnabled, + policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER, + policy::POLICY_SOURCE_CLOUD, base::MakeUnique<base::Value>(false), + nullptr); + UpdatePolicy(policy); + + // The pref is disabled and managed, and the corresponding broadcast is sent + // at least once. + EXPECT_FALSE(prefs->GetBoolean(prefs::kArcBackupRestoreEnabled)); + EXPECT_TRUE(prefs->IsManagedPreference(prefs::kArcBackupRestoreEnabled)); + base::DictionaryValue expected_broadcast_extras; + expected_broadcast_extras.SetBoolean("enabled", false); + expected_broadcast_extras.SetBoolean("managed", true); + EXPECT_GE(CountBroadcasts(fake_intent_helper_instance_->broadcasts(), + kBackupBroadcastAction, &expected_broadcast_extras), + 1); + + fake_intent_helper_instance_->clear_broadcasts(); + + // The policy is set to true. + policy.Set(policy::key::kArcBackupRestoreEnabled, + policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER, + policy::POLICY_SOURCE_CLOUD, base::MakeUnique<base::Value>(true), + nullptr); + UpdatePolicy(policy); + + // The pref is enabled and managed, and the corresponding broadcast is sent at + // least once. + EXPECT_TRUE(prefs->GetBoolean(prefs::kArcBackupRestoreEnabled)); + EXPECT_TRUE(prefs->IsManagedPreference(prefs::kArcBackupRestoreEnabled)); + expected_broadcast_extras.SetBoolean("enabled", true); + EXPECT_GE(CountBroadcasts(fake_intent_helper_instance_->broadcasts(), + kBackupBroadcastAction, &expected_broadcast_extras), + 1); + + fake_intent_helper_instance_->clear_broadcasts(); + + // The policy is unset. + policy.Erase(policy::key::kArcBackupRestoreEnabled); + UpdatePolicy(policy); + + // The pref is disabled and unmanaged, and the corresponding broadcast is + // sent. + EXPECT_FALSE(prefs->GetBoolean(prefs::kArcBackupRestoreEnabled)); + EXPECT_FALSE(prefs->IsManagedPreference(prefs::kArcBackupRestoreEnabled)); + expected_broadcast_extras.SetBoolean("enabled", false); + expected_broadcast_extras.SetBoolean("managed", false); + EXPECT_EQ(CountBroadcasts(fake_intent_helper_instance_->broadcasts(), + kBackupBroadcastAction, &expected_broadcast_extras), + 1); +} + IN_PROC_BROWSER_TEST_F(ArcSettingsServiceTest, LocationServicePolicyTest) { + PrefService* const prefs = browser()->profile()->GetPrefs(); + + // Set the user pref as initially enabled. + prefs->SetBoolean(prefs::kArcLocationServiceEnabled, true); + EXPECT_TRUE(prefs->GetBoolean(prefs::kArcLocationServiceEnabled)); + fake_intent_helper_instance_->clear_broadcasts(); // The policy is set to false. @@ -328,11 +400,14 @@ nullptr); UpdatePolicy(policy); - // The broadcast is sent which says that the pref is disabled and managed. + // The pref is disabled and managed, and the corresponding broadcast is sent + // at least once. + EXPECT_FALSE(prefs->GetBoolean(prefs::kArcLocationServiceEnabled)); + EXPECT_TRUE(prefs->IsManagedPreference(prefs::kArcLocationServiceEnabled)); base::DictionaryValue expected_broadcast_extras; expected_broadcast_extras.SetBoolean("enabled", false); expected_broadcast_extras.SetBoolean("managed", true); - EXPECT_EQ(CountBroadcasts(fake_intent_helper_instance_->broadcasts(), + EXPECT_GE(CountBroadcasts(fake_intent_helper_instance_->broadcasts(), kLocationServiceBroadcastAction, &expected_broadcast_extras), 1); @@ -346,8 +421,28 @@ nullptr); UpdatePolicy(policy); - // The broadcast is sent which says that the pref is enabled and managed. + // The pref is enabled and managed, and the corresponding broadcast is sent at + // least once. + EXPECT_TRUE(prefs->GetBoolean(prefs::kArcLocationServiceEnabled)); + EXPECT_TRUE(prefs->IsManagedPreference(prefs::kArcLocationServiceEnabled)); expected_broadcast_extras.SetBoolean("enabled", true); + EXPECT_GE(CountBroadcasts(fake_intent_helper_instance_->broadcasts(), + kLocationServiceBroadcastAction, + &expected_broadcast_extras), + 1); + + fake_intent_helper_instance_->clear_broadcasts(); + + // The policy is unset. + policy.Erase(policy::key::kArcLocationServiceEnabled); + UpdatePolicy(policy); + + // The pref is disabled and unmanaged, and the corresponding broadcast is + // sent. + EXPECT_FALSE(prefs->GetBoolean(prefs::kArcLocationServiceEnabled)); + EXPECT_FALSE(prefs->IsManagedPreference(prefs::kArcLocationServiceEnabled)); + expected_broadcast_extras.SetBoolean("enabled", false); + expected_broadcast_extras.SetBoolean("managed", false); EXPECT_EQ(CountBroadcasts(fake_intent_helper_instance_->broadcasts(), kLocationServiceBroadcastAction, &expected_broadcast_extras),
diff --git a/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler.cc b/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler.cc index 11972d6..8ae3b0f 100644 --- a/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler.cc +++ b/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler.cc
@@ -72,14 +72,26 @@ } void ArcOptInPreferenceHandler::SendBackupAndRestoreMode() { + // Override the pref default to the true value, in order to encourage users to + // consent with it during OptIn flow. + const bool enabled = + pref_service_->HasPrefPath(prefs::kArcBackupRestoreEnabled) + ? pref_service_->GetBoolean(prefs::kArcBackupRestoreEnabled) + : true; observer_->OnBackupAndRestoreModeChanged( - pref_service_->GetBoolean(prefs::kArcBackupRestoreEnabled), + enabled, pref_service_->IsManagedPreference(prefs::kArcBackupRestoreEnabled)); } void ArcOptInPreferenceHandler::SendLocationServicesMode() { + // Override the pref default to the true value, in order to encourage users to + // consent with it during OptIn flow. + const bool enabled = + pref_service_->HasPrefPath(prefs::kArcLocationServiceEnabled) + ? pref_service_->GetBoolean(prefs::kArcLocationServiceEnabled) + : true; observer_->OnLocationServicesModeChanged( - pref_service_->GetBoolean(prefs::kArcLocationServiceEnabled), + enabled, pref_service_->IsManagedPreference(prefs::kArcLocationServiceEnabled)); }
diff --git a/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler.h b/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler.h index 0d4ec31..2cc6d3b9 100644 --- a/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler.h +++ b/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler.h
@@ -20,6 +20,10 @@ // observes changes there. Changes in preferences and metrics mode are passed to // external consumer via ArcOptInPreferenceHandlerObserver. Once started it // immediately sends current state of metrics mode and preferences. +// +// Note that the preferences and metrics mode passed by this class should only +// be used for the OptIn flow, as this class overrides some of the defaults in +// order to encourage users to consent with the settings. class ArcOptInPreferenceHandler { public: ArcOptInPreferenceHandler(ArcOptInPreferenceHandlerObserver* observer,
diff --git a/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler_observer.h b/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler_observer.h index 56d9699..3d121f3 100644 --- a/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler_observer.h +++ b/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler_observer.h
@@ -10,11 +10,11 @@ // Notifies about changes in ARC related preferences and metrics mode. class ArcOptInPreferenceHandlerObserver { public: - // Notifies metrics mode has been changed. + // Notifies that the metrics mode has been changed. virtual void OnMetricsModeChanged(bool enabled, bool managed) = 0; - // Notifies use backup and restore preference has been changed. + // Notifies that the backup and restore mode has been changed. virtual void OnBackupAndRestoreModeChanged(bool enabled, bool managed) = 0; - // Notifies location service consent preference has been changed. + // Notifies that the location service mode has been changed. virtual void OnLocationServicesModeChanged(bool enabled, bool managed) = 0; virtual ~ArcOptInPreferenceHandlerObserver() = default;
diff --git a/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc b/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc index ce080c8d..78eb27f 100644 --- a/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc +++ b/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc
@@ -9,6 +9,7 @@ #include "base/macros.h" #include "base/memory/ptr_util.h" #include "base/run_loop.h" +#include "base/values.h" #include "chrome/browser/chromeos/arc/arc_support_host.h" #include "chrome/browser/chromeos/arc/extensions/fake_arc_support.h" #include "chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator.h" @@ -18,6 +19,7 @@ #include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_profile.h" #include "components/prefs/pref_service.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" #include "content/public/test/test_browser_thread_bundle.h" #include "testing/gtest/include/gtest/gtest.h" @@ -34,8 +36,6 @@ new chromeos::FakeChromeUserManager()); profile_ = base::MakeUnique<TestingProfile>(); - profile_->GetPrefs()->SetBoolean(prefs::kArcBackupRestoreEnabled, false); - profile_->GetPrefs()->SetBoolean(prefs::kArcLocationServiceEnabled, false); support_host_ = base::MakeUnique<ArcSupportHost>(profile_.get()); fake_arc_support_ = base::MakeUnique<FakeArcSupport>(support_host_.get()); @@ -51,7 +51,7 @@ user_manager_enabler_.reset(); } - Profile* profile() { return profile_.get(); } + TestingProfile* profile() { return profile_.get(); } ArcSupportHost* support_host() { return support_host_.get(); } FakeArcSupport* fake_arc_support() { return fake_arc_support_.get(); } ArcTermsOfServiceNegotiator* negotiator() { return negotiator_.get(); } @@ -112,10 +112,32 @@ EXPECT_EQ(status, Status::PENDING); EXPECT_EQ(fake_arc_support()->ui_page(), ArcSupportHost::UIPage::TERMS); - // Check the preference related checkbox. - fake_arc_support()->set_metrics_mode(true); - fake_arc_support()->set_backup_and_restore_mode(true); - fake_arc_support()->set_location_service_mode(true); + // By default, the preference related checkboxes are checked, despite that + // the preferences default to false. + EXPECT_FALSE( + profile()->GetPrefs()->GetBoolean(prefs::kArcBackupRestoreEnabled)); + EXPECT_FALSE( + profile()->GetPrefs()->GetBoolean(prefs::kArcLocationServiceEnabled)); + EXPECT_TRUE(fake_arc_support()->backup_and_restore_mode()); + EXPECT_TRUE(fake_arc_support()->location_service_mode()); + + // The preferences are assigned to the managed false value, and the + // corresponding checkboxes are unchecked. + profile()->GetTestingPrefService()->SetManagedPref( + prefs::kArcBackupRestoreEnabled, new base::Value(false)); + EXPECT_FALSE(fake_arc_support()->backup_and_restore_mode()); + profile()->GetTestingPrefService()->SetManagedPref( + prefs::kArcLocationServiceEnabled, new base::Value(false)); + EXPECT_FALSE(fake_arc_support()->location_service_mode()); + + // The managed preference values are removed, and the corresponding checkboxes + // are checked again. + profile()->GetTestingPrefService()->RemoveManagedPref( + prefs::kArcBackupRestoreEnabled); + EXPECT_TRUE(fake_arc_support()->backup_and_restore_mode()); + profile()->GetTestingPrefService()->RemoveManagedPref( + prefs::kArcLocationServiceEnabled); + EXPECT_TRUE(fake_arc_support()->location_service_mode()); // Make sure preference values are not yet updated. EXPECT_FALSE( @@ -135,6 +157,41 @@ profile()->GetPrefs()->GetBoolean(prefs::kArcLocationServiceEnabled)); } +TEST_F(ArcTermsOfServiceDefaultNegotiatorTest, AcceptWithUnchecked) { + // Show Terms of service page. + Status status = Status::PENDING; + negotiator()->StartNegotiation(UpdateStatusCallback(&status)); + + // TERMS page should be shown. + EXPECT_EQ(status, Status::PENDING); + EXPECT_EQ(fake_arc_support()->ui_page(), ArcSupportHost::UIPage::TERMS); + + // Override the preferences from the default values to true. + profile()->GetPrefs()->SetBoolean(prefs::kArcBackupRestoreEnabled, true); + profile()->GetPrefs()->SetBoolean(prefs::kArcLocationServiceEnabled, true); + + // Uncheck the preference related checkboxes. + fake_arc_support()->set_backup_and_restore_mode(false); + fake_arc_support()->set_location_service_mode(false); + + // Make sure preference values are not yet updated. + EXPECT_TRUE( + profile()->GetPrefs()->GetBoolean(prefs::kArcBackupRestoreEnabled)); + EXPECT_TRUE( + profile()->GetPrefs()->GetBoolean(prefs::kArcLocationServiceEnabled)); + + // Click the "AGREE" button so that the callback should be invoked + // with |agreed| = true. + fake_arc_support()->ClickAgreeButton(); + EXPECT_EQ(status, Status::ACCEPTED); + + // Make sure preference values are now updated. + EXPECT_FALSE( + profile()->GetPrefs()->GetBoolean(prefs::kArcBackupRestoreEnabled)); + EXPECT_FALSE( + profile()->GetPrefs()->GetBoolean(prefs::kArcLocationServiceEnabled)); +} + TEST_F(ArcTermsOfServiceDefaultNegotiatorTest, Cancel) { // Show Terms of service page. Status status = Status::PENDING;
diff --git a/chrome/browser/policy/policy_browsertest.cc b/chrome/browser/policy/policy_browsertest.cc index daf6d39..fda3b28 100644 --- a/chrome/browser/policy/policy_browsertest.cc +++ b/chrome/browser/policy/policy_browsertest.cc
@@ -4170,13 +4170,17 @@ // Test ArcBackupRestoreEnabled policy. IN_PROC_BROWSER_TEST_F(ArcPolicyTest, ArcBackupRestoreEnabled) { - const PrefService* const pref = browser()->profile()->GetPrefs(); + PrefService* const pref = browser()->profile()->GetPrefs(); - // ARC Backup & Restore is switched on by default. - EXPECT_TRUE(pref->GetBoolean(prefs::kArcBackupRestoreEnabled)); + // ARC Backup & Restore is switched off by default. + EXPECT_FALSE(pref->GetBoolean(prefs::kArcBackupRestoreEnabled)); EXPECT_FALSE(pref->IsManagedPreference(prefs::kArcBackupRestoreEnabled)); - // Disable ARC Backup & Restore. + // Switch on ARC Backup & Restore in the user prefs. + pref->SetBoolean(prefs::kArcBackupRestoreEnabled, true); + EXPECT_TRUE(pref->GetBoolean(prefs::kArcBackupRestoreEnabled)); + + // Disable ARC Backup & Restore through the policy. PolicyMap policies; policies.Set(key::kArcBackupRestoreEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD, @@ -4185,7 +4189,7 @@ EXPECT_FALSE(pref->GetBoolean(prefs::kArcBackupRestoreEnabled)); EXPECT_TRUE(pref->IsManagedPreference(prefs::kArcBackupRestoreEnabled)); - // Enable ARC Backup & Restore. + // Enable ARC Backup & Restore through the policy. policies.Set(key::kArcBackupRestoreEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD, base::MakeUnique<base::FundamentalValue>(true), nullptr); @@ -4197,7 +4201,7 @@ // Test ArcLocationServiceEnabled policy and its interplay with the // DefaultGeolocationSetting policy. IN_PROC_BROWSER_TEST_F(ArcPolicyTest, ArcLocationServiceEnabled) { - const PrefService* const pref = browser()->profile()->GetPrefs(); + PrefService* const pref = browser()->profile()->GetPrefs(); // Values of the ArcLocationServiceEnabled policy to be tested. const std::vector<base::Value> test_policy_values = { @@ -4213,6 +4217,14 @@ base::FundamentalValue(3), // 'AskGeolocation' }; + // The pref is switched off by default. + EXPECT_FALSE(pref->GetBoolean(prefs::kArcLocationServiceEnabled)); + EXPECT_FALSE(pref->IsManagedPreference(prefs::kArcLocationServiceEnabled)); + + // Switch on the pref in the user prefs. + pref->SetBoolean(prefs::kArcLocationServiceEnabled, true); + EXPECT_TRUE(pref->GetBoolean(prefs::kArcLocationServiceEnabled)); + for (const auto& test_policy_value : test_policy_values) { for (const auto& test_default_geo_policy_value : test_default_geo_policy_values) {
diff --git a/chrome/browser/resources/chromeos/login/oobe_a11y_option.html b/chrome/browser/resources/chromeos/login/oobe_a11y_option.html index a130a33..4f7b080 100644 --- a/chrome/browser/resources/chromeos/login/oobe_a11y_option.html +++ b/chrome/browser/resources/chromeos/login/oobe_a11y_option.html
@@ -41,7 +41,8 @@ <content select=".unchecked-value"></content> </div> </div> - <paper-toggle-button checked="{{checked}}" aria-label$="[[ariaLabel]]"> + <paper-toggle-button id="button" checked="{{checked}}" + aria-label$="[[labelForAria]]"> </paper-toggle-button> </div> </template>
diff --git a/chrome/browser/resources/chromeos/login/oobe_a11y_option.js b/chrome/browser/resources/chromeos/login/oobe_a11y_option.js index 08a24d8..b053ec7 100644 --- a/chrome/browser/resources/chromeos/login/oobe_a11y_option.js +++ b/chrome/browser/resources/chromeos/login/oobe_a11y_option.js
@@ -23,6 +23,10 @@ /** * ARIA-label for the button. */ - ariaLabel: String, + labelForAria: String, + }, + + focus: function() { + this.$.button.focus(); }, });
diff --git a/chrome/browser/resources/chromeos/login/oobe_buttons.js b/chrome/browser/resources/chromeos/login/oobe_buttons.js index c89ea0d..53cf9f53 100644 --- a/chrome/browser/resources/chromeos/login/oobe_buttons.js +++ b/chrome/browser/resources/chromeos/login/oobe_buttons.js
@@ -67,4 +67,8 @@ ariaLabel: String }, + + focus: function() { + this.$.button.focus(); + }, });
diff --git a/chrome/browser/resources/chromeos/login/oobe_dialog.html b/chrome/browser/resources/chromeos/login/oobe_dialog.html index 628d8e143..e13624190 100644 --- a/chrome/browser/resources/chromeos/login/oobe_dialog.html +++ b/chrome/browser/resources/chromeos/login/oobe_dialog.html
@@ -11,15 +11,21 @@ three parts: header (which should have <h1>title</h1>), footer (content part), and optional buttons container at the bottom. + When shown (i.e. when outside container calls .show()): + 1. If this dialog has tags in class "focus-on-show", the first one will be + focused. + 2. 'show-dialog' is fired. + Example: <link rel="stylesheet" href="oobe_dialog_parameters.css"> - <oobe-dialog has-buttons> + <oobe-dialog on-show-dialog="onTestDialogShown_" has-buttons> <iron-icon ... class="oobe-icon"> <div class="header"> <h1 class="title">Title</h1> <div class="subtitle">Subtitle</div> </div> <div class="footer"> + <div class="focus-on-show">...</div> ... </div> <div class="bottom-buttons">
diff --git a/chrome/browser/resources/chromeos/login/oobe_dialog.js b/chrome/browser/resources/chromeos/login/oobe_dialog.js index 9496e39..ab3be06 100644 --- a/chrome/browser/resources/chromeos/login/oobe_dialog.js +++ b/chrome/browser/resources/chromeos/login/oobe_dialog.js
@@ -23,4 +23,17 @@ reflectToAttribute: true, }, }, + + focus: function() {}, + + /** + * This is called from oobe_welcome when this dialog is shown. + */ + show: function() { + var focusedElements = this.getElementsByClassName('focus-on-show'); + if (focusedElements) + focusedElements[0].focus(); + + this.fire('show-dialog'); + }, });
diff --git a/chrome/browser/resources/chromeos/login/oobe_eula.html b/chrome/browser/resources/chromeos/login/oobe_eula.html index 013c863f..12958f59 100644 --- a/chrome/browser/resources/chromeos/login/oobe_eula.html +++ b/chrome/browser/resources/chromeos/login/oobe_eula.html
@@ -27,21 +27,25 @@ <template> <link rel="stylesheet" href="oobe_eula.css"> <link rel="stylesheet" href="oobe_dialog_parameters.css"> - <oobe-dialog hidden="[[!eulaLoadingScreenShown]]" + <oobe-dialog id="eulaLoadingDialog" hidden="[[!eulaLoadingScreenShown]]" + role="dialog" aria-label="$i18n{termsOfServiceLoading}" has-buttons> <iron-icon icon="oobe-eula:googleg" class="oobe-icon"></iron-icon> <div class="header"> <h1 class="title" i18n-content="termsOfServiceLoading"></h1> </div> </oobe-dialog> - <oobe-dialog hidden="[[!eulaScreenShown]]" + <oobe-dialog id="eulaDialog" hidden="[[eulaLoadingScreenShown]]" + role="dialog" aria-label="$i18n{oobeEulaSectionTitle}" has-buttons> <iron-icon icon="oobe-eula:googleg" class="oobe-icon"></iron-icon> <div class="header"> <h1 class="title" i18n-content="oobeEulaSectionTitle"></h1> </div> <div class="footer flex layout vertical"> - <iframe id="crosEulaFrame" class="flex" src="chrome://terms" + <iframe id="crosEulaFrame" src="chrome://terms" + role="document" class="flex focus-on-show" + aria-label="$i18n{oobeEulaIframeLabel}" on-load="onFrameLoad_"> </iframe> <a id="installationSettings" href="#"
diff --git a/chrome/browser/resources/chromeos/login/oobe_eula.js b/chrome/browser/resources/chromeos/login/oobe_eula.js index 8ff6cf2..9790217 100644 --- a/chrome/browser/resources/chromeos/login/oobe_eula.js +++ b/chrome/browser/resources/chromeos/login/oobe_eula.js
@@ -20,13 +20,6 @@ }, /** - * Shows Terms Of Service frame. - */ - eulaScreenShown: { - type: Array, - }, - - /** * "Accepot and continue" button is disabled until content is loaded. */ acceptButtonDisabled: { @@ -51,6 +44,14 @@ }, }, + focus: function() { + if (this.eulaLoadingScreenShown) { + this.$.eulaLoadingDialog.show(); + } else { + this.$.eulaDialog.show(); + } + }, + /** * Event handler that is invoked when 'chrome://terms' is loaded. */
diff --git a/chrome/browser/resources/chromeos/login/oobe_i18n_dropdown.html b/chrome/browser/resources/chromeos/login/oobe_i18n_dropdown.html index 67100b00..829bfcfb 100644 --- a/chrome/browser/resources/chromeos/login/oobe_i18n_dropdown.html +++ b/chrome/browser/resources/chromeos/login/oobe_i18n_dropdown.html
@@ -8,7 +8,7 @@ <style include="md-select"></style> <div id="container" class="flex vertical layout center center-justified md-select-wrapper"> - <select id="select" aria-label$="[[ariaLabel]]" class="md-select" > + <select id="select" aria-label$="[[labelForAria]]" class="md-select" > </select> <span class="md-select-underline"></span> </div>
diff --git a/chrome/browser/resources/chromeos/login/oobe_i18n_dropdown.js b/chrome/browser/resources/chromeos/login/oobe_i18n_dropdown.js index bcd062d..7e7253a 100644 --- a/chrome/browser/resources/chromeos/login/oobe_i18n_dropdown.js +++ b/chrome/browser/resources/chromeos/login/oobe_i18n_dropdown.js
@@ -30,7 +30,7 @@ /** * ARIA-label for the selection menu. */ - ariaLabel: String, + labelForAria: String, }, /** @@ -39,6 +39,10 @@ */ idToItem_: null, + focus: function() { + this.$.select.focus(); + }, + /** * @param {string} value Option value. * @private
diff --git a/chrome/browser/resources/chromeos/login/oobe_screen_eula.js b/chrome/browser/resources/chromeos/login/oobe_screen_eula.js index ef6ca5d..ba1a2b8 100644 --- a/chrome/browser/resources/chromeos/login/oobe_screen_eula.js +++ b/chrome/browser/resources/chromeos/login/oobe_screen_eula.js
@@ -117,6 +117,9 @@ * Returns a control which should receive an initial focus. */ get defaultControl() { + if (loadTimeData.getString('newOobeUI') == 'on') + return $('oobe-eula-md'); + return $('accept-button').disabled ? $('back-button') : $('accept-button'); },
diff --git a/chrome/browser/resources/chromeos/login/oobe_screen_network.js b/chrome/browser/resources/chromeos/login/oobe_screen_network.js index b401e7a9..9291982e 100644 --- a/chrome/browser/resources/chromeos/login/oobe_screen_network.js +++ b/chrome/browser/resources/chromeos/login/oobe_screen_network.js
@@ -146,6 +146,9 @@ * Returns a control which should receive an initial focus. */ get defaultControl() { + if (loadTimeData.getString('newOobeUI') == 'on') + return $('oobe-welcome-md'); + return $('language-select'); },
diff --git a/chrome/browser/resources/chromeos/login/oobe_welcome.html b/chrome/browser/resources/chromeos/login/oobe_welcome.html index 02cd158..52ff0bb 100644 --- a/chrome/browser/resources/chromeos/login/oobe_welcome.html +++ b/chrome/browser/resources/chromeos/login/oobe_welcome.html
@@ -57,7 +57,8 @@ <link rel="stylesheet" href="oobe_dialog_host.css"> <link rel="stylesheet" href="oobe_welcome.css"> <link rel="stylesheet" href="oobe_dialog_parameters.css"> - <oobe-welcome-dialog id="welcomeScreen" hidden="[[!welcomeScreenShown]]" + <oobe-welcome-dialog id="welcomeScreen" role="dialog" + aria-label="[[formatMessage_('networkScreenGreeting')]]" current-language="[[currentLanguage]]" on-language-button-clicked="onWelcomeSelectLanguageButtonClicked_" on-accessibility-button-clicked="onWelcomeAccessibilityButtonClicked_" @@ -67,8 +68,8 @@ debugging-link-visible="[[debuggingLinkVisible]]" > </oobe-welcome-dialog> - <oobe-dialog id="languageScreen" hidden="[[!languageSelectionScreenShown]]" - has-buttons> + <oobe-dialog id="languageScreen" role="dialog" hidden has-buttons + aria-label="[[formatMessage_('languageSectionTitle')]]"> <iron-icon icon="icons:language" class="oobe-icon"></iron-icon> <div class="header"> <h1 class="title" i18n-content="languageSectionTitle"></h1> @@ -82,8 +83,7 @@ </div> <oobe-i18n-dropdown id="languageSelect" items="[[languages]]" on-select-item="onLanguageSelected_" - aria-label="[[formatMessage_('languageDropdownLabel', - currentLanguage)]]"> + class="focus-on-show"> </oobe-i18n-dropdown> </div> <div id="keyboardDropdownContainer" @@ -92,9 +92,7 @@ i18n-content="keyboardDropdownTitle"> </div> <oobe-i18n-dropdown id="keyboardSelect" items="[[keyboards]]" - on-select-item="onKeyboardSelected_" - aria-label="[[formatMessage_('keyboardDropdownLabel', - currentKeyboard)]]"> + on-select-item="onKeyboardSelected_"> </oobe-i18n-dropdown> </div> </template> @@ -105,8 +103,8 @@ </oobe-text-button> </div> </oobe-dialog> - <oobe-dialog id="accessibilityScreen" - hidden="[[!accessibilityOptionsScreenShown]]" has-buttons> + <oobe-dialog id="accessibilityScreen" role="dialog" hidden has-buttons + aria-label="[[formatMessage_('accessibilitySectionTitle')]]"> <iron-icon icon="icons:accessibility" class="oobe-icon"></iron-icon> <div class="header"> <h1 class="title" i18n-content="accessibilitySectionTitle"></h1> @@ -116,7 +114,8 @@ <oobe-a11y-option checked="[[a11yStatus.spokenFeedbackEnabled]]" on-change="onA11yOptionChanged_" chrome-message="enableSpokenFeedback" - aria-label="[[formatMessage_('spokenFeedbackOption')]]"> + label-for-aria="[[formatMessage_('spokenFeedbackOption')]]" + class="focus-on-show"> <span class="title" i18n-content="spokenFeedbackOption"></span> <span class="checked-value" i18n-content="spokenFeedbackOptionOn"> </span> @@ -126,7 +125,7 @@ <oobe-a11y-option checked="[[a11yStatus.largeCursorEnabled]]" on-change="onA11yOptionChanged_" chrome-message="enableLargeCursor" - aria-label="[[formatMessage_('largeCursorOption')]]"> + label-for-aria="[[formatMessage_('largeCursorOption')]]"> <span class="title" i18n-content="largeCursorOption"></span> <span class="checked-value" i18n-content="largeCursorOptionOn"> </span> @@ -136,7 +135,7 @@ <oobe-a11y-option checked="[[a11yStatus.highContrastEnabled]]" on-change="onA11yOptionChanged_" chrome-message="enableHighContrast" - aria-label="[[formatMessage_('highContrastOption')]]"> + label-for-aria="[[formatMessage_('highContrastOption')]]"> <span class="title" i18n-content="highContrastOption"></span> <span class="checked-value" i18n-content="highContrastOptionOn"> </span> @@ -146,7 +145,7 @@ <oobe-a11y-option checked="[[a11yStatus.screenMagnifierEnabled]]" on-change="onA11yOptionChanged_" chrome-message="enableScreenMagnifier" - aria-label="[[formatMessage_('screenMagnifierOption')]]"> + label-for-aria="[[formatMessage_('screenMagnifierOption')]]"> <span class="title" i18n-content="screenMagnifierOption"></span> <span class="checked-value" i18n-content="screenMagnifierOptionOn"> </span> @@ -156,7 +155,7 @@ <oobe-a11y-option checked="[[a11yStatus.virtualKeyboardEnabled]]" on-change="onA11yOptionChanged_" chrome-message="enableVirtualKeyboard" - aria-label="[[formatMessage_('virtualKeyboardOption')]]"> + label-for-aria="[[formatMessage_('virtualKeyboardOption')]]"> <span class="title" i18n-content="virtualKeyboardOption"></span> <span class="checked-value" i18n-content="virtualKeyboardOptionOn"> </span> @@ -170,8 +169,8 @@ </oobe-text-button> </div> </oobe-dialog> - <oobe-dialog id="timezoneScreen" hidden="[[!timezoneScreenShown]]" - has-buttons> + <oobe-dialog id="timezoneScreen" role="dialog" hidden has-buttons + aria-label="[[formatMessage_('timezoneSectionTitle')]]"> <iron-icon icon="oobe-welcome-64:timezone" class="oobe-icon"></iron-icon> <div class="header"> <h1 class="title" i18n-content="timezoneSectionTitle"></h1> @@ -183,7 +182,8 @@ </div> <oobe-i18n-dropdown id="timezoneSelect" items="[[timezones]]" on-select-item="onTimezoneSelected_" - aria-label="[[formatMessage_('timezoneDropdownTitle')]]"> + label-for-aria="[[formatMessage_('timezoneDropdownTitle')]]" + class="focus-on-show"> </oobe-i18n-dropdown> </div> </div> @@ -193,8 +193,9 @@ </oobe-text-button> </div> </oobe-dialog> - <oobe-dialog id="networkSelectionScreen" - hidden="[[!networkSelectionScreenShown]]" has-buttons> + <oobe-dialog id="networkSelectionScreen" role="dialog" hidden has-buttons + aria-label="[[formatMessage_('networkSectionTitle')]]" + on-show-dialog="onNetworkSelectionScreenShown_"> <iron-icon icon="oobe-welcome:wifi" class="oobe-icon"></iron-icon> <div class="header"> <h1 class="title" i18n-content="networkSectionTitle"></h1> @@ -205,8 +206,9 @@ on-default-network-changed="onDefaultNetworkChanged_" on-network-item-selected="onNetworkListNetworkItemSelected_" on-custom-item-selected="onNetworkListCustomItemSelected_" - custom-items="[[_getNetworkCustomItems()]]" - no-bottom-scroll-border> + custom-items="[[getNetworkCustomItems_()]]" + no-bottom-scroll-border + class="focus-on-show"> </cr-network-select> </div> <div class="bottom-buttons layout horizontal justified">
diff --git a/chrome/browser/resources/chromeos/login/oobe_welcome.js b/chrome/browser/resources/chromeos/login/oobe_welcome.js index 88fb183..f2be9d8 100644 --- a/chrome/browser/resources/chromeos/login/oobe_welcome.js +++ b/chrome/browser/resources/chromeos/login/oobe_welcome.js
@@ -45,47 +45,6 @@ }, /** - * Flag that shows Welcome screen. - */ - welcomeScreenShown: { - type: Boolean, - value: true, - }, - - /** - * Flag that shows Language Selection screen. - */ - languageSelectionScreenShown: { - type: Boolean, - value: false, - }, - - /** - * Flag that shows Accessibility Options screen. - */ - accessibilityOptionsScreenShown: { - type: Boolean, - value: false, - }, - - /** - * Flag that shows Timezone Selection screen. - */ - timezoneScreenShown: { - type: Boolean, - value: false, - }, - - /** - * Flag that shows Network Selection screen. - */ - networkSelectionScreenShown: { - type: Boolean, - value: false, - observer: 'networkSelectionScreenShownChanged_', - }, - - /** * Flag that enables MD-OOBE. */ enabled: { @@ -133,6 +92,13 @@ debuggingLinkVisible: Boolean, }, + /** + * GUID of the user-selected network. It is remembered after user taps on + * network entry. After we receive event "connected" on this network, + * OOBE will proceed. + */ + networkLastSelectedGuid_: '', + /** @override */ ready: function() { CrOncStrings = { @@ -162,39 +128,56 @@ /** * Hides all screens to help switching from one screen to another. + * @private */ hideAllScreens_: function() { - this.welcomeScreenShown = false; - this.networkSelectionScreenShown = false; - this.languageSelectionScreenShown = false; - this.accessibilityOptionsScreenShown = false; - this.timezoneScreenShown = false; + this.$.welcomeScreen.hidden = true; + + var screens = Polymer.dom(this.root).querySelectorAll('oobe-dialog') + for (var i = 0; i < screens.length; ++i) { + screens[i].hidden = true; + } }, /** - * GUID of the user-selected network. It is remembered after user taps on - * network entry. After we receive event "connected" on this network, - * OOBE will proceed. + * Shows given screen. + * @param id String Screen ID. + * @private */ - networkLastSelectedGuid_: '', + showScreen_: function(id) { + this.hideAllScreens_(); + + var screen = this.$[id]; + assert(screen); + screen.hidden = false; + screen.show(); + }, /** - * Sets proper focus. + * Returns active screen object. + * @private */ + getActiveScreen_: function() { + var screens = Polymer.dom(this.root).querySelectorAll('oobe-dialog') + for (var i = 0; i < screens.length; ++i) { + if (!screens[i].hidden) + return screens[i]; + } + return this.$.welcomeScreen; + }, + focus: function() { - this.$.welcomeNextButton.focus(); + this.getActiveScreen_().focus(); }, /** @private */ - networkSelectionScreenShownChanged_: function() { - if (this.networkSelectionScreenShown) { - // After #networkSelect is stamped, trigger a refresh so that the list - // will be updated with the currently visible networks and sized - // appropriately. - this.async(function() { - this.$.networkSelect.refreshNetworks(); - }.bind(this)); - } + onNetworkSelectionScreenShown_: function() { + // After #networkSelect is stamped, trigger a refresh so that the list + // will be updated with the currently visible networks and sized + // appropriately. + this.async(function() { + this.$.networkSelect.refreshNetworks(); + }.bind(this)); }, /** @@ -207,8 +190,9 @@ /** * Returns custom items for network selector. + * @private */ - _getNetworkCustomItems: function() { + getNetworkCustomItems_: function() { var self = this; return [ { @@ -249,8 +233,7 @@ * @private */ onWelcomeNextButtonClicked_: function() { - this.hideAllScreens_(); - this.networkSelectionScreenShown = true; + this.showScreen_('networkSelectionScreen'); }, /** @@ -259,8 +242,7 @@ * @private */ onWelcomeSelectLanguageButtonClicked_: function() { - this.hideAllScreens_(); - this.languageSelectionScreenShown = true; + this.showScreen_('languageScreen'); }, /** @@ -269,8 +251,7 @@ * @private */ onWelcomeAccessibilityButtonClicked_: function() { - this.hideAllScreens_(); - this.accessibilityOptionsScreenShown = true; + this.showScreen_('accessibilityScreen'); }, /** @@ -279,8 +260,7 @@ * @private */ onWelcomeTimezoneButtonClicked_: function() { - this.hideAllScreens_(); - this.timezoneScreenShown = true; + this.showScreen_('timezoneScreen'); }, /** @@ -392,8 +372,7 @@ */ onNetworkSelectionBackButtonPressed_: function() { this.networkLastSelectedGuid_ = ''; - this.hideAllScreens_(); - this.welcomeScreenShown = true; + this.showScreen_('welcomeScreen'); }, /** @@ -436,8 +415,7 @@ * @private */ closeLanguageSection_: function() { - this.hideAllScreens_(); - this.welcomeScreenShown = true; + this.showScreen_('welcomeScreen'); }, /** ******************** Accessibility section ******************* */ @@ -448,8 +426,7 @@ * @private */ closeAccessibilitySection_: function() { - this.hideAllScreens_(); - this.welcomeScreenShown = true; + this.showScreen_('welcomeScreen'); }, /** @@ -473,8 +450,7 @@ * @private */ closeTimezoneSection_: function() { - this.hideAllScreens_(); - this.welcomeScreenShown = true; + this.showScreen_('welcomeScreen'); }, /**
diff --git a/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.html b/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.html index 0dd1a8d..5e290c1f 100644 --- a/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.html +++ b/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.html
@@ -24,22 +24,24 @@ </div> <div class="flex"></div> <div id="buttons" class="layout horizontal center"> - <oobe-welcome-secondary-button icon="icons:language" - on-tap="onLanguageClicked_" + <oobe-welcome-secondary-button id="languageSelectionButton" + icon="icons:language" on-tap="onLanguageClicked_" aria-label="[[formatMessage_('languageButtonLabel', currentLanguage)]]"> <div>[[currentLanguage]]</div> </oobe-welcome-secondary-button> - <oobe-welcome-secondary-button icon="icons:accessibility" - on-tap="onAccessibilityClicked_"> + <oobe-welcome-secondary-button id="accessibilitySettingsButton" + icon="icons:accessibility" on-tap="onAccessibilityClicked_"> <div i18n-content="accessibilityLink"></div> </oobe-welcome-secondary-button> - <oobe-welcome-secondary-button icon="oobe-welcome-64:timezone" - on-tap="onTimezoneClicked_" hidden="[[!timezoneButtonVisible]]"> + <oobe-welcome-secondary-button id="timezoneSettingsButton" + icon="oobe-welcome-64:timezone" on-tap="onTimezoneClicked_" + hidden="[[!timezoneButtonVisible]]"> <div i18n-content="timezoneButtonText"></div> </oobe-welcome-secondary-button> <div class="flex"></div> - <oobe-text-button inverse on-tap="onNextClicked_"> + <oobe-text-button id="welcomeNextButton" + inverse on-tap="onNextClicked_"> <div i18n-content="welcomeNextButtonText"></div> </oobe-text-button> </div>
diff --git a/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.js b/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.js index f969be3f..2ee3039 100644 --- a/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.js +++ b/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.js
@@ -12,6 +12,7 @@ type: String, value: '', }, + /** * Controls visibility of "Timezone" button. */ @@ -26,19 +27,29 @@ debuggingLinkVisible: Boolean, }, + /** + * This is stored ID of currently focused element to restore id on returns + * to this dialog from Language / Timezone Selection dialogs. + */ + focusedElement_: 'languageSelectionButton', + onLanguageClicked_: function() { + this.focusedElement_ = "languageSelectionButton"; this.fire('language-button-clicked'); }, onAccessibilityClicked_: function() { + this.focusedElement_ = "accessibilitySettingsButton"; this.fire('accessibility-button-clicked'); }, onTimezoneClicked_: function() { + this.focusedElement_ = "timezoneSettingsButton"; this.fire('timezone-button-clicked'); }, onNextClicked_: function() { + this.focusedElement_ = "welcomeNextButton"; this.fire('next-button-clicked'); }, @@ -47,6 +58,23 @@ ['connect-debugging-features']); }, + attached: function() { + this.focus(); + }, + + focus: function() { + var focusedElement = this.$[this.focusedElement_]; + if (focusedElement) + focusedElement.focus(); + }, + + /** + * This is called from oobe_welcome when this dialog is shown. + */ + show: function() { + this.focus(); + }, + /** * This function formats message for labels. * @param String label i18n string ID.
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn index 08e73667..92cca038 100644 --- a/chrome/browser/ui/BUILD.gn +++ b/chrome/browser/ui/BUILD.gn
@@ -2570,6 +2570,8 @@ "cocoa/browser_window_fullscreen_transition.mm", "cocoa/browser_window_layout.h", "cocoa/browser_window_layout.mm", + "cocoa/browser_window_touch_bar.h", + "cocoa/browser_window_touch_bar.mm", "cocoa/browser_window_utils.h", "cocoa/browser_window_utils.mm", "cocoa/bubble_combobox.h",
diff --git a/chrome/browser/ui/cocoa/browser_window_controller.h b/chrome/browser/ui/cocoa/browser_window_controller.h index 0504425..705b592 100644 --- a/chrome/browser/ui/cocoa/browser_window_controller.h +++ b/chrome/browser/ui/cocoa/browser_window_controller.h
@@ -36,6 +36,7 @@ class BrowserWindow; class BrowserWindowCocoa; @class BrowserWindowFullscreenTransition; +@class BrowserWindowTouchBar; @class DevToolsController; @class DownloadShelfController; class ExtensionKeybindingRegistryCocoa; @@ -92,6 +93,7 @@ fullscreenTransition_; std::unique_ptr<FullscreenLowPowerCoordinatorCocoa> fullscreenLowPowerCoordinator_; + base::scoped_nsobject<BrowserWindowTouchBar> touchBar_; // Strong. StatusBubble is a special case of a strong reference that // we don't wrap in a scoped_ptr because it is acting the same @@ -385,6 +387,9 @@ // UpdateAlertState. - (TabAlertState)alertState; +// Returns the BrowserWindowTouchBar object associated with the window. +- (BrowserWindowTouchBar*)browserWindowTouchBar; + @end // @interface BrowserWindowController
diff --git a/chrome/browser/ui/cocoa/browser_window_controller.mm b/chrome/browser/ui/cocoa/browser_window_controller.mm index 8ddd56220..3c12b1c 100644 --- a/chrome/browser/ui/cocoa/browser_window_controller.mm +++ b/chrome/browser/ui/cocoa/browser_window_controller.mm
@@ -48,6 +48,7 @@ #import "chrome/browser/ui/cocoa/browser_window_command_handler.h" #import "chrome/browser/ui/cocoa/browser_window_controller_private.h" #import "chrome/browser/ui/cocoa/browser_window_layout.h" +#import "chrome/browser/ui/cocoa/browser_window_touch_bar.h" #import "chrome/browser/ui/cocoa/browser_window_utils.h" #import "chrome/browser/ui/cocoa/dev_tools_controller.h" #import "chrome/browser/ui/cocoa/download/download_shelf_controller.h" @@ -96,6 +97,7 @@ #include "content/public/browser/web_contents.h" #import "ui/base/cocoa/cocoa_base_utils.h" #import "ui/base/cocoa/nsview_additions.h" +#import "ui/base/cocoa/touch_bar_forward_declarations.h" #include "ui/base/material_design/material_design_controller.h" #include "ui/display/screen.h" #import "ui/gfx/mac/coordinate_conversion.h" @@ -1003,6 +1005,10 @@ - (void)setStarredState:(BOOL)isStarred { [toolbarController_ setStarredState:isStarred]; + + [touchBar_ setIsStarred:isStarred]; + if ([[self window] respondsToSelector:@selector(setTouchBar:)]) + [[self window] performSelector:@selector(setTouchBar:) withObject:nil]; } - (void)setCurrentPageIsTranslated:(BOOL)on { @@ -1144,6 +1150,9 @@ - (void)setIsLoading:(BOOL)isLoading force:(BOOL)force { [toolbarController_ setIsLoading:isLoading force:force]; + [touchBar_ setIsPageLoading:isLoading]; + if ([[self window] respondsToSelector:@selector(setTouchBar:)]) + [[self window] performSelector:@selector(setTouchBar:) withObject:nil]; } // Make the location bar the first responder, if possible. @@ -1846,6 +1855,15 @@ return static_cast<BrowserWindowCocoa*>([self browserWindow])->alert_state(); } +- (BrowserWindowTouchBar*)browserWindowTouchBar { + if (!touchBar_) { + touchBar_.reset( + [[BrowserWindowTouchBar alloc] initWithBrowser:browser_.get()]); + } + + return touchBar_.get(); +} + @end // @implementation BrowserWindowController @implementation BrowserWindowController(Fullscreen)
diff --git a/chrome/browser/ui/cocoa/browser_window_touch_bar.h b/chrome/browser/ui/cocoa/browser_window_touch_bar.h new file mode 100644 index 0000000..5e670650 --- /dev/null +++ b/chrome/browser/ui/cocoa/browser_window_touch_bar.h
@@ -0,0 +1,32 @@ +// 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 CHROME_BROWSER_UI_COCOA_BROWSER_WINDOW_TOUCH_BAR_H_ +#define CHROME_BROWSER_UI_COCOA_BROWSER_WINDOW_TOUCH_BAR_H_ + +#import <Cocoa/Cocoa.h> + +#import "ui/base/cocoa/touch_bar_forward_declarations.h" + +class Browser; + +// Provides a touch bar for the browser window. This class implements the +// NSTouchBarDelegate and handles the items in the touch bar. +@interface BrowserWindowTouchBar : NSObject<NSTouchBarDelegate> +// True is the current page is loading. Used to determine if a stop or reload +// button should be provided. +@property(nonatomic, assign) BOOL isPageLoading; + +// True if the current page is starred. Used by star touch bar button. +@property(nonatomic, assign) BOOL isStarred; + +// Designated initializer. +- (instancetype)initWithBrowser:(Browser*)browser; + +// Creates and returns a touch bar for the browser window. +- (NSTouchBar*)makeTouchBar; + +@end + +#endif // CHROME_BROWSER_UI_COCOA_BROWSER_WINDOW_TOUCH_BAR_H_
diff --git a/chrome/browser/ui/cocoa/browser_window_touch_bar.mm b/chrome/browser/ui/cocoa/browser_window_touch_bar.mm new file mode 100644 index 0000000..21de9b4a --- /dev/null +++ b/chrome/browser/ui/cocoa/browser_window_touch_bar.mm
@@ -0,0 +1,210 @@ +// 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. + +#import "chrome/browser/ui/cocoa/browser_window_touch_bar.h" + +#include "base/mac/mac_util.h" +#import "base/mac/scoped_nsobject.h" +#import "base/mac/sdk_forward_declarations.h" +#include "base/strings/sys_string_conversions.h" +#include "chrome/app/chrome_command_ids.h" +#include "chrome/app/vector_icons/vector_icons.h" +#include "chrome/browser/command_updater.h" +#include "chrome/browser/search_engines/template_url_service_factory.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_command_controller.h" +#include "chrome/common/chrome_features.h" +#include "chrome/grit/generated_resources.h" +#include "components/omnibox/browser/vector_icons.h" +#include "components/search_engines/util.h" +#include "components/toolbar/vector_icons.h" +#include "ui/base/l10n/l10n_util.h" +#include "ui/gfx/color_palette.h" +#include "ui/gfx/color_utils.h" +#include "ui/gfx/image/image.h" +#include "ui/gfx/image/image_skia_util_mac.h" +#include "ui/gfx/paint_vector_icon.h" +#include "ui/native_theme/native_theme.h" +#include "ui/vector_icons/vector_icons.h" + +namespace { + +// The touch bar's identifier. +const NSTouchBarCustomizationIdentifier kBrowserWindowTouchBarId = + @"BrowserWindowTouchBarId"; + +// Touch bar items identifiers. +const NSTouchBarItemIdentifier kBackForwardTouchId = @"BackForwardTouchId"; +const NSTouchBarItemIdentifier kReloadOrStopTouchId = @"ReloadOrStopTouchId"; +const NSTouchBarItemIdentifier kSearchTouchId = @"SearchTouchId"; +const NSTouchBarItemIdentifier kNewTabTouchId = @"NewTabTouchId"; +const NSTouchBarItemIdentifier kStarTouchId = @"StarTouchId"; + +// The button indexes in the back and forward segment control. +const int kBackSegmentIndex = 0; +const int kForwardSegmentIndex = 1; + +// Touch bar icon colors values. +const SkColor kTouchBarDefaultIconColor = SK_ColorWHITE; + +// The size of the touch bar icons. +const int kTouchBarIconSize = 16; + +// The width of the search button in the touch bar. +const int kTouchBarSearchButtonWidth = 280; + +// Creates an NSImage from the given VectorIcon. +NSImage* CreateNSImageFromIcon(const gfx::VectorIcon& icon, + SkColor color = kTouchBarDefaultIconColor) { + return NSImageFromImageSkiaWithColorSpace( + gfx::CreateVectorIcon(icon, kTouchBarIconSize, color), + base::mac::GetSRGBColorSpace()); +} + +// Creates a NSButton for the touch bar. +NSButton* CreateTouchBarButton(const gfx::VectorIcon& icon, + BrowserWindowTouchBar* owner, + int command, + SkColor color = kTouchBarDefaultIconColor) { + NSButton* button = + [NSButton buttonWithImage:CreateNSImageFromIcon(icon, color) + target:owner + action:@selector(executeCommand:)]; + button.tag = command; + return button; +} + +} // namespace + +@interface BrowserWindowTouchBar () { + // Used to execute commands such as navigating back and forward. + CommandUpdater* commandUpdater_; // Weak, owned by Browser. + + // The browser associated with the touch bar. + Browser* browser_; // Weak. +} + +// Creates and return the back and forward segmented buttons. +- (NSView*)backOrForwardTouchBarView; + +// Creates and returns the search button. +- (NSView*)searchTouchBarView; +@end + +@implementation BrowserWindowTouchBar + +@synthesize isPageLoading = isPageLoading_; +@synthesize isStarred = isStarred_; + +- (instancetype)initWithBrowser:(Browser*)browser { + if ((self = [self init])) { + DCHECK(browser); + commandUpdater_ = browser->command_controller()->command_updater(); + browser_ = browser; + } + + return self; +} + +- (NSTouchBar*)makeTouchBar { + if (!base::FeatureList::IsEnabled(features::kBrowserTouchBar)) + return nil; + + base::scoped_nsobject<NSTouchBar> touchBar( + [[NSClassFromString(@"NSTouchBar") alloc] init]); + NSArray* touchBarItemIdentifiers = @[ + kBackForwardTouchId, kReloadOrStopTouchId, kSearchTouchId, kNewTabTouchId, + kStarTouchId + ]; + [touchBar setCustomizationIdentifier:kBrowserWindowTouchBarId]; + [touchBar setDefaultItemIdentifiers:touchBarItemIdentifiers]; + [touchBar setCustomizationAllowedItemIdentifiers:touchBarItemIdentifiers]; + [touchBar setDelegate:self]; + + return touchBar.autorelease(); +} + +- (NSTouchBarItem*)touchBar:(NSTouchBar*)touchBar + makeItemForIdentifier:(NSTouchBarItemIdentifier)identifier { + if (!touchBar) + return nil; + + base::scoped_nsobject<NSCustomTouchBarItem> touchBarItem([[NSClassFromString( + @"NSCustomTouchBarItem") alloc] initWithIdentifier:identifier]); + if ([identifier isEqualTo:kBackForwardTouchId]) { + [touchBarItem setView:[self backOrForwardTouchBarView]]; + } else if ([identifier isEqualTo:kReloadOrStopTouchId]) { + const gfx::VectorIcon& icon = + isPageLoading_ ? kNavigateStopIcon : kNavigateReloadIcon; + int command_id = isPageLoading_ ? IDC_STOP : IDC_RELOAD; + [touchBarItem setView:CreateTouchBarButton(icon, self, command_id)]; + } else if ([identifier isEqualTo:kNewTabTouchId]) { + [touchBarItem setView:CreateTouchBarButton(kNewTabMacTouchbarIcon, self, + IDC_NEW_TAB)]; + } else if ([identifier isEqualTo:kStarTouchId]) { + const SkColor kStarredIconColor = + ui::NativeTheme::GetInstanceForNativeUi()->GetSystemColor( + ui::NativeTheme::kColorId_ProminentButtonColor); + const gfx::VectorIcon& icon = + isStarred_ ? toolbar::kStarActiveIcon : toolbar::kStarIcon; + SkColor iconColor = + isStarred_ ? kStarredIconColor : kTouchBarDefaultIconColor; + [touchBarItem + setView:CreateTouchBarButton(icon, self, IDC_BOOKMARK_PAGE, iconColor)]; + } else if ([identifier isEqualTo:kSearchTouchId]) { + [touchBarItem setView:[self searchTouchBarView]]; + } + + return touchBarItem.autorelease(); +} + +- (NSView*)backOrForwardTouchBarView { + NSArray* images = @[ + CreateNSImageFromIcon(ui::kBackArrowIcon), + CreateNSImageFromIcon(ui::kForwardArrowIcon) + ]; + + NSSegmentedControl* control = [NSSegmentedControl + segmentedControlWithImages:images + trackingMode:NSSegmentSwitchTrackingMomentary + target:self + action:@selector(backOrForward:)]; + control.segmentStyle = NSSegmentStyleSeparated; + [control setEnabled:commandUpdater_->IsCommandEnabled(IDC_BACK) + forSegment:kBackSegmentIndex]; + [control setEnabled:commandUpdater_->IsCommandEnabled(IDC_FORWARD) + forSegment:kForwardSegmentIndex]; + return control; +} + +- (NSView*)searchTouchBarView { + // TODO(spqchan): Use the Google search icon if the default search engine is + // Google. + base::string16 title = l10n_util::GetStringUTF16(IDS_TOUCH_BAR_SEARCH); + NSButton* searchButton = + [NSButton buttonWithTitle:base::SysUTF16ToNSString(title) + image:CreateNSImageFromIcon(omnibox::kSearchIcon) + target:self + action:@selector(executeCommand:)]; + searchButton.imageHugsTitle = YES; + searchButton.tag = IDC_FOCUS_LOCATION; + [searchButton.widthAnchor + constraintEqualToConstant:kTouchBarSearchButtonWidth] + .active = YES; + return searchButton; +} + +- (void)backOrForward:(id)sender { + NSSegmentedControl* control = sender; + if ([control selectedSegment] == kBackSegmentIndex) + commandUpdater_->ExecuteCommand(IDC_BACK); + else + commandUpdater_->ExecuteCommand(IDC_FORWARD); +} + +- (void)executeCommand:(id)sender { + commandUpdater_->ExecuteCommand([sender tag]); +} + +@end \ No newline at end of file
diff --git a/chrome/browser/ui/cocoa/browser_window_touch_bar_unittest.mm b/chrome/browser/ui/cocoa/browser_window_touch_bar_unittest.mm new file mode 100644 index 0000000..92ac48ee --- /dev/null +++ b/chrome/browser/ui/cocoa/browser_window_touch_bar_unittest.mm
@@ -0,0 +1,58 @@ +// 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. + +#import <Cocoa/Cocoa.h> + +#include "base/mac/mac_util.h" +#include "base/mac/scoped_nsobject.h" +#include "chrome/app/chrome_command_ids.h" +#import "chrome/browser/ui/cocoa/browser_window_touch_bar.h" +#include "chrome/browser/ui/cocoa/test/cocoa_profile_test.h" +#include "testing/gtest/include/gtest/gtest.h" + +class BrowserWindowTouchBarUnitTest : public CocoaProfileTest { + public: + void SetUp() override { + CocoaProfileTest::SetUp(); + ASSERT_TRUE(browser()); + browserWindowTouchBar_.reset( + [[BrowserWindowTouchBar alloc] initWithBrowser:browser()]); + } + + void TearDown() override { CocoaProfileTest::TearDown(); } + + base::scoped_nsobject<BrowserWindowTouchBar> browserWindowTouchBar_; +}; + +TEST_F(BrowserWindowTouchBarUnitTest, TouchBarItems) { + if (!base::mac::IsAtLeastOS10_12()) + return; + + NSTouchBar* touchBar = [browserWindowTouchBar_ makeTouchBar]; + NSArray* touchBarItemIds = [touchBar itemIdentifiers]; + EXPECT_TRUE([touchBarItemIds containsObject:@"BackForwardTouchId"]); + EXPECT_TRUE([touchBarItemIds containsObject:@"ReloadOrStopTouchId"]); + EXPECT_TRUE([touchBarItemIds containsObject:@"SearchTouchId"]); + EXPECT_TRUE([touchBarItemIds containsObject:@"NewTabTouchId"]); + EXPECT_TRUE([touchBarItemIds containsObject:@"StarTouchId"]); +} + +TEST_F(BrowserWindowTouchBarUnitTest, ReloadOrStopTouchBarItem) { + if (!base::mac::IsAtLeastOS10_12()) + return; + + NSTouchBar* touchBar = [browserWindowTouchBar_ makeTouchBar]; + [browserWindowTouchBar_ setIsPageLoading:NO]; + NSTouchBarItem* item = + [browserWindowTouchBar_ touchBar:touchBar + makeItemForIdentifier:@"ReloadOrStopTouchId"]; + + EXPECT_EQ(IDC_RELOAD, [[item view] tag]); + + [browserWindowTouchBar_ setIsPageLoading:YES]; + item = [browserWindowTouchBar_ touchBar:touchBar + makeItemForIdentifier:@"ReloadOrStopTouchId"]; + + EXPECT_EQ(IDC_STOP, [[item view] tag]); +}
diff --git a/chrome/browser/ui/cocoa/drag_util.h b/chrome/browser/ui/cocoa/drag_util.h index 04a0932..1060624 100644 --- a/chrome/browser/ui/cocoa/drag_util.h +++ b/chrome/browser/ui/cocoa/drag_util.h
@@ -26,7 +26,7 @@ // Returns a drag image for a bookmark. NSImage* DragImageForBookmark(NSImage* favicon, const base::string16& title, - CGFloat title_width); + CGFloat drag_image_width); } // namespace drag_util
diff --git a/chrome/browser/ui/cocoa/drag_util.mm b/chrome/browser/ui/cocoa/drag_util.mm index 4a3e00ea..0ce31db0 100644 --- a/chrome/browser/ui/cocoa/drag_util.mm +++ b/chrome/browser/ui/cocoa/drag_util.mm
@@ -7,9 +7,11 @@ #include <cmath> #include "base/files/file_path.h" +#include "base/i18n/rtl.h" #include "base/mac/scoped_nsobject.h" #include "base/strings/sys_string_conversions.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/cocoa/l10n_util.h" #include "components/mime_util/mime_util.h" #include "content/public/browser/plugin_service.h" #include "content/public/common/webplugininfo.h" @@ -59,24 +61,33 @@ // Draws string |title| within box |frame|, positioning it at the origin. // Truncates text with fading if it is too long to fit horizontally. // Based on code from GradientButtonCell but simplified where possible. -void DrawTruncatedTitle(NSAttributedString* title, NSRect frame) { +void DrawTruncatedTitle(NSAttributedString* title, + NSRect frame, + bool is_title_rtl) { NSSize size = [title size]; if (std::floor(size.width) <= NSWidth(frame)) { [title drawAtPoint:frame.origin]; return; } - // Gradient is about twice our line height long. + // The gradient is about twice the line height long. + NSRectEdge gradient_edge; + if (is_title_rtl) + gradient_edge = NSMinXEdge; + else + gradient_edge = NSMaxXEdge; + CGFloat gradient_width = std::min(size.height * 2, NSWidth(frame) / 4); NSRect solid_part, gradient_part; - NSDivideRect(frame, &gradient_part, &solid_part, gradient_width, NSMaxXEdge); + NSDivideRect(frame, &gradient_part, &solid_part, gradient_width, + gradient_edge); CGContextRef context = static_cast<CGContextRef>( [[NSGraphicsContext currentContext] graphicsPort]); CGContextBeginTransparencyLayerWithRect(context, NSRectToCGRect(frame), 0); { // Draw text clipped to frame. gfx::ScopedNSGraphicsContextSaveGState scoped_state; [NSBezierPath clipRect:frame]; - [title drawAtPoint:frame.origin]; + [title drawInRect:frame]; } NSColor* color = [NSColor blackColor]; @@ -85,10 +96,16 @@ [[NSGradient alloc] initWithStartingColor:color endingColor:alpha_color]); // Draw the gradient mask. CGContextSetBlendMode(context, kCGBlendModeDestinationIn); - [mask drawFromPoint:NSMakePoint(NSMaxX(frame) - gradient_width, - NSMinY(frame)) - toPoint:NSMakePoint(NSMaxX(frame), - NSMinY(frame)) + CGFloat gradient_x_start, gradient_x_end; + if (is_title_rtl) { + gradient_x_start = NSMaxX(gradient_part); + gradient_x_end = NSMinX(gradient_part); + } else { + gradient_x_start = NSMinX(gradient_part); + gradient_x_end = NSMaxX(gradient_part); + } + [mask drawFromPoint:NSMakePoint(gradient_x_start, NSMinY(frame)) + toPoint:NSMakePoint(gradient_x_end, NSMinY(frame)) options:NSGradientDrawsBeforeStartingLocation]; CGContextEndTransparencyLayer(context); } @@ -121,7 +138,7 @@ NSImage* DragImageForBookmark(NSImage* favicon, const base::string16& title, - CGFloat title_width) { + CGFloat drag_image_width) { // If no favicon, use a default. if (!favicon) { ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); @@ -142,25 +159,46 @@ [[NSAttributedString alloc] initWithString:ns_title attributes:attrs]); // Set up sizes and locations for rendering. - const CGFloat kIconMargin = 2.0; // Gap between icon and text. - CGFloat text_left = [favicon size].width + kIconMargin; - NSSize drag_image_size = [favicon size]; - NSSize text_size = [rich_title size]; - CGFloat max_text_width = title_width - text_left; - text_size.width = std::min(text_size.width, max_text_width); - drag_image_size.width = text_left + text_size.width; + const CGFloat kIconPadding = 2.0; // Gap between icon and text. + NSRect favicon_rect = {NSZeroPoint, [favicon size]}; + CGFloat icon_plus_padding_width = NSWidth(favicon_rect) + kIconPadding; + CGFloat full_text_width = [rich_title size].width; + CGFloat allowed_text_width = drag_image_width - icon_plus_padding_width; + CGFloat used_text_width = std::min(full_text_width, allowed_text_width); + NSRect full_drag_image_rect = NSMakeRect( + 0, 0, icon_plus_padding_width + used_text_width, NSHeight(favicon_rect)); + + NSRectEdge icon_edge; + if (cocoa_l10n_util::ShouldDoExperimentalRTLLayout()) + icon_edge = NSMaxXEdge; + else + icon_edge = NSMinXEdge; + + NSRect icon_rect; + NSRect text_plus_padding_rect; + NSRect padding_rect; + NSRect text_rect; + + // Slice off the icon. + NSDivideRect(full_drag_image_rect, &icon_rect, &text_plus_padding_rect, + NSWidth(favicon_rect), icon_edge); + + // Slice off the padding. + NSDivideRect(text_plus_padding_rect, &padding_rect, &text_rect, kIconPadding, + icon_edge); // Render the drag image. NSImage* drag_image = - [[[NSImage alloc] initWithSize:drag_image_size] autorelease]; + [[[NSImage alloc] initWithSize:full_drag_image_rect.size] autorelease]; [drag_image lockFocus]; - [favicon drawAtPoint:NSZeroPoint + [favicon drawAtPoint:icon_rect.origin fromRect:NSZeroRect operation:NSCompositeSourceOver fraction:0.7]; - NSRect target_text_rect = NSMakeRect(text_left, 0, - text_size.width, drag_image_size.height); - DrawTruncatedTitle(rich_title, target_text_rect); + + bool is_title_rtl = base::i18n::GetFirstStrongCharacterDirection(title) == + base::i18n::RIGHT_TO_LEFT; + DrawTruncatedTitle(rich_title, text_rect, is_title_rtl); [drag_image unlockFocus]; return drag_image;
diff --git a/chrome/browser/ui/cocoa/framed_browser_window.mm b/chrome/browser/ui/cocoa/framed_browser_window.mm index 65e14a2..8818003 100644 --- a/chrome/browser/ui/cocoa/framed_browser_window.mm +++ b/chrome/browser/ui/cocoa/framed_browser_window.mm
@@ -9,6 +9,7 @@ #include <stddef.h> #include "base/logging.h" +#include "base/mac/foundation_util.h" #include "base/mac/sdk_forward_declarations.h" #include "chrome/browser/global_keyboard_shortcuts_mac.h" #include "chrome/browser/profiles/profile_avatar_icon_util.h" @@ -16,6 +17,7 @@ #include "chrome/browser/themes/theme_service.h" #import "chrome/browser/ui/cocoa/browser_window_controller.h" #import "chrome/browser/ui/cocoa/browser_window_layout.h" +#import "chrome/browser/ui/cocoa/browser_window_touch_bar.h" #import "chrome/browser/ui/cocoa/browser_window_utils.h" #include "chrome/browser/ui/cocoa/l10n_util.h" #import "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h" @@ -24,7 +26,7 @@ #include "ui/base/cocoa/cocoa_base_utils.h" #include "ui/base/cocoa/nsgraphics_context_additions.h" #import "ui/base/cocoa/nsview_additions.h" -#include "ui/base/material_design/material_design_controller.h" +#import "ui/base/cocoa/touch_bar_forward_declarations.h" // Implementer's note: Moving the window controls is tricky. When altering the // code, ensure that: @@ -408,7 +410,8 @@ // width and some padding. The new avatar button is displayed to the right // of the fullscreen icon, so it doesn't need to be shifted. BrowserWindowController* bwc = - static_cast<BrowserWindowController*>([self windowController]); + base::mac::ObjCCastStrict<BrowserWindowController>( + [self windowController]); if ([bwc shouldShowAvatar] && ![bwc shouldUseNewAvatarButton]) { NSView* avatarButton = [[bwc avatarButtonController] view]; origin.x = -(NSWidth([avatarButton frame]) + 3); @@ -500,6 +503,13 @@ return themed; } +- (NSTouchBar*)makeTouchBar { + BrowserWindowController* bwc = + base::mac::ObjCCastStrict<BrowserWindowController>( + [self windowController]); + return [[bwc browserWindowTouchBar] makeTouchBar]; +} + - (NSColor*)titleColor { const ui::ThemeProvider* themeProvider = [self themeProvider]; if (!themeProvider)
diff --git a/chrome/browser/ui/cocoa/tabs/tab_view.mm b/chrome/browser/ui/cocoa/tabs/tab_view.mm index 39df6448..3804cfa 100644 --- a/chrome/browser/ui/cocoa/tabs/tab_view.mm +++ b/chrome/browser/ui/cocoa/tabs/tab_view.mm
@@ -10,6 +10,7 @@ #include "base/strings/sys_string_conversions.h" #include "chrome/browser/themes/theme_properties.h" #include "chrome/browser/themes/theme_service.h" +#include "chrome/browser/ui/cocoa/l10n_util.h" #import "chrome/browser/ui/cocoa/tabs/alert_indicator_button_cocoa.h" #import "chrome/browser/ui/cocoa/tabs/tab_controller.h" #import "chrome/browser/ui/cocoa/tabs/tab_window_controller.h" @@ -201,6 +202,8 @@ base::scoped_nsobject<GTMFadeTruncatingTextFieldCell> labelCell( [[GTMFadeTruncatingTextFieldCell alloc] initTextCell:@"Label"]); [labelCell setControlSize:NSSmallControlSize]; + if (cocoa_l10n_util::ShouldDoExperimentalRTLLayout()) + [labelCell setAlignment:NSRightTextAlignment]; [titleView_ setCell:labelCell]; titleViewCell_ = labelCell;
diff --git a/chrome/browser/ui/views/infobars/infobar_container_view.cc b/chrome/browser/ui/views/infobars/infobar_container_view.cc index a1e59e75..cb23d65 100644 --- a/chrome/browser/ui/views/infobars/infobar_container_view.cc +++ b/chrome/browser/ui/views/infobars/infobar_container_view.cc
@@ -122,6 +122,14 @@ node_data->SetName(l10n_util::GetStringUTF8(IDS_ACCNAME_INFOBAR_CONTAINER)); } +void InfoBarContainerView::OnNativeThemeChanged(const ui::NativeTheme* theme) { + // Infobars must be redrawn when the NativeTheme changes because + // they have a border with color COLOR_TOOLBAR_BOTTOM_SEPARATOR, + // which might have changed. + for (int i = 0; i < child_count(); ++i) + child_at(i)->SchedulePaint(); +} + void InfoBarContainerView::PlatformSpecificAddInfoBar( infobars::InfoBar* infobar, size_t position) {
diff --git a/chrome/browser/ui/views/infobars/infobar_container_view.h b/chrome/browser/ui/views/infobars/infobar_container_view.h index 3f178d2..b73ea89e 100644 --- a/chrome/browser/ui/views/infobars/infobar_container_view.h +++ b/chrome/browser/ui/views/infobars/infobar_container_view.h
@@ -26,6 +26,7 @@ const char* GetClassName() const override; void Layout() override; void GetAccessibleNodeData(ui::AXNodeData* node_data) override; + void OnNativeThemeChanged(const ui::NativeTheme* theme) override; // InfobarContainer: void PlatformSpecificAddInfoBar(infobars::InfoBar* infobar,
diff --git a/chrome/browser/ui/webui/chromeos/login/eula_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/eula_screen_handler.cc index e0a08b80..7f15ab01 100644 --- a/chrome/browser/ui/webui/chromeos/login/eula_screen_handler.cc +++ b/chrome/browser/ui/webui/chromeos/login/eula_screen_handler.cc
@@ -146,6 +146,7 @@ /* MD-OOBE */ builder->Add("oobeEulaSectionTitle", IDS_OOBE_EULA_SECTION_TITLE); + builder->Add("oobeEulaIframeLabel", IDS_OOBE_EULA_IFRAME_LABEL); builder->Add("oobeEulaAcceptAndContinueButtonText", IDS_OOBE_EULA_ACCEPT_AND_CONTINUE_BUTTON_TEXT); }
diff --git a/chrome/common/chrome_features.cc b/chrome/common/chrome_features.cc index ab7b093..62e7eb1b 100644 --- a/chrome/common/chrome_features.cc +++ b/chrome/common/chrome_features.cc
@@ -70,6 +70,12 @@ const base::Feature kBrowserHangFixesExperiment{ "BrowserHangFixesExperiment", base::FEATURE_DISABLED_BY_DEFAULT}; +#if defined(OS_MACOSX) +// Enables or disables the browser's touch bar. +const base::Feature kBrowserTouchBar{"BrowserTouchBar", + base::FEATURE_ENABLED_BY_DEFAULT}; +#endif // defined(OS_MACOSX) + #if defined(OS_ANDROID) // Experiment to make Geolocation permissions in the omnibox and the default // search engine's search page consistent.
diff --git a/chrome/common/chrome_features.h b/chrome/common/chrome_features.h index c632e65..6477e1f 100644 --- a/chrome/common/chrome_features.h +++ b/chrome/common/chrome_features.h
@@ -50,6 +50,10 @@ extern const base::Feature kBrowserHangFixesExperiment; +#if defined(OS_MACOSX) +extern const base::Feature kBrowserTouchBar; +#endif // defined(OS_MACOSX) + #if defined(OS_ANDROID) extern const base::Feature kConsistentOmniboxGeolocation; #endif
diff --git a/chrome/installer/mini_installer/chrome.release b/chrome/installer/mini_installer/chrome.release index 6176a03..5a603b1 100644 --- a/chrome/installer/mini_installer/chrome.release +++ b/chrome/installer/mini_installer/chrome.release
@@ -67,6 +67,8 @@ # WidevineCdm\manifest.json: %(VersionDir)s\WidevineCdm\ WidevineCdm\_platform_specific\win_x86\widevinecdm.dll: %(VersionDir)s\WidevineCdm\_platform_specific\win_x86\ +WidevineCdm\_platform_specific\win_x86\widevinecdm.dll.sig: %(VersionDir)s\WidevineCdm\_platform_specific\win_x86\ WidevineCdm\_platform_specific\win_x86\widevinecdmadapter.dll: %(VersionDir)s\WidevineCdm\_platform_specific\win_x86\ WidevineCdm\_platform_specific\win_x64\widevinecdm.dll: %(VersionDir)s\WidevineCdm\_platform_specific\win_x64\ +WidevineCdm\_platform_specific\win_x64\widevinecdm.dll.sig: %(VersionDir)s\WidevineCdm\_platform_specific\win_x64\ WidevineCdm\_platform_specific\win_x64\widevinecdmadapter.dll: %(VersionDir)s\WidevineCdm\_platform_specific\win_x64\
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 46a800e..4bab94b 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn
@@ -4574,6 +4574,7 @@ "../browser/ui/cocoa/browser_window_cocoa_unittest.mm", "../browser/ui/cocoa/browser_window_controller_unittest.mm", "../browser/ui/cocoa/browser_window_layout_unittest.mm", + "../browser/ui/cocoa/browser_window_touch_bar_unittest.mm", "../browser/ui/cocoa/browser_window_utils_unittest.mm", "../browser/ui/cocoa/bubble_anchor_helper_views_unittest.mm", "../browser/ui/cocoa/bubble_view_unittest.mm",
diff --git a/chrome/tools/build/win/FILES.cfg b/chrome/tools/build/win/FILES.cfg index f7d03ee..74ec2ab 100644 --- a/chrome/tools/build/win/FILES.cfg +++ b/chrome/tools/build/win/FILES.cfg
@@ -343,6 +343,11 @@ 'buildtype': ['official'], }, { + 'filename': 'WidevineCdm/_platform_specific/win_x86/widevinecdm.dll.sig', + 'arch': ['32bit'], + 'buildtype': ['official'], + }, + { 'filename': 'WidevineCdm/_platform_specific/win_x86/widevinecdmadapter.dll', 'arch': ['32bit'], 'buildtype': ['official'], @@ -353,6 +358,11 @@ 'buildtype': ['official'], }, { + 'filename': 'WidevineCdm/_platform_specific/win_x64/widevinecdm.dll.sig', + 'arch': ['64bit'], + 'buildtype': ['official'], + }, + { 'filename': 'WidevineCdm/_platform_specific/win_x64/widevinecdmadapter.dll', 'arch': ['64bit'], 'buildtype': ['official'],
diff --git a/device/usb/BUILD.gn b/device/usb/BUILD.gn index 9fb1235..76fa59e 100644 --- a/device/usb/BUILD.gn +++ b/device/usb/BUILD.gn
@@ -32,6 +32,8 @@ "usb_device_handle.h", "usb_device_handle_android.cc", "usb_device_handle_android.h", + "usb_device_handle_win.cc", + "usb_device_handle_win.h", "usb_device_linux.cc", "usb_device_linux.h", "usb_device_win.cc",
diff --git a/device/usb/usb_descriptors.cc b/device/usb/usb_descriptors.cc index 18316c58d..5ad332b 100644 --- a/device/usb/usb_descriptors.cc +++ b/device/usb/usb_descriptors.cc
@@ -82,10 +82,14 @@ std::unique_ptr<UsbDeviceDescriptor> desc, const base::Callback<void(std::unique_ptr<UsbDeviceDescriptor>)>& callback) { - if (desc->num_configurations == desc->configurations.size()) + if (desc->num_configurations == desc->configurations.size()) { callback.Run(std::move(desc)); - else + } else { + LOG(ERROR) << "Failed to read all configuration descriptors. Expected " + << static_cast<int>(desc->num_configurations) << ", got " + << desc->configurations.size() << "."; callback.Run(nullptr); + } } void OnReadConfigDescriptor(UsbDeviceDescriptor* desc, @@ -93,8 +97,14 @@ UsbTransferStatus status, scoped_refptr<IOBuffer> buffer, size_t length) { - if (status == USB_TRANSFER_COMPLETED) - desc->Parse(std::vector<uint8_t>(buffer->data(), buffer->data() + length)); + if (status == USB_TRANSFER_COMPLETED) { + if (!desc->Parse( + std::vector<uint8_t>(buffer->data(), buffer->data() + length))) { + LOG(ERROR) << "Failed to parse configuration descriptor."; + } + } else { + LOG(ERROR) << "Failed to read configuration descriptor."; + } closure.Run(); } @@ -116,6 +126,8 @@ kControlTransferTimeout, base::Bind(&OnReadConfigDescriptor, desc, closure)); } else { + LOG(ERROR) << "Failed to read length for configuration " + << static_cast<int>(index) << "."; closure.Run(); } } @@ -127,6 +139,7 @@ scoped_refptr<IOBuffer> buffer, size_t length) { if (status != USB_TRANSFER_COMPLETED) { + LOG(ERROR) << "Failed to read device descriptor."; callback.Run(nullptr); return; } @@ -134,6 +147,7 @@ std::unique_ptr<UsbDeviceDescriptor> desc(new UsbDeviceDescriptor()); if (!desc->Parse( std::vector<uint8_t>(buffer->data(), buffer->data() + length))) { + LOG(ERROR) << "Device descriptor parsing error."; callback.Run(nullptr); return; } @@ -430,6 +444,9 @@ vendor_id = data[8] | data[9] << 8; product_id = data[10] | data[11] << 8; device_version = data[12] | data[13] << 8; + i_manufacturer = data[14]; + i_product = data[15]; + i_serial_number = data[16]; num_configurations = data[17]; break; case kConfigurationDescriptorType:
diff --git a/device/usb/usb_descriptors.h b/device/usb/usb_descriptors.h index c25d51a2..9039c95 100644 --- a/device/usb/usb_descriptors.h +++ b/device/usb/usb_descriptors.h
@@ -133,6 +133,9 @@ uint16_t vendor_id = 0; uint16_t product_id = 0; uint16_t device_version = 0; + uint8_t i_manufacturer = 0; + uint8_t i_product = 0; + uint8_t i_serial_number = 0; uint8_t num_configurations = 0; std::vector<UsbConfigDescriptor> configurations; };
diff --git a/device/usb/usb_device.h b/device/usb/usb_device.h index 4dc115c..bc678f4 100644 --- a/device/usb/usb_device.h +++ b/device/usb/usb_device.h
@@ -132,6 +132,7 @@ friend class base::RefCountedThreadSafe<UsbDevice>; friend class UsbDeviceHandleImpl; friend class UsbDeviceHandleUsbfs; + friend class UsbDeviceHandleWin; friend class UsbServiceAndroid; friend class UsbServiceImpl; friend class UsbServiceLinux;
diff --git a/device/usb/usb_device_handle_win.cc b/device/usb/usb_device_handle_win.cc new file mode 100644 index 0000000..74a7de6 --- /dev/null +++ b/device/usb/usb_device_handle_win.cc
@@ -0,0 +1,349 @@ +// 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 "device/usb/usb_device_handle_win.h" + +#include <usbioctl.h> +#include <usbspec.h> +#include <winioctl.h> +#include <winusb.h> + +#include "base/bind.h" +#include "base/location.h" +#include "base/macros.h" +#include "base/memory/ptr_util.h" +#include "base/single_thread_task_runner.h" +#include "base/stl_util.h" +#include "base/strings/string16.h" +#include "base/synchronization/lock.h" +#include "base/threading/thread_task_runner_handle.h" +#include "base/win/object_watcher.h" +#include "components/device_event_log/device_event_log.h" +#include "device/usb/usb_context.h" +#include "device/usb/usb_descriptors.h" +#include "device/usb/usb_device_win.h" +#include "device/usb/usb_service.h" +#include "net/base/io_buffer.h" + +namespace device { + +// Encapsulates waiting for the completion of an overlapped event. +class UsbDeviceHandleWin::Request : public base::win::ObjectWatcher::Delegate { + public: + explicit Request(HANDLE handle) + : handle_(handle), event_(CreateEvent(nullptr, false, false, nullptr)) { + memset(&overlapped_, 0, sizeof(overlapped_)); + overlapped_.hEvent = event_.Get(); + } + + ~Request() override { + if (callback_) { + SetLastError(ERROR_REQUEST_ABORTED); + std::move(callback_).Run(this, false, 0); + } + } + + // Starts watching for completion of the overlapped event. + void MaybeStartWatching( + BOOL success, + base::OnceCallback<void(Request*, DWORD, size_t)> callback) { + callback_ = std::move(callback); + if (success) { + OnObjectSignaled(event_.Get()); + } else { + DWORD error = GetLastError(); + if (error == ERROR_IO_PENDING) { + watcher_.StartWatchingOnce(event_.Get(), this); + } else { + std::move(callback_).Run(this, error, 0); + } + } + } + + OVERLAPPED* overlapped() { return &overlapped_; } + + // base::win::ObjectWatcher::Delegate + void OnObjectSignaled(HANDLE object) override { + DCHECK_EQ(object, event_.Get()); + DWORD size; + if (GetOverlappedResult(handle_, &overlapped_, &size, true)) + std::move(callback_).Run(this, ERROR_SUCCESS, size); + else + std::move(callback_).Run(this, GetLastError(), 0); + } + + private: + HANDLE handle_; + OVERLAPPED overlapped_; + base::win::ScopedHandle event_; + base::win::ObjectWatcher watcher_; + base::OnceCallback<void(Request*, DWORD, size_t)> callback_; + + DISALLOW_COPY_AND_ASSIGN(Request); +}; + +scoped_refptr<UsbDevice> UsbDeviceHandleWin::GetDevice() const { + return device_; +} + +void UsbDeviceHandleWin::Close() { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!device_) + return; + + device_->HandleClosed(this); + device_ = nullptr; + + if (hub_handle_.IsValid()) { + CancelIo(hub_handle_.Get()); + hub_handle_.Close(); + } + + requests_.clear(); +} + +void UsbDeviceHandleWin::SetConfiguration(int configuration_value, + const ResultCallback& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!device_) { + callback.Run(false); + return; + } +} + +void UsbDeviceHandleWin::ClaimInterface(int interface_number, + const ResultCallback& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!device_) { + callback.Run(false); + return; + } +} + +void UsbDeviceHandleWin::ReleaseInterface(int interface_number, + const ResultCallback& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!device_) { + task_runner_->PostTask(FROM_HERE, base::Bind(callback, false)); + return; + } +} + +void UsbDeviceHandleWin::SetInterfaceAlternateSetting( + int interface_number, + int alternate_setting, + const ResultCallback& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!device_) { + callback.Run(false); + return; + } +} + +void UsbDeviceHandleWin::ResetDevice(const ResultCallback& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!device_) { + callback.Run(false); + return; + } +} + +void UsbDeviceHandleWin::ClearHalt(uint8_t endpoint, + const ResultCallback& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!device_) { + callback.Run(false); + return; + } +} + +void UsbDeviceHandleWin::ControlTransfer(UsbEndpointDirection direction, + TransferRequestType request_type, + TransferRecipient recipient, + uint8_t request, + uint16_t value, + uint16_t index, + scoped_refptr<net::IOBuffer> buffer, + size_t length, + unsigned int timeout, + const TransferCallback& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); + + if (!device_) { + task_runner_->PostTask( + FROM_HERE, base::Bind(callback, USB_TRANSFER_DISCONNECT, nullptr, 0)); + return; + } + + if (hub_handle_.IsValid()) { + if (direction == USB_DIRECTION_INBOUND && + request_type == TransferRequestType::STANDARD && + recipient == TransferRecipient::DEVICE && + request == USB_REQUEST_GET_DESCRIPTOR) { + if ((value >> 8) == USB_DEVICE_DESCRIPTOR_TYPE) { + auto* node_connection_info = new USB_NODE_CONNECTION_INFORMATION_EX; + node_connection_info->ConnectionIndex = device_->port_number(); + + Request* request = MakeRequest(hub_handle_.Get()); + request->MaybeStartWatching( + DeviceIoControl(hub_handle_.Get(), + IOCTL_USB_GET_NODE_CONNECTION_INFORMATION_EX, + node_connection_info, sizeof(*node_connection_info), + node_connection_info, sizeof(*node_connection_info), + nullptr, request->overlapped()), + base::Bind(&UsbDeviceHandleWin::GotNodeConnectionInformation, + weak_factory_.GetWeakPtr(), callback, + base::Owned(node_connection_info), buffer, length)); + return; + } else if (((value >> 8) == USB_CONFIGURATION_DESCRIPTOR_TYPE) || + ((value >> 8) == USB_STRING_DESCRIPTOR_TYPE)) { + size_t size = sizeof(USB_DESCRIPTOR_REQUEST) + length; + scoped_refptr<net::IOBuffer> request_buffer(new net::IOBuffer(size)); + USB_DESCRIPTOR_REQUEST* descriptor_request = + reinterpret_cast<USB_DESCRIPTOR_REQUEST*>(request_buffer->data()); + descriptor_request->ConnectionIndex = device_->port_number(); + descriptor_request->SetupPacket.bmRequest = BMREQUEST_DEVICE_TO_HOST; + descriptor_request->SetupPacket.bRequest = USB_REQUEST_GET_DESCRIPTOR; + descriptor_request->SetupPacket.wValue = value; + descriptor_request->SetupPacket.wIndex = index; + descriptor_request->SetupPacket.wLength = length; + + Request* request = MakeRequest(hub_handle_.Get()); + request->MaybeStartWatching( + DeviceIoControl(hub_handle_.Get(), + IOCTL_USB_GET_DESCRIPTOR_FROM_NODE_CONNECTION, + request_buffer->data(), size, + request_buffer->data(), size, nullptr, + request->overlapped()), + base::Bind(&UsbDeviceHandleWin::GotDescriptorFromNodeConnection, + weak_factory_.GetWeakPtr(), callback, request_buffer, + buffer, length)); + return; + } + } + + // Unsupported transfer for hub. + task_runner_->PostTask( + FROM_HERE, base::Bind(callback, USB_TRANSFER_ERROR, nullptr, 0)); + return; + } + + // Regular control transfers unimplemented. + task_runner_->PostTask(FROM_HERE, + base::Bind(callback, USB_TRANSFER_ERROR, nullptr, 0)); +} + +void UsbDeviceHandleWin::IsochronousTransferIn( + uint8_t endpoint_number, + const std::vector<uint32_t>& packet_lengths, + unsigned int timeout, + const IsochronousTransferCallback& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); +} + +void UsbDeviceHandleWin::IsochronousTransferOut( + uint8_t endpoint_number, + scoped_refptr<net::IOBuffer> buffer, + const std::vector<uint32_t>& packet_lengths, + unsigned int timeout, + const IsochronousTransferCallback& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); +} + +void UsbDeviceHandleWin::GenericTransfer(UsbEndpointDirection direction, + uint8_t endpoint_number, + scoped_refptr<net::IOBuffer> buffer, + size_t length, + unsigned int timeout, + const TransferCallback& callback) { + // This one must be callable from any thread. +} + +const UsbInterfaceDescriptor* UsbDeviceHandleWin::FindInterfaceByEndpoint( + uint8_t endpoint_address) { + DCHECK(thread_checker_.CalledOnValidThread()); + return nullptr; +} + +UsbDeviceHandleWin::UsbDeviceHandleWin( + scoped_refptr<UsbDeviceWin> device, + base::win::ScopedHandle handle, + scoped_refptr<base::SequencedTaskRunner> blocking_task_runner) + : device_(std::move(device)), + hub_handle_(std::move(handle)), + task_runner_(base::ThreadTaskRunnerHandle::Get()), + blocking_task_runner_(std::move(blocking_task_runner)), + weak_factory_(this) {} + +UsbDeviceHandleWin::~UsbDeviceHandleWin() {} + +UsbDeviceHandleWin::Request* UsbDeviceHandleWin::MakeRequest(HANDLE handle) { + auto request = base::MakeUnique<Request>(hub_handle_.Get()); + Request* request_ptr = request.get(); + requests_[request_ptr] = std::move(request); + return request_ptr; +} + +std::unique_ptr<UsbDeviceHandleWin::Request> UsbDeviceHandleWin::UnlinkRequest( + UsbDeviceHandleWin::Request* request_ptr) { + auto it = requests_.find(request_ptr); + DCHECK(it != requests_.end()); + std::unique_ptr<Request> request = std::move(it->second); + requests_.erase(it); + return request; +} + +void UsbDeviceHandleWin::GotNodeConnectionInformation( + const TransferCallback& callback, + void* node_connection_info_ptr, + scoped_refptr<net::IOBuffer> buffer, + size_t buffer_length, + Request* request_ptr, + DWORD win32_result, + size_t bytes_transferred) { + USB_NODE_CONNECTION_INFORMATION_EX* node_connection_info = + static_cast<USB_NODE_CONNECTION_INFORMATION_EX*>( + node_connection_info_ptr); + std::unique_ptr<Request> request = UnlinkRequest(request_ptr); + + if (win32_result != ERROR_SUCCESS) { + SetLastError(win32_result); + USB_PLOG(ERROR) << "Failed to get node connection information"; + callback.Run(USB_TRANSFER_ERROR, nullptr, 0); + return; + } + + DCHECK_EQ(bytes_transferred, sizeof(USB_NODE_CONNECTION_INFORMATION_EX)); + bytes_transferred = std::min(sizeof(USB_DEVICE_DESCRIPTOR), buffer_length); + memcpy(buffer->data(), &node_connection_info->DeviceDescriptor, + bytes_transferred); + callback.Run(USB_TRANSFER_COMPLETED, buffer, bytes_transferred); +} + +void UsbDeviceHandleWin::GotDescriptorFromNodeConnection( + const TransferCallback& callback, + scoped_refptr<net::IOBuffer> request_buffer, + scoped_refptr<net::IOBuffer> original_buffer, + size_t original_buffer_length, + Request* request_ptr, + DWORD win32_result, + size_t bytes_transferred) { + std::unique_ptr<Request> request = UnlinkRequest(request_ptr); + + if (win32_result != ERROR_SUCCESS) { + SetLastError(win32_result); + USB_PLOG(ERROR) << "Failed to read descriptor from node connection"; + callback.Run(USB_TRANSFER_ERROR, nullptr, 0); + return; + } + + DCHECK_GE(bytes_transferred, sizeof(USB_DESCRIPTOR_REQUEST)); + bytes_transferred -= sizeof(USB_DESCRIPTOR_REQUEST); + memcpy(original_buffer->data(), + request_buffer->data() + sizeof(USB_DESCRIPTOR_REQUEST), + bytes_transferred); + callback.Run(USB_TRANSFER_COMPLETED, original_buffer, bytes_transferred); +} + +} // namespace device
diff --git a/device/usb/usb_device_handle_win.h b/device/usb/usb_device_handle_win.h new file mode 100644 index 0000000..69e6e28 --- /dev/null +++ b/device/usb/usb_device_handle_win.h
@@ -0,0 +1,133 @@ +// 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 DEVICE_USB_USB_DEVICE_HANDLE_WIN_H_ +#define DEVICE_USB_USB_DEVICE_HANDLE_WIN_H_ + +#include <unordered_map> +#include <vector> + +#include "base/callback.h" +#include "base/macros.h" +#include "base/memory/ref_counted.h" +#include "base/memory/weak_ptr.h" +#include "base/threading/thread_checker.h" +#include "base/win/scoped_handle.h" +#include "device/usb/usb_device_handle.h" + +namespace base { +class SequencedTaskRunner; +class SingleThreadTaskRunner; +} + +namespace net { +class IOBuffer; +} + +namespace device { + +class UsbDeviceWin; + +// UsbDeviceHandle class provides basic I/O related functionalities. +class UsbDeviceHandleWin : public UsbDeviceHandle { + public: + scoped_refptr<UsbDevice> GetDevice() const override; + void Close() override; + void SetConfiguration(int configuration_value, + const ResultCallback& callback) override; + void ClaimInterface(int interface_number, + const ResultCallback& callback) override; + void ReleaseInterface(int interface_number, + const ResultCallback& callback) override; + void SetInterfaceAlternateSetting(int interface_number, + int alternate_setting, + const ResultCallback& callback) override; + void ResetDevice(const ResultCallback& callback) override; + void ClearHalt(uint8_t endpoint, const ResultCallback& callback) override; + + void ControlTransfer(UsbEndpointDirection direction, + TransferRequestType request_type, + TransferRecipient recipient, + uint8_t request, + uint16_t value, + uint16_t index, + scoped_refptr<net::IOBuffer> buffer, + size_t length, + unsigned int timeout, + const TransferCallback& callback) override; + + void IsochronousTransferIn( + uint8_t endpoint, + const std::vector<uint32_t>& packet_lengths, + unsigned int timeout, + const IsochronousTransferCallback& callback) override; + + void IsochronousTransferOut( + uint8_t endpoint, + scoped_refptr<net::IOBuffer> buffer, + const std::vector<uint32_t>& packet_lengths, + unsigned int timeout, + const IsochronousTransferCallback& callback) override; + + void GenericTransfer(UsbEndpointDirection direction, + uint8_t endpoint_number, + scoped_refptr<net::IOBuffer> buffer, + size_t length, + unsigned int timeout, + const TransferCallback& callback) override; + const UsbInterfaceDescriptor* FindInterfaceByEndpoint( + uint8_t endpoint_address) override; + + protected: + friend class UsbDeviceWin; + + // Constructor used to build a connection to the device's parent hub. + UsbDeviceHandleWin( + scoped_refptr<UsbDeviceWin> device, + base::win::ScopedHandle handle, + scoped_refptr<base::SequencedTaskRunner> blocking_task_runner); + + ~UsbDeviceHandleWin() override; + + private: + class Request; + + Request* MakeRequest(HANDLE handle); + std::unique_ptr<Request> UnlinkRequest(Request* request); + void GotNodeConnectionInformation(const TransferCallback& callback, + void* node_connection_info, + scoped_refptr<net::IOBuffer> buffer, + size_t buffer_length, + Request* request_ptr, + DWORD win32_result, + size_t bytes_transferred); + void GotDescriptorFromNodeConnection( + const TransferCallback& callback, + scoped_refptr<net::IOBuffer> request_buffer, + scoped_refptr<net::IOBuffer> original_buffer, + size_t original_buffer_length, + Request* request_ptr, + DWORD win32_result, + size_t bytes_transferred); + + base::ThreadChecker thread_checker_; + + scoped_refptr<UsbDeviceWin> device_; + + // |hub_handle_| must outlive |requests_| because individual Request objects + // hold on to the handle for the purpose of calling GetOverlappedResult(). + base::win::ScopedHandle hub_handle_; + std::unordered_map<Request*, std::unique_ptr<Request>> requests_; + + scoped_refptr<base::SingleThreadTaskRunner> task_runner_; + scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_; + + base::WeakPtrFactory<UsbDeviceHandleWin> weak_factory_; + + DISALLOW_COPY_AND_ASSIGN(UsbDeviceHandleWin); +}; + +} // namespace device + +#endif // DEVICE_USB_USB_DEVICE_HANDLE_WIN_H_
diff --git a/device/usb/usb_device_win.cc b/device/usb/usb_device_win.cc index 172858d..e54d1c88 100644 --- a/device/usb/usb_device_win.cc +++ b/device/usb/usb_device_win.cc
@@ -10,7 +10,7 @@ #include "base/single_thread_task_runner.h" #include "base/threading/thread_task_runner_handle.h" #include "components/device_event_log/device_event_log.h" -#include "device/usb/usb_device_handle.h" +#include "device/usb/usb_device_handle_win.h" namespace device { @@ -32,4 +32,67 @@ task_runner_->PostTask(FROM_HERE, base::Bind(callback, nullptr)); } +void UsbDeviceWin::ReadDescriptors(const base::Callback<void(bool)>& callback) { + DCHECK(thread_checker_.CalledOnValidThread()); + scoped_refptr<UsbDeviceHandle> device_handle; + base::win::ScopedHandle handle( + CreateFileA(hub_path_.c_str(), GENERIC_WRITE, FILE_SHARE_WRITE, nullptr, + OPEN_EXISTING, FILE_FLAG_OVERLAPPED, nullptr)); + if (handle.IsValid()) { + device_handle = + new UsbDeviceHandleWin(this, std::move(handle), blocking_task_runner_); + } else { + USB_PLOG(ERROR) << "Failed to open " << hub_path_; + callback.Run(false); + return; + } + + ReadUsbDescriptors(device_handle, base::Bind(&UsbDeviceWin::OnReadDescriptors, + this, callback, device_handle)); +} + +void UsbDeviceWin::OnReadDescriptors( + const base::Callback<void(bool)>& callback, + scoped_refptr<UsbDeviceHandle> device_handle, + std::unique_ptr<UsbDeviceDescriptor> descriptor) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!descriptor) { + USB_LOG(ERROR) << "Failed to read descriptors from " << device_path_ << "."; + device_handle->Close(); + callback.Run(false); + return; + } + + descriptor_ = *descriptor; + + auto string_map = base::MakeUnique<std::map<uint8_t, base::string16>>(); + if (descriptor_.i_manufacturer) + (*string_map)[descriptor_.i_manufacturer] = base::string16(); + if (descriptor_.i_product) + (*string_map)[descriptor_.i_product] = base::string16(); + if (descriptor_.i_serial_number) + (*string_map)[descriptor_.i_serial_number] = base::string16(); + + ReadUsbStringDescriptors(device_handle, std::move(string_map), + base::Bind(&UsbDeviceWin::OnReadStringDescriptors, + this, callback, device_handle)); +} + +void UsbDeviceWin::OnReadStringDescriptors( + const base::Callback<void(bool)>& callback, + scoped_refptr<UsbDeviceHandle> device_handle, + std::unique_ptr<std::map<uint8_t, base::string16>> string_map) { + DCHECK(thread_checker_.CalledOnValidThread()); + device_handle->Close(); + + if (descriptor_.i_manufacturer) + manufacturer_string_ = (*string_map)[descriptor_.i_manufacturer]; + if (descriptor_.i_product) + product_string_ = (*string_map)[descriptor_.i_product]; + if (descriptor_.i_serial_number) + serial_number_ = (*string_map)[descriptor_.i_serial_number]; + + callback.Run(true); +} + } // namespace device
diff --git a/device/usb/usb_device_win.h b/device/usb/usb_device_win.h index 0a74c06..2c43a52 100644 --- a/device/usb/usb_device_win.h +++ b/device/usb/usb_device_win.h
@@ -17,6 +17,8 @@ namespace device { +struct UsbDeviceDescriptor; + class UsbDeviceWin : public UsbDevice { public: // UsbDevice implementation: @@ -37,6 +39,20 @@ const std::string& device_path() const { return device_path_; } int port_number() const { return port_number_; } + // Opens the device's parent hub in order to read the device, configuration + // and string descriptors. + void ReadDescriptors(const base::Callback<void(bool)>& callback); + + private: + void OpenOnBlockingThread(const OpenCallback& callback); + void OnReadDescriptors(const base::Callback<void(bool)>& callback, + scoped_refptr<UsbDeviceHandle> device_handle, + std::unique_ptr<UsbDeviceDescriptor> descriptor); + void OnReadStringDescriptors( + const base::Callback<void(bool)>& callback, + scoped_refptr<UsbDeviceHandle> device_handle, + std::unique_ptr<std::map<uint8_t, base::string16>> string_map); + private: base::ThreadChecker thread_checker_;
diff --git a/device/usb/usb_service_win.cc b/device/usb/usb_service_win.cc index 131fdb33..65de4b0 100644 --- a/device/usb/usb_service_win.cc +++ b/device/usb/usb_service_win.cc
@@ -334,12 +334,11 @@ if (!enumeration_ready()) ++first_enumeration_countdown_; - scoped_refptr<UsbDeviceWin> device(new UsbDeviceWin( - device_path, hub_path, port_number, blocking_task_runner())); + scoped_refptr<UsbDeviceWin> device( + new UsbDeviceWin(device_path, hub_path, port_number, task_runner())); devices_by_path_[device->device_path()] = device; - - // TODO(reillyg): Read device descriptors. - DeviceReady(device, true); + device->ReadDescriptors(base::Bind(&UsbServiceWin::DeviceReady, + weak_factory_.GetWeakPtr(), device)); } void UsbServiceWin::DeviceReady(scoped_refptr<UsbDeviceWin> device,
diff --git a/net/nqe/network_quality_estimator.cc b/net/nqe/network_quality_estimator.cc index 1e0b7a64c..7adb2ac2 100644 --- a/net/nqe/network_quality_estimator.cc +++ b/net/nqe/network_quality_estimator.cc
@@ -415,8 +415,6 @@ effective_connection_type_at_last_main_frame_ = effective_connection_type_; estimated_quality_at_last_main_frame_ = network_quality_; - RecordMetricsOnMainFrameRequest(); - // Post the tasks which will run in the future and record the estimation // accuracy based on the observations received between now and the time of // task execution. Posting the task at different intervals makes it @@ -444,8 +442,10 @@ return; } - if (request.load_flags() & LOAD_MAIN_FRAME_DEPRECATED) + if (request.load_flags() & LOAD_MAIN_FRAME_DEPRECATED) { + RecordMetricsOnMainFrameRequest(); MaybeQueryExternalEstimateProvider(); + } LoadTimingInfo load_timing_info; request.GetLoadTimingInfo(&load_timing_info);
diff --git a/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/4776765-expected.txt b/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/4776765-expected.txt index bf3323c..98491b5 100644 --- a/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/4776765-expected.txt +++ b/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/4776765-expected.txt
@@ -30,6 +30,10 @@ "reason": "border box change" }, { + "object": "Caret", + "reason": "caret" + }, + { "object": "LayoutBlockFlow DIV id='div'", "reason": "layoutObject insertion" },
diff --git a/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt b/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt index bf3323c..98491b5 100644 --- a/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt +++ b/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt
@@ -30,6 +30,10 @@ "reason": "border box change" }, { + "object": "Caret", + "reason": "caret" + }, + { "object": "LayoutBlockFlow DIV id='div'", "reason": "layoutObject insertion" },
diff --git a/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/4776765-expected.txt b/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/4776765-expected.txt index 24d7fa1..18afe0c 100644 --- a/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/4776765-expected.txt +++ b/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/4776765-expected.txt
@@ -30,6 +30,10 @@ "reason": "border box change" }, { + "object": "Caret", + "reason": "caret" + }, + { "object": "LayoutBlockFlow DIV id='div'", "reason": "layoutObject insertion" },
diff --git a/third_party/WebKit/LayoutTests/platform/mac/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt b/third_party/WebKit/LayoutTests/platform/mac/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt index 24d7fa1..18afe0c 100644 --- a/third_party/WebKit/LayoutTests/platform/mac/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt +++ b/third_party/WebKit/LayoutTests/platform/mac/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt
@@ -30,6 +30,10 @@ "reason": "border box change" }, { + "object": "Caret", + "reason": "caret" + }, + { "object": "LayoutBlockFlow DIV id='div'", "reason": "layoutObject insertion" },
diff --git a/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/4776765-expected.txt b/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/4776765-expected.txt index 782dcd5..cd7d99a 100644 --- a/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/4776765-expected.txt +++ b/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/4776765-expected.txt
@@ -30,6 +30,10 @@ "reason": "border box change" }, { + "object": "Caret", + "reason": "caret" + }, + { "object": "LayoutBlockFlow DIV id='div'", "reason": "layoutObject insertion" },
diff --git a/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt b/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt index 782dcd5..cd7d99a 100644 --- a/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt +++ b/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/paint/invalidation/4776765-expected.txt
@@ -30,6 +30,10 @@ "reason": "border box change" }, { + "object": "Caret", + "reason": "caret" + }, + { "object": "LayoutBlockFlow DIV id='div'", "reason": "layoutObject insertion" },
diff --git a/third_party/WebKit/Source/core/BUILD.gn b/third_party/WebKit/Source/core/BUILD.gn index ac03242..eb820f2 100644 --- a/third_party/WebKit/Source/core/BUILD.gn +++ b/third_party/WebKit/Source/core/BUILD.gn
@@ -1189,6 +1189,7 @@ "dom/custom/CustomElementUpgradeSorterTest.cpp", "dom/shadow/FlatTreeTraversalTest.cpp", "dom/shadow/SlotScopedTraversalTest.cpp", + "editing/CaretDisplayItemClientTest.cpp", "events/EventPathTest.cpp", "events/EventTargetTest.cpp", "events/PointerEventFactoryTest.cpp", @@ -1327,7 +1328,6 @@ "page/WindowFeaturesTest.cpp", "page/scrolling/ScrollStateTest.cpp", "page/scrolling/SnapCoordinatorTest.cpp", - "paint/BlockPaintInvalidatorTest.cpp", "paint/BoxPaintInvalidatorTest.cpp", "paint/FirstMeaningfulPaintDetectorTest.cpp", "paint/HTMLCanvasPainterTest.cpp",
diff --git a/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp b/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp index cbd1bef..7d20faa 100644 --- a/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp +++ b/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp
@@ -117,15 +117,17 @@ void CaretDisplayItemClient::updateStyleAndLayoutIfNeeded( const PositionWithAffinity& caretPosition) { + m_previousLayoutBlock = m_layoutBlock; + m_previousVisualRect = m_visualRect; + LayoutBlock* newLayoutBlock = caretLayoutBlock(caretPosition.anchorNode()); if (newLayoutBlock != m_layoutBlock) { if (m_layoutBlock) m_layoutBlock->setMayNeedPaintInvalidation(); - m_previousLayoutBlock = m_layoutBlock; m_layoutBlock = newLayoutBlock; - m_needsPaintInvalidation = true; - } else { - m_previousLayoutBlock = nullptr; + if (newLayoutBlock) + m_needsPaintInvalidation = true; + m_visualRect = LayoutRect(); } if (!newLayoutBlock) { @@ -158,39 +160,44 @@ const LayoutBlock& block, const PaintInvalidatorContext& context, PaintInvalidationReason layoutBlockPaintInvalidationReason) { - if (block == m_previousLayoutBlock) { - // Invalidate the previous caret if it was in a different block. - // m_previousLayoutBlock is set only when it's different from m_layoutBlock. - DCHECK(block != m_layoutBlock); - - ObjectPaintInvalidatorWithContext objectInvalidator(*m_previousLayoutBlock, - context); - if (!isImmediateFullPaintInvalidationReason( - layoutBlockPaintInvalidationReason)) { - objectInvalidator.fullyInvalidatePaint(PaintInvalidationCaret, - m_visualRect, LayoutRect()); - } - - // If m_layoutBlock is not null, the following will be done when - // the new caret is invalidated in m_layoutBlock. - if (!m_layoutBlock) { - context.paintingLayer->setNeedsRepaint(); - objectInvalidator.invalidateDisplayItemClient(*this, - PaintInvalidationCaret); - m_visualRect = LayoutRect(); - m_needsPaintInvalidation = false; - } - - m_previousLayoutBlock = nullptr; + if (block == m_layoutBlock) { + invalidatePaintInCurrentLayoutBlock(context, + layoutBlockPaintInvalidationReason); return; } - if (block != m_layoutBlock) - return; + if (block == m_previousLayoutBlock) { + invalidatePaintInPreviousLayoutBlock(context, + layoutBlockPaintInvalidationReason); + } +} - // Invalidate the new caret, and the old caret if it was in the same block. +void CaretDisplayItemClient::invalidatePaintInPreviousLayoutBlock( + const PaintInvalidatorContext& context, + PaintInvalidationReason layoutBlockPaintInvalidationReason) { + DCHECK(m_previousLayoutBlock); + + ObjectPaintInvalidatorWithContext objectInvalidator(*m_previousLayoutBlock, + context); + if (!isImmediateFullPaintInvalidationReason( + layoutBlockPaintInvalidationReason)) { + objectInvalidator.invalidatePaintUsingContainer( + *context.paintInvalidationContainer, m_previousVisualRect, + PaintInvalidationCaret); + } + + context.paintingLayer->setNeedsRepaint(); + objectInvalidator.invalidateDisplayItemClient(*this, PaintInvalidationCaret); + m_previousLayoutBlock = nullptr; +} + +void CaretDisplayItemClient::invalidatePaintInCurrentLayoutBlock( + const PaintInvalidatorContext& context, + PaintInvalidationReason layoutBlockPaintInvalidationReason) { + DCHECK(m_layoutBlock); + LayoutRect newVisualRect; - if (m_layoutBlock && !m_localRect.isEmpty()) { + if (!m_localRect.isEmpty()) { newVisualRect = m_localRect; context.mapLocalRectToPaintInvalidationBacking(*m_layoutBlock, newVisualRect); @@ -204,22 +211,21 @@ } } - if (m_needsPaintInvalidation || newVisualRect != m_visualRect) { - m_needsPaintInvalidation = false; + if (!m_needsPaintInvalidation && newVisualRect == m_visualRect) + return; - ObjectPaintInvalidatorWithContext objectInvalidator(*m_layoutBlock, - context); - if (!isImmediateFullPaintInvalidationReason( - layoutBlockPaintInvalidationReason)) { - objectInvalidator.fullyInvalidatePaint(PaintInvalidationCaret, - m_visualRect, newVisualRect); - } + m_needsPaintInvalidation = false; - context.paintingLayer->setNeedsRepaint(); - objectInvalidator.invalidateDisplayItemClient(*this, - PaintInvalidationCaret); + ObjectPaintInvalidatorWithContext objectInvalidator(*m_layoutBlock, context); + if (!isImmediateFullPaintInvalidationReason( + layoutBlockPaintInvalidationReason)) { + objectInvalidator.fullyInvalidatePaint(PaintInvalidationCaret, m_visualRect, + newVisualRect); } + context.paintingLayer->setNeedsRepaint(); + objectInvalidator.invalidateDisplayItemClient(*this, PaintInvalidationCaret); + m_visualRect = newVisualRect; }
diff --git a/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.h b/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.h index 2c740f5..576381e 100644 --- a/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.h +++ b/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.h
@@ -91,19 +91,33 @@ String debugName() const final; private: + void invalidatePaintInCurrentLayoutBlock( + const PaintInvalidatorContext&, + PaintInvalidationReason layoutBlockPaintInvalidationReason); + + void invalidatePaintInPreviousLayoutBlock( + const PaintInvalidatorContext&, + PaintInvalidationReason layoutBlockPaintInvalidationReason); + // These are updated by updateStyleAndLayoutIfNeeded(). Color m_color; LayoutRect m_localRect; LayoutBlock* m_layoutBlock = nullptr; - // This is set to the previous m_layoutBlock if m_layoutLayout will change - // during updateStyleAndLayoutIfNeeded() and can be used in - // invalidatePaintIfNeeded() only. + // This is set to the previous value of m_layoutBlock during + // updateStyleAndLayoutIfNeeded() and can be used in invalidatePaintIfNeeded() + // only. const LayoutBlock* m_previousLayoutBlock = nullptr; - // This is updated by invalidatePaintIfNeeded(). + // Visual rect of the caret in m_layoutBlock. This is updated by + // invalidatePaintIfNeeded(). LayoutRect m_visualRect; + // This is set to the previous value of m_visualRect during + // updateStyleAndLayoutIfNeeded() and can be used in invalidatePaintIfNeeded() + // only. + LayoutRect m_previousVisualRect; + bool m_needsPaintInvalidation = false; };
diff --git a/third_party/WebKit/Source/core/editing/CaretDisplayItemClientTest.cpp b/third_party/WebKit/Source/core/editing/CaretDisplayItemClientTest.cpp new file mode 100644 index 0000000..141f5a4 --- /dev/null +++ b/third_party/WebKit/Source/core/editing/CaretDisplayItemClientTest.cpp
@@ -0,0 +1,217 @@ +// 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 "core/HTMLNames.h" +#include "core/editing/FrameSelection.h" +#include "core/frame/FrameView.h" +#include "core/layout/LayoutTestHelper.h" +#include "core/layout/LayoutView.h" +#include "core/page/FocusController.h" +#include "core/paint/PaintLayer.h" +#include "platform/graphics/GraphicsLayer.h" +#include "platform/graphics/paint/RasterInvalidationTracking.h" + +namespace blink { + +class CaretDisplayItemClientTest : public RenderingTest { + protected: + void SetUp() override { + RenderingTest::SetUp(); + enableCompositing(); + } + + const RasterInvalidationTracking* getRasterInvalidationTracking() const { + // TODO(wangxianzhu): Test SPv2. + return layoutView() + .layer() + ->graphicsLayerBacking() + ->getRasterInvalidationTracking(); + } + + FrameSelection& selection() const { + return document().view()->frame().selection(); + } + + const DisplayItemClient& caretDisplayItemClient() const { + return selection().caretDisplayItemClientForTesting(); + } + + Text* appendTextNode(const String& data) { + Text* text = document().createTextNode(data); + document().body()->appendChild(text); + return text; + } + + Element* appendBlock(const String& data) { + Element* block = document().createElement("div"); + Text* text = document().createTextNode(data); + block->appendChild(text); + document().body()->appendChild(block); + return block; + } +}; + +TEST_F(CaretDisplayItemClientTest, CaretPaintInvalidation) { + document().body()->setContentEditable("true", ASSERT_NO_EXCEPTION); + document().page()->focusController().setActive(true); + document().page()->focusController().setFocused(true); + + Text* text = appendTextNode("Hello, World!"); + document().view()->updateAllLifecyclePhases(); + const auto* block = toLayoutBlock(document().body()->layoutObject()); + + // Focus the body. Should invalidate the new caret. + document().view()->setTracksPaintInvalidations(true); + document().body()->focus(); + document().view()->updateAllLifecyclePhases(); + EXPECT_TRUE(block->shouldPaintCursorCaret()); + + LayoutRect caretVisualRect = caretDisplayItemClient().visualRect(); + EXPECT_EQ(1, caretVisualRect.width()); + EXPECT_EQ(block->location(), caretVisualRect.location()); + + const auto* rasterInvalidations = + &getRasterInvalidationTracking()->trackedRasterInvalidations; + ASSERT_EQ(1u, rasterInvalidations->size()); + EXPECT_EQ(enclosingIntRect(caretVisualRect), (*rasterInvalidations)[0].rect); + EXPECT_EQ(block, (*rasterInvalidations)[0].client); + EXPECT_EQ(PaintInvalidationCaret, (*rasterInvalidations)[0].reason); + + std::unique_ptr<JSONArray> objectInvalidations = + document().view()->trackedObjectPaintInvalidationsAsJSON(); + ASSERT_EQ(1u, objectInvalidations->size()); + String s; + JSONObject::cast(objectInvalidations->at(0))->get("object")->asString(&s); + EXPECT_EQ("Caret", s); + document().view()->setTracksPaintInvalidations(false); + + // Move the caret to the end of the text. Should invalidate both the old and + // new carets. + document().view()->setTracksPaintInvalidations(true); + selection().setSelection( + SelectionInDOMTree::Builder().collapse(Position(text, 5)).build()); + document().view()->updateAllLifecyclePhases(); + EXPECT_TRUE(block->shouldPaintCursorCaret()); + + LayoutRect newCaretVisualRect = caretDisplayItemClient().visualRect(); + EXPECT_EQ(caretVisualRect.size(), newCaretVisualRect.size()); + EXPECT_EQ(caretVisualRect.y(), newCaretVisualRect.y()); + EXPECT_LT(caretVisualRect.x(), newCaretVisualRect.x()); + + rasterInvalidations = + &getRasterInvalidationTracking()->trackedRasterInvalidations; + ASSERT_EQ(2u, rasterInvalidations->size()); + EXPECT_EQ(enclosingIntRect(caretVisualRect), (*rasterInvalidations)[0].rect); + EXPECT_EQ(block, (*rasterInvalidations)[0].client); + EXPECT_EQ(PaintInvalidationCaret, (*rasterInvalidations)[0].reason); + EXPECT_EQ(enclosingIntRect(newCaretVisualRect), + (*rasterInvalidations)[1].rect); + EXPECT_EQ(block, (*rasterInvalidations)[1].client); + EXPECT_EQ(PaintInvalidationCaret, (*rasterInvalidations)[1].reason); + + objectInvalidations = + document().view()->trackedObjectPaintInvalidationsAsJSON(); + ASSERT_EQ(1u, objectInvalidations->size()); + JSONObject::cast(objectInvalidations->at(0))->get("object")->asString(&s); + EXPECT_EQ("Caret", s); + document().view()->setTracksPaintInvalidations(false); + + // Remove selection. Should invalidate the old caret. + LayoutRect oldCaretVisualRect = newCaretVisualRect; + document().view()->setTracksPaintInvalidations(true); + selection().setSelection(SelectionInDOMTree()); + document().view()->updateAllLifecyclePhases(); + EXPECT_FALSE(block->shouldPaintCursorCaret()); + EXPECT_EQ(LayoutRect(), caretDisplayItemClient().visualRect()); + + rasterInvalidations = + &getRasterInvalidationTracking()->trackedRasterInvalidations; + ASSERT_EQ(1u, rasterInvalidations->size()); + EXPECT_EQ(enclosingIntRect(oldCaretVisualRect), + (*rasterInvalidations)[0].rect); + EXPECT_EQ(block, (*rasterInvalidations)[0].client); + + objectInvalidations = + document().view()->trackedObjectPaintInvalidationsAsJSON(); + ASSERT_EQ(1u, objectInvalidations->size()); + JSONObject::cast(objectInvalidations->at(0))->get("object")->asString(&s); + EXPECT_EQ("Caret", s); + document().view()->setTracksPaintInvalidations(false); +} + +TEST_F(CaretDisplayItemClientTest, CaretMovesBetweenBlocks) { + document().body()->setContentEditable("true", ASSERT_NO_EXCEPTION); + document().page()->focusController().setActive(true); + document().page()->focusController().setFocused(true); + auto* blockElement1 = appendBlock("Block1"); + auto* blockElement2 = appendBlock("Block2"); + document().view()->updateAllLifecyclePhases(); + auto* block1 = toLayoutBlock(blockElement1->layoutObject()); + auto* block2 = toLayoutBlock(blockElement2->layoutObject()); + + // Focus the body. + document().body()->focus(); + document().view()->updateAllLifecyclePhases(); + LayoutRect caretVisualRect1 = caretDisplayItemClient().visualRect(); + EXPECT_EQ(1, caretVisualRect1.width()); + EXPECT_EQ(block1->visualRect().location(), caretVisualRect1.location()); + EXPECT_TRUE(block1->shouldPaintCursorCaret()); + EXPECT_FALSE(block2->shouldPaintCursorCaret()); + + // Move the caret into block2. Should invalidate both the old and new carets. + document().view()->setTracksPaintInvalidations(true); + selection().setSelection(SelectionInDOMTree::Builder() + .collapse(Position(blockElement2, 0)) + .build()); + document().view()->updateAllLifecyclePhases(); + + LayoutRect caretVisualRect2 = caretDisplayItemClient().visualRect(); + EXPECT_EQ(1, caretVisualRect2.width()); + EXPECT_EQ(block2->visualRect().location(), caretVisualRect2.location()); + EXPECT_FALSE(block1->shouldPaintCursorCaret()); + EXPECT_TRUE(block2->shouldPaintCursorCaret()); + + const auto* rasterInvalidations = + &getRasterInvalidationTracking()->trackedRasterInvalidations; + ASSERT_EQ(2u, rasterInvalidations->size()); + EXPECT_EQ(enclosingIntRect(caretVisualRect1), (*rasterInvalidations)[0].rect); + EXPECT_EQ(block1, (*rasterInvalidations)[0].client); + EXPECT_EQ(PaintInvalidationCaret, (*rasterInvalidations)[0].reason); + EXPECT_EQ(enclosingIntRect(caretVisualRect2), (*rasterInvalidations)[1].rect); + EXPECT_EQ(block2, (*rasterInvalidations)[1].client); + EXPECT_EQ(PaintInvalidationCaret, (*rasterInvalidations)[1].reason); + + std::unique_ptr<JSONArray> objectInvalidations = + document().view()->trackedObjectPaintInvalidationsAsJSON(); + ASSERT_EQ(2u, objectInvalidations->size()); + document().view()->setTracksPaintInvalidations(false); + + // Move the caret back into block1. + document().view()->setTracksPaintInvalidations(true); + selection().setSelection(SelectionInDOMTree::Builder() + .collapse(Position(blockElement1, 0)) + .build()); + document().view()->updateAllLifecyclePhases(); + + EXPECT_EQ(caretVisualRect1, caretDisplayItemClient().visualRect()); + EXPECT_TRUE(block1->shouldPaintCursorCaret()); + EXPECT_FALSE(block2->shouldPaintCursorCaret()); + + rasterInvalidations = + &getRasterInvalidationTracking()->trackedRasterInvalidations; + ASSERT_EQ(2u, rasterInvalidations->size()); + EXPECT_EQ(enclosingIntRect(caretVisualRect1), (*rasterInvalidations)[0].rect); + EXPECT_EQ(block1, (*rasterInvalidations)[0].client); + EXPECT_EQ(PaintInvalidationCaret, (*rasterInvalidations)[0].reason); + EXPECT_EQ(enclosingIntRect(caretVisualRect2), (*rasterInvalidations)[1].rect); + EXPECT_EQ(block2, (*rasterInvalidations)[1].client); + EXPECT_EQ(PaintInvalidationCaret, (*rasterInvalidations)[1].reason); + + objectInvalidations = + document().view()->trackedObjectPaintInvalidationsAsJSON(); + ASSERT_EQ(2u, objectInvalidations->size()); + document().view()->setTracksPaintInvalidations(false); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/editing/FrameSelection.h b/third_party/WebKit/Source/core/editing/FrameSelection.h index 7744022..44328838 100644 --- a/third_party/WebKit/Source/core/editing/FrameSelection.h +++ b/third_party/WebKit/Source/core/editing/FrameSelection.h
@@ -296,7 +296,7 @@ DECLARE_TRACE(); private: - friend class BlockPaintInvalidatorTest; + friend class CaretDisplayItemClientTest; friend class FrameSelectionTest; friend class PaintControllerPaintTestForSlimmingPaintV1AndV2; friend class SelectionControllerTest;
diff --git a/third_party/WebKit/Source/core/paint/BlockPaintInvalidatorTest.cpp b/third_party/WebKit/Source/core/paint/BlockPaintInvalidatorTest.cpp deleted file mode 100644 index 0f3868d..0000000 --- a/third_party/WebKit/Source/core/paint/BlockPaintInvalidatorTest.cpp +++ /dev/null
@@ -1,135 +0,0 @@ -// 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 "core/HTMLNames.h" -#include "core/editing/FrameSelection.h" -#include "core/frame/FrameView.h" -#include "core/layout/LayoutTestHelper.h" -#include "core/layout/LayoutView.h" -#include "core/page/FocusController.h" -#include "core/paint/PaintLayer.h" -#include "platform/graphics/GraphicsLayer.h" -#include "platform/graphics/paint/RasterInvalidationTracking.h" - -namespace blink { - -class BlockPaintInvalidatorTest : public RenderingTest { - protected: - void SetUp() override { - RenderingTest::SetUp(); - enableCompositing(); - } - - const RasterInvalidationTracking* getRasterInvalidationTracking() const { - // TODO(wangxianzhu): Test SPv2. - return layoutView() - .layer() - ->graphicsLayerBacking() - ->getRasterInvalidationTracking(); - } - - FrameSelection& selection() const { - return document().view()->frame().selection(); - } - - const DisplayItemClient& caretDisplayItemClient() const { - return selection().caretDisplayItemClientForTesting(); - } - - Text* appendTextNode(const String& data) { - Text* text = document().createTextNode(data); - document().body()->appendChild(text); - return text; - } -}; - -TEST_F(BlockPaintInvalidatorTest, CaretPaintInvalidation) { - document().body()->setContentEditable("true", ASSERT_NO_EXCEPTION); - document().page()->focusController().setActive(true); - document().page()->focusController().setFocused(true); - - Text* text = appendTextNode("Hello, World!"); - document().view()->updateAllLifecyclePhases(); - const auto* block = toLayoutBlock(document().body()->layoutObject()); - - // Focus the body. Should invalidate the new caret. - document().view()->setTracksPaintInvalidations(true); - document().body()->focus(); - document().view()->updateAllLifecyclePhases(); - EXPECT_TRUE(block->shouldPaintCursorCaret()); - - LayoutRect caretVisualRect = caretDisplayItemClient().visualRect(); - EXPECT_EQ(1, caretVisualRect.width()); - EXPECT_EQ(block->location(), caretVisualRect.location()); - - const auto* rasterInvalidations = - &getRasterInvalidationTracking()->trackedRasterInvalidations; - ASSERT_EQ(1u, rasterInvalidations->size()); - EXPECT_EQ(enclosingIntRect(caretVisualRect), (*rasterInvalidations)[0].rect); - EXPECT_EQ(block, (*rasterInvalidations)[0].client); - EXPECT_EQ(PaintInvalidationCaret, (*rasterInvalidations)[0].reason); - - std::unique_ptr<JSONArray> objectInvalidations = - document().view()->trackedObjectPaintInvalidationsAsJSON(); - ASSERT_EQ(1u, objectInvalidations->size()); - String s; - JSONObject::cast(objectInvalidations->at(0))->get("object")->asString(&s); - EXPECT_EQ("Caret", s); - document().view()->setTracksPaintInvalidations(false); - - // Move the caret to the end of the text. Should invalidate both the old and - // new carets. - document().view()->setTracksPaintInvalidations(true); - selection().setSelection( - SelectionInDOMTree::Builder().collapse(Position(text, 5)).build()); - document().view()->updateAllLifecyclePhases(); - EXPECT_TRUE(block->shouldPaintCursorCaret()); - - LayoutRect newCaretVisualRect = caretDisplayItemClient().visualRect(); - EXPECT_EQ(caretVisualRect.size(), newCaretVisualRect.size()); - EXPECT_EQ(caretVisualRect.y(), newCaretVisualRect.y()); - EXPECT_LT(caretVisualRect.x(), newCaretVisualRect.x()); - - rasterInvalidations = - &getRasterInvalidationTracking()->trackedRasterInvalidations; - ASSERT_EQ(2u, rasterInvalidations->size()); - EXPECT_EQ(enclosingIntRect(caretVisualRect), (*rasterInvalidations)[0].rect); - EXPECT_EQ(block, (*rasterInvalidations)[0].client); - EXPECT_EQ(PaintInvalidationCaret, (*rasterInvalidations)[0].reason); - EXPECT_EQ(enclosingIntRect(newCaretVisualRect), - (*rasterInvalidations)[1].rect); - EXPECT_EQ(block, (*rasterInvalidations)[1].client); - EXPECT_EQ(PaintInvalidationCaret, (*rasterInvalidations)[1].reason); - - objectInvalidations = - document().view()->trackedObjectPaintInvalidationsAsJSON(); - ASSERT_EQ(1u, objectInvalidations->size()); - JSONObject::cast(objectInvalidations->at(0))->get("object")->asString(&s); - EXPECT_EQ("Caret", s); - document().view()->setTracksPaintInvalidations(false); - - // Remove selection. Should invalidate the old caret. - LayoutRect oldCaretVisualRect = newCaretVisualRect; - document().view()->setTracksPaintInvalidations(true); - selection().setSelection(SelectionInDOMTree()); - document().view()->updateAllLifecyclePhases(); - EXPECT_FALSE(block->shouldPaintCursorCaret()); - EXPECT_EQ(LayoutRect(), caretDisplayItemClient().visualRect()); - - rasterInvalidations = - &getRasterInvalidationTracking()->trackedRasterInvalidations; - ASSERT_EQ(1u, rasterInvalidations->size()); - EXPECT_EQ(enclosingIntRect(oldCaretVisualRect), - (*rasterInvalidations)[0].rect); - EXPECT_EQ(block, (*rasterInvalidations)[0].client); - - objectInvalidations = - document().view()->trackedObjectPaintInvalidationsAsJSON(); - ASSERT_EQ(1u, objectInvalidations->size()); - JSONObject::cast(objectInvalidations->at(0))->get("object")->asString(&s); - EXPECT_EQ("Caret", s); - document().view()->setTracksPaintInvalidations(false); -} - -} // namespace blink
diff --git a/third_party/WebKit/Source/platform/scheduler/base/real_time_domain.cc b/third_party/WebKit/Source/platform/scheduler/base/real_time_domain.cc index 7406dc4..312136a 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/real_time_domain.cc +++ b/third_party/WebKit/Source/platform/scheduler/base/real_time_domain.cc
@@ -39,16 +39,11 @@ return task_queue_manager_->delegate()->NowTicks(); } -void RealTimeDomain::RequestWakeupAt(base::TimeTicks now, - base::TimeTicks run_time) { +void RealTimeDomain::RequestWakeup(base::TimeTicks now, base::TimeDelta delay) { // NOTE this is only called if the scheduled runtime is sooner than any // previously scheduled runtime, or there is no (outstanding) previously // scheduled runtime. - task_queue_manager_->MaybeScheduleDelayedWork(FROM_HERE, this, now, run_time); -} - -void RealTimeDomain::CancelWakeupAt(base::TimeTicks run_time) { - task_queue_manager_->CancelDelayedWork(this, run_time); + task_queue_manager_->MaybeScheduleDelayedWork(FROM_HERE, now, delay); } base::Optional<base::TimeDelta> RealTimeDomain::DelayTillNextTask(
diff --git a/third_party/WebKit/Source/platform/scheduler/base/real_time_domain.h b/third_party/WebKit/Source/platform/scheduler/base/real_time_domain.h index 1457204..6883599 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/real_time_domain.h +++ b/third_party/WebKit/Source/platform/scheduler/base/real_time_domain.h
@@ -29,8 +29,7 @@ protected: void OnRegisterWithTaskQueueManager( TaskQueueManager* task_queue_manager) override; - void RequestWakeupAt(base::TimeTicks now, base::TimeTicks run_time) override; - void CancelWakeupAt(base::TimeTicks run_time) override; + void RequestWakeup(base::TimeTicks now, base::TimeDelta delay) override; void AsValueIntoInternal( base::trace_event::TracedValue* state) const override;
diff --git a/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc b/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc index 2d1e295..762f079 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc +++ b/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
@@ -276,14 +276,11 @@ main_thread_only().delayed_incoming_queue.push(std::move(pending_task)); // If |pending_task| is at the head of the queue, then make sure a wakeup - // is requested if the queue is enabled. Note we still want to schedule a - // wakeup even if blocked by a fence, because we'd break throttling logic - // otherwise. - base::TimeTicks next_delayed_task = - main_thread_only().delayed_incoming_queue.top().delayed_run_time; - if (next_delayed_task == delayed_run_time && IsQueueEnabled()) { - main_thread_only().time_domain->ScheduleDelayedWork(this, delayed_run_time, - now); + // is requested. + if (main_thread_only().delayed_incoming_queue.top().delayed_run_time == + delayed_run_time) { + main_thread_only().time_domain->ScheduleDelayedWork( + this, pending_task.delayed_run_time, now); } TraceQueueSize(false); @@ -407,8 +404,7 @@ } base::Optional<base::TimeTicks> TaskQueueImpl::GetNextScheduledWakeUp() { - // Note we don't scheduled a wakeup for disabled queues. - if (main_thread_only().delayed_incoming_queue.empty() || !IsQueueEnabled()) + if (main_thread_only().delayed_incoming_queue.empty()) return base::nullopt; return main_thread_only().delayed_incoming_queue.top().delayed_run_time; @@ -803,18 +799,10 @@ return; if (enable) { - if (!main_thread_only().delayed_incoming_queue.empty()) { - main_thread_only().time_domain->ScheduleDelayedWork( - this, - main_thread_only().delayed_incoming_queue.top().delayed_run_time, - main_thread_only().time_domain->Now()); - } // Note the selector calls TaskQueueManager::OnTaskQueueEnabled which posts // a DoWork if needed. main_thread_only().task_queue_manager->selector_.EnableQueue(this); } else { - if (!main_thread_only().delayed_incoming_queue.empty()) - main_thread_only().time_domain->CancelDelayedWork(this); main_thread_only().task_queue_manager->selector_.DisableQueue(this); } }
diff --git a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc index 49fd2f8..e42fee1 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc +++ b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
@@ -230,13 +230,10 @@ void TaskQueueManager::MaybeScheduleDelayedWork( const tracked_objects::Location& from_here, - TimeDomain* requesting_time_domain, base::TimeTicks now, - base::TimeTicks run_time) { + base::TimeDelta delay) { DCHECK(main_thread_checker_.CalledOnValidThread()); - // Make sure we don't cancel another TimeDomain's wakeup. - DCHECK(!next_delayed_do_work_ || - next_delayed_do_work_.time_domain() == requesting_time_domain); + DCHECK_GE(delay, base::TimeDelta()); { base::AutoLock lock(any_thread_lock_); @@ -250,35 +247,22 @@ } } - // If there's a delayed DoWork scheduled to run sooner, we don't need to do - // anything because DoWork will post a delayed continuation as needed. - if (next_delayed_do_work_ && next_delayed_do_work_.run_time() <= run_time) + // De-duplicate DoWork posts. + base::TimeTicks run_time = now + delay; + if (next_scheduled_delayed_do_work_time_ <= run_time && + !next_scheduled_delayed_do_work_time_.is_null()) return; - cancelable_delayed_do_work_closure_.Reset(delayed_do_work_closure_); - - base::TimeDelta delay = std::max(base::TimeDelta(), run_time - now); TRACE_EVENT1(disabled_by_default_tracing_category_, "TaskQueueManager::MaybeScheduleDelayedWork::PostDelayedTask", "delay_ms", delay.InMillisecondsF()); cancelable_delayed_do_work_closure_.Reset(delayed_do_work_closure_); - next_delayed_do_work_ = NextDelayedDoWork(run_time, requesting_time_domain); + next_scheduled_delayed_do_work_time_ = run_time; delegate_->PostDelayedTask( from_here, cancelable_delayed_do_work_closure_.callback(), delay); } -void TaskQueueManager::CancelDelayedWork(TimeDomain* requesting_time_domain, - base::TimeTicks run_time) { - DCHECK(main_thread_checker_.CalledOnValidThread()); - if (next_delayed_do_work_.run_time() != run_time) - return; - - DCHECK_EQ(next_delayed_do_work_.time_domain(), requesting_time_domain); - cancelable_delayed_do_work_closure_.Cancel(); - next_delayed_do_work_.Clear(); -} - void TaskQueueManager::DoWork(bool delayed) { DCHECK(main_thread_checker_.CalledOnValidThread()); TRACE_EVENT1(tracing_category_, "TaskQueueManager::DoWork", "delayed", @@ -290,9 +274,10 @@ queues_to_delete_.clear(); // This must be done before running any tasks because they could invoke a - // nested message loop and we risk having a stale |next_delayed_do_work_|. + // nested message loop and we risk having a stale + // |next_scheduled_delayed_do_work_time_|. if (delayed) - next_delayed_do_work_.Clear(); + next_scheduled_delayed_do_work_time_ = base::TimeTicks(); for (int i = 0; i < work_batch_size_; i++) { IncomingImmediateWorkMap queues_to_reload; @@ -354,7 +339,7 @@ { MoveableAutoLock lock(any_thread_lock_); - base::Optional<NextTaskDelay> next_delay = + base::Optional<base::TimeDelta> next_delay = ComputeDelayTillNextTaskLocked(&lazy_now); any_thread().do_work_running_count--; @@ -368,10 +353,11 @@ } void TaskQueueManager::PostDoWorkContinuationLocked( - base::Optional<NextTaskDelay> next_delay, + base::Optional<base::TimeDelta> next_delay, LazyNow* lazy_now, MoveableAutoLock&& lock) { DCHECK(main_thread_checker_.CalledOnValidThread()); + base::TimeDelta delay; { MoveableAutoLock auto_lock(std::move(lock)); @@ -379,8 +365,8 @@ // If there are no tasks left then we don't need to post a continuation. if (!next_delay) { // If there's a pending delayed DoWork, cancel it because it's not needed. - if (next_delayed_do_work_) { - next_delayed_do_work_.Clear(); + if (!next_scheduled_delayed_do_work_time_.is_null()) { + next_scheduled_delayed_do_work_time_ = base::TimeTicks(); cancelable_delayed_do_work_closure_.Cancel(); } return; @@ -390,37 +376,42 @@ if (any_thread().immediate_do_work_posted_count > 0) return; - if (next_delay->delay() <= base::TimeDelta()) { + delay = next_delay.value(); + + // This isn't supposed to happen, but in case it does convert to + // non-delayed. + if (delay < base::TimeDelta()) + delay = base::TimeDelta(); + + if (delay.is_zero()) { // If a delayed DoWork is pending then we don't need to post a // continuation because it should run immediately. - if (next_delayed_do_work_ && - next_delayed_do_work_.run_time() <= lazy_now->Now()) { + if (!next_scheduled_delayed_do_work_time_.is_null() && + next_scheduled_delayed_do_work_time_ <= lazy_now->Now()) { return; } any_thread().immediate_do_work_posted_count++; + } else { + base::TimeTicks run_time = lazy_now->Now() + delay; + if (next_scheduled_delayed_do_work_time_ == run_time) + return; + + next_scheduled_delayed_do_work_time_ = run_time; } } // We avoid holding |any_thread_lock_| while posting the task. - if (next_delay->delay() <= base::TimeDelta()) { + if (delay.is_zero()) { delegate_->PostTask(FROM_HERE, immediate_do_work_closure_); } else { - base::TimeTicks run_time = lazy_now->Now() + next_delay->delay(); - - if (next_delayed_do_work_.run_time() == run_time) - return; - - next_delayed_do_work_ = - NextDelayedDoWork(run_time, next_delay->time_domain()); cancelable_delayed_do_work_closure_.Reset(delayed_do_work_closure_); - delegate_->PostDelayedTask(FROM_HERE, - cancelable_delayed_do_work_closure_.callback(), - next_delay->delay()); + delegate_->PostDelayedTask( + FROM_HERE, cancelable_delayed_do_work_closure_.callback(), delay); } } -base::Optional<TaskQueueManager::NextTaskDelay> +base::Optional<base::TimeDelta> TaskQueueManager::ComputeDelayTillNextTaskLocked(LazyNow* lazy_now) { DCHECK(main_thread_checker_.CalledOnValidThread()); @@ -429,32 +420,27 @@ // check is equavalent to calling ReloadEmptyWorkQueues first. for (const auto& pair : any_thread().has_incoming_immediate_work) { if (pair.first->CouldTaskRun(pair.second)) - return NextTaskDelay(); + return base::TimeDelta(); } // If the selector has non-empty queues we trivially know there is immediate // work to be done. if (!selector_.EnabledWorkQueuesEmpty()) - return NextTaskDelay(); + return base::TimeDelta(); // Otherwise we need to find the shortest delay, if any. NB we don't need to // call WakeupReadyDelayedQueues because it's assumed DelayTillNextTask will // return base::TimeDelta>() if the delayed task is due to run now. - base::Optional<NextTaskDelay> delay_till_next_task; + base::Optional<base::TimeDelta> next_continuation; for (TimeDomain* time_domain : time_domains_) { - base::Optional<base::TimeDelta> delay = + base::Optional<base::TimeDelta> continuation = time_domain->DelayTillNextTask(lazy_now); - if (!delay) + if (!continuation) continue; - - NextTaskDelay task_delay = (delay.value() == base::TimeDelta()) - ? NextTaskDelay() - : NextTaskDelay(delay.value(), time_domain); - - if (!delay_till_next_task || delay_till_next_task > task_delay) - delay_till_next_task = task_delay; + if (!next_continuation || next_continuation.value() > continuation.value()) + next_continuation = continuation; } - return delay_till_next_task; + return next_continuation; } bool TaskQueueManager::SelectWorkQueueToService(
diff --git a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.h b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.h index fa568cc6..21c4aedc 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.h +++ b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.h
@@ -74,14 +74,8 @@ // runner. These delayed tasks are de-duplicated. Must be called on the thread // this class was created on. void MaybeScheduleDelayedWork(const tracked_objects::Location& from_here, - TimeDomain* requesting_time_domain, base::TimeTicks now, - base::TimeTicks run_time); - - // Cancels a delayed task to process work at |run_time|, previously requested - // with MaybeScheduleDelayedWork. - void CancelDelayedWork(TimeDomain* requesting_time_domain, - base::TimeTicks run_time); + base::TimeDelta delay); // Set the number of tasks executed in a single invocation of the task queue // manager. Increasing the batch size can reduce the overhead of yielding @@ -155,73 +149,11 @@ // need them, you can turn them off. void SetRecordTaskDelayHistograms(bool record_task_delay_histograms); - protected: + private: friend class LazyNow; friend class internal::TaskQueueImpl; friend class TaskQueueManagerTest; - // Intermediate data structure, used to compute NextDelayedDoWork. - class NextTaskDelay { - public: - NextTaskDelay() : time_domain_(nullptr) {} - - using AllowAnyDelayForTesting = int; - - NextTaskDelay(base::TimeDelta delay, TimeDomain* time_domain) - : delay_(delay), time_domain_(time_domain) { - DCHECK_GT(delay, base::TimeDelta()); - DCHECK(time_domain); - } - - NextTaskDelay(base::TimeDelta delay, - TimeDomain* time_domain, - AllowAnyDelayForTesting) - : delay_(delay), time_domain_(time_domain) { - DCHECK(time_domain); - } - - base::TimeDelta delay() const { return delay_; } - TimeDomain* time_domain() const { return time_domain_; } - - bool operator>(const NextTaskDelay& other) const { - return delay_ > other.delay_; - } - - bool operator<(const NextTaskDelay& other) const { - return delay_ < other.delay_; - } - - private: - base::TimeDelta delay_; - TimeDomain* time_domain_; - }; - - private: - // Represents a scheduled delayed DoWork (if any). Only public for testing. - class NextDelayedDoWork { - public: - NextDelayedDoWork() : time_domain_(nullptr) {} - NextDelayedDoWork(base::TimeTicks run_time, TimeDomain* time_domain) - : run_time_(run_time), time_domain_(time_domain) { - DCHECK_NE(run_time, base::TimeTicks()); - DCHECK(time_domain); - } - - base::TimeTicks run_time() const { return run_time_; } - TimeDomain* time_domain() const { return time_domain_; } - - void Clear() { - run_time_ = base::TimeTicks(); - time_domain_ = nullptr; - } - - explicit operator bool() const { return !run_time_.is_null(); } - - private: - base::TimeTicks run_time_; - TimeDomain* time_domain_; - }; - class DeletionSentinel : public base::RefCounted<DeletionSentinel> { private: friend class base::RefCounted<DeletionSentinel>; @@ -246,7 +178,7 @@ void DoWork(bool delayed); // Post a DoWork continuation if |next_delay| is not empty. - void PostDoWorkContinuationLocked(base::Optional<NextTaskDelay> next_delay, + void PostDoWorkContinuationLocked(base::Optional<base::TimeDelta> next_delay, LazyNow* lazy_now, MoveableAutoLock&& lock); @@ -284,7 +216,7 @@ // Calls DelayTillNextTask on all time domains and returns the smallest delay // requested if any. - base::Optional<NextTaskDelay> ComputeDelayTillNextTaskLocked( + base::Optional<base::TimeDelta> ComputeDelayTillNextTaskLocked( LazyNow* lazy_now); void MaybeRecordTaskDelayHistograms( @@ -361,7 +293,7 @@ return any_thread_; } - NextDelayedDoWork next_delayed_do_work_; + base::TimeTicks next_scheduled_delayed_do_work_time_; bool record_task_delay_histograms_;
diff --git a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_perftest.cc b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_perftest.cc index a39f1b6..a7f7454 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_perftest.cc +++ b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_perftest.cc
@@ -42,17 +42,12 @@ return base::TimeDelta(); // Makes DoWork post an immediate continuation. } - void RequestWakeupAt(base::TimeTicks now, base::TimeTicks run_time) override { + void RequestWakeup(base::TimeTicks now, base::TimeDelta delay) override { // De-dupe DoWorks. if (NumberOfScheduledWakeups() == 1u) RequestDoWork(); } - void CancelWakeupAt(base::TimeTicks run_time) override { - // We didn't post a delayed task in RequestWakeupAt so there's no need to do - // anything here. - } - const char* GetName() const override { return "PerfTestTimeDomain"; } DISALLOW_COPY_AND_ASSIGN(PerfTestTimeDomain);
diff --git a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc index ed91bfa..fc290a3 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc +++ b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
@@ -32,11 +32,9 @@ #include "platform/scheduler/base/work_queue_sets.h" #include "testing/gmock/include/gmock/gmock.h" -using testing::AnyNumber; using testing::Contains; using testing::ElementsAre; using testing::ElementsAreArray; -using testing::Mock; using testing::Not; using testing::_; using blink::scheduler::internal::EnqueueOrder; @@ -44,21 +42,6 @@ namespace blink { namespace scheduler { -class TaskQueueManagerForTest : public TaskQueueManager { - public: - TaskQueueManagerForTest( - scoped_refptr<TaskQueueManagerDelegate> delegate, - const char* tracing_category, - const char* disabled_by_default_tracing_category, - const char* disabled_by_default_verbose_tracing_category) - : TaskQueueManager(delegate, - tracing_category, - disabled_by_default_tracing_category, - disabled_by_default_verbose_tracing_category) {} - - using TaskQueueManager::NextTaskDelay; -}; - class MessageLoopTaskRunner : public TaskQueueManagerDelegateForTest { public: static scoped_refptr<MessageLoopTaskRunner> Create( @@ -102,7 +85,7 @@ test_task_runner_.get(), base::MakeUnique<TestTimeSource>(now_src_.get())); - manager_ = base::MakeUnique<TaskQueueManagerForTest>( + manager_ = base::MakeUnique<TaskQueueManager>( main_task_runner_, "test.scheduler", "test.scheduler", "test.scheduler.debug"); @@ -123,7 +106,7 @@ message_loop_.reset(new base::MessageLoop()); // A null clock triggers some assertions. now_src_->Advance(base::TimeDelta::FromMicroseconds(1000)); - manager_ = base::MakeUnique<TaskQueueManagerForTest>( + manager_ = base::MakeUnique<TaskQueueManager>( MessageLoopTaskRunner::Create( base::WrapUnique(new TestTimeSource(now_src_.get()))), "test.scheduler", "test.scheduler", "test.scheduler.debug"); @@ -137,14 +120,12 @@ manager_->WakeupReadyDelayedQueues(&lazy_now); } - using NextTaskDelay = TaskQueueManagerForTest::NextTaskDelay; - - base::Optional<NextTaskDelay> ComputeDelayTillNextTask(LazyNow* lazy_now) { + base::Optional<base::TimeDelta> ComputeDelayTillNextTask(LazyNow* lazy_now) { base::AutoLock lock(manager_->any_thread_lock_); return manager_->ComputeDelayTillNextTaskLocked(lazy_now); } - void PostDoWorkContinuation(base::Optional<NextTaskDelay> next_delay, + void PostDoWorkContinuation(base::Optional<base::TimeDelta> next_delay, LazyNow* lazy_now) { MoveableAutoLock lock(manager_->any_thread_lock_); return manager_->PostDoWorkContinuationLocked(next_delay, lazy_now, @@ -156,8 +137,8 @@ return manager_->any_thread().immediate_do_work_posted_count; } - base::TimeTicks next_delayed_do_work_time() const { - return manager_->next_delayed_do_work_.run_time(); + base::TimeTicks next_scheduled_delayed_do_work_time() const { + return manager_->next_scheduled_delayed_do_work_time_; } EnqueueOrder GetNextSequenceNumber() const { @@ -194,7 +175,7 @@ std::unique_ptr<base::SimpleTestTickClock> now_src_; scoped_refptr<TaskQueueManagerDelegateForTest> main_task_runner_; scoped_refptr<cc::OrderedSimpleTaskRunner> test_task_runner_; - std::unique_ptr<TaskQueueManagerForTest> manager_; + std::unique_ptr<TaskQueueManager> manager_; std::vector<scoped_refptr<internal::TaskQueueImpl>> runners_; TestTaskTimeObserver test_task_time_observer_; }; @@ -223,7 +204,7 @@ TestCountUsesTimeSource* test_count_uses_time_source = new TestCountUsesTimeSource(); - manager_ = base::MakeUnique<TaskQueueManagerForTest>( + manager_ = base::MakeUnique<TaskQueueManager>( MessageLoopTaskRunner::Create( base::WrapUnique(test_count_uses_time_source)), "test.scheduler", "test.scheduler", "test.scheduler.debug"); @@ -256,7 +237,7 @@ TestCountUsesTimeSource* test_count_uses_time_source = new TestCountUsesTimeSource(); - manager_ = base::MakeUnique<TaskQueueManagerForTest>( + manager_ = base::MakeUnique<TaskQueueManager>( MessageLoopTaskRunner::Create( base::WrapUnique(test_count_uses_time_source)), "test.scheduler", "test.scheduler", "test.scheduler.debug"); @@ -401,11 +382,16 @@ EXPECT_TRUE(runners_[0]->HasPendingImmediateWork()); // Move the task into the |delayed_work_queue|. - WakeupReadyDelayedQueues(LazyNow(now_src_.get())); + EXPECT_TRUE(runners_[0]->delayed_work_queue()->Empty()); + std::unique_ptr<TaskQueue::QueueEnabledVoter> voter = + runners_[0]->CreateQueueEnabledVoter(); + voter->SetQueueEnabled(false); + test_task_runner_->RunUntilIdle(); EXPECT_FALSE(runners_[0]->delayed_work_queue()->Empty()); EXPECT_TRUE(runners_[0]->HasPendingImmediateWork()); // Run the task, making the queue empty. + voter->SetQueueEnabled(true); test_task_runner_->RunUntilIdle(); EXPECT_FALSE(runners_[0]->HasPendingImmediateWork()); } @@ -1690,137 +1676,6 @@ } namespace { -class MockTimeDomainObserver : public TimeDomain::Observer { - public: - ~MockTimeDomainObserver() override {} - - MOCK_METHOD1(OnTimeDomainHasImmediateWork, void(TaskQueue*)); - MOCK_METHOD1(OnTimeDomainHasDelayedWork, void(TaskQueue*)); -}; -} // namespace - -TEST_F(TaskQueueManagerTest, TimeDomainObserver_ImmediateTask) { - Initialize(1u); - - MockTimeDomainObserver observer; - std::unique_ptr<VirtualTimeDomain> domain( - new VirtualTimeDomain(&observer, manager_->delegate()->NowTicks())); - manager_->RegisterTimeDomain(domain.get()); - runners_[0]->SetTimeDomain(domain.get()); - - // We should get a notification when a task is posted on an empty queue. - EXPECT_CALL(observer, OnTimeDomainHasImmediateWork(runners_[0].get())); - runners_[0]->PostTask(FROM_HERE, base::Bind(&NopTask)); - Mock::VerifyAndClearExpectations(&observer); - - // But not subsequently. - EXPECT_CALL(observer, OnTimeDomainHasImmediateWork(_)).Times(0); - runners_[0]->PostTask(FROM_HERE, base::Bind(&NopTask)); - Mock::VerifyAndClearExpectations(&observer); - - // Unless the immediate work queue is emptied. - runners_[0]->ReloadImmediateWorkQueueIfEmpty(); - EXPECT_CALL(observer, OnTimeDomainHasImmediateWork(runners_[0].get())); - runners_[0]->PostTask(FROM_HERE, base::Bind(&NopTask)); - - // Tidy up. - runners_[0]->UnregisterTaskQueue(); - manager_->UnregisterTimeDomain(domain.get()); -} - -TEST_F(TaskQueueManagerTest, TimeDomainObserver_DelayedTask) { - Initialize(1u); - - MockTimeDomainObserver observer; - std::unique_ptr<VirtualTimeDomain> domain( - new VirtualTimeDomain(&observer, manager_->delegate()->NowTicks())); - manager_->RegisterTimeDomain(domain.get()); - runners_[0]->SetTimeDomain(domain.get()); - - // We should get a notification when a delayed task is posted on an empty - // queue. - EXPECT_CALL(observer, OnTimeDomainHasDelayedWork(runners_[0].get())); - runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), - base::TimeDelta::FromSeconds(10)); - Mock::VerifyAndClearExpectations(&observer); - - // We should not get a notification for a longer delay. - EXPECT_CALL(observer, OnTimeDomainHasDelayedWork(_)).Times(0); - runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), - base::TimeDelta::FromSeconds(100)); - Mock::VerifyAndClearExpectations(&observer); - - // We should get a notification for a shorter delay. - EXPECT_CALL(observer, OnTimeDomainHasDelayedWork(runners_[0].get())); - runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), - base::TimeDelta::FromSeconds(1)); - Mock::VerifyAndClearExpectations(&observer); - - std::unique_ptr<TaskQueue::QueueEnabledVoter> voter = - runners_[0]->CreateQueueEnabledVoter(); - voter->SetQueueEnabled(false); - - // When a queue has been enabled, we may get a notification if the - // TimeDomain's next scheduled wakeup has changed. - EXPECT_CALL(observer, OnTimeDomainHasDelayedWork(runners_[0].get())); - voter->SetQueueEnabled(true); - - // Tidy up. - runners_[0]->UnregisterTaskQueue(); - manager_->UnregisterTimeDomain(domain.get()); -} - -TEST_F(TaskQueueManagerTest, TimeDomainObserver_DelayedTaskMultipleQueues) { - Initialize(2u); - - MockTimeDomainObserver observer; - std::unique_ptr<VirtualTimeDomain> domain( - new VirtualTimeDomain(&observer, manager_->delegate()->NowTicks())); - manager_->RegisterTimeDomain(domain.get()); - runners_[0]->SetTimeDomain(domain.get()); - runners_[1]->SetTimeDomain(domain.get()); - - EXPECT_CALL(observer, OnTimeDomainHasDelayedWork(runners_[0].get())).Times(1); - EXPECT_CALL(observer, OnTimeDomainHasDelayedWork(runners_[1].get())).Times(1); - runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), - base::TimeDelta::FromSeconds(1)); - runners_[1]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), - base::TimeDelta::FromSeconds(10)); - testing::Mock::VerifyAndClearExpectations(&observer); - - std::unique_ptr<TaskQueue::QueueEnabledVoter> voter0 = - runners_[0]->CreateQueueEnabledVoter(); - std::unique_ptr<TaskQueue::QueueEnabledVoter> voter1 = - runners_[1]->CreateQueueEnabledVoter(); - - // Disabling a queue should not trigger a notification. - EXPECT_CALL(observer, OnTimeDomainHasDelayedWork(_)).Times(0); - voter0->SetQueueEnabled(false); - Mock::VerifyAndClearExpectations(&observer); - - // Re-enabling it should should also trigger a notification. - EXPECT_CALL(observer, OnTimeDomainHasDelayedWork(runners_[0].get())); - voter0->SetQueueEnabled(true); - Mock::VerifyAndClearExpectations(&observer); - - // Disabling a queue should not trigger a notification. - EXPECT_CALL(observer, OnTimeDomainHasDelayedWork(_)).Times(0); - voter1->SetQueueEnabled(false); - Mock::VerifyAndClearExpectations(&observer); - - // Re-enabling it should should trigger a notification. - EXPECT_CALL(observer, OnTimeDomainHasDelayedWork(runners_[1].get())); - voter1->SetQueueEnabled(true); - Mock::VerifyAndClearExpectations(&observer); - - // Tidy up. - EXPECT_CALL(observer, OnTimeDomainHasDelayedWork(_)).Times(AnyNumber()); - runners_[0]->UnregisterTaskQueue(); - runners_[1]->UnregisterTaskQueue(); - manager_->UnregisterTimeDomain(domain.get()); -} - -namespace { void ChromiumRunloopInspectionTask( scoped_refptr<cc::OrderedSimpleTaskRunner> test_task_runner) { EXPECT_EQ(1u, test_task_runner->NumPendingTasks()); @@ -2466,6 +2321,10 @@ TEST_F(TaskQueueManagerTest, ComputeDelayTillNextTask) { Initialize(2u); + std::unique_ptr<RealTimeDomain> domain2(new RealTimeDomain("test")); + manager_->RegisterTimeDomain(domain2.get()); + runners_[1]->SetTimeDomain(domain2.get()); + LazyNow lazy_now(now_src_.get()); EXPECT_FALSE(static_cast<bool>(ComputeDelayTillNextTask(&lazy_now))); @@ -2473,23 +2332,27 @@ base::TimeDelta::FromSeconds(10)); EXPECT_EQ(base::TimeDelta::FromSeconds(10), - ComputeDelayTillNextTask(&lazy_now)->delay()); + ComputeDelayTillNextTask(&lazy_now).value()); runners_[1]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), base::TimeDelta::FromSeconds(15)); EXPECT_EQ(base::TimeDelta::FromSeconds(10), - ComputeDelayTillNextTask(&lazy_now)->delay()); + ComputeDelayTillNextTask(&lazy_now).value()); runners_[1]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), base::TimeDelta::FromSeconds(5)); EXPECT_EQ(base::TimeDelta::FromSeconds(5), - ComputeDelayTillNextTask(&lazy_now)->delay()); + ComputeDelayTillNextTask(&lazy_now).value()); runners_[0]->PostTask(FROM_HERE, base::Bind(&NopTask)); - EXPECT_EQ(base::TimeDelta(), ComputeDelayTillNextTask(&lazy_now)->delay()); + EXPECT_EQ(base::TimeDelta(), ComputeDelayTillNextTask(&lazy_now).value()); + + // Tidy up. + runners_[1]->UnregisterTaskQueue(); + manager_->UnregisterTimeDomain(domain2.get()); } TEST_F(TaskQueueManagerTest, ComputeDelayTillNextTask_Disabled) { @@ -2522,7 +2385,7 @@ runners_[0]->InsertFence(TaskQueue::InsertFencePosition::NOW); LazyNow lazy_now(now_src_.get()); - EXPECT_EQ(base::TimeDelta(), ComputeDelayTillNextTask(&lazy_now)->delay()); + EXPECT_EQ(base::TimeDelta(), ComputeDelayTillNextTask(&lazy_now).value()); } TEST_F(TaskQueueManagerTest, ComputeDelayTillNextTask_DelayedTaskReady) { @@ -2534,63 +2397,44 @@ now_src_->Advance(base::TimeDelta::FromSeconds(10)); LazyNow lazy_now(now_src_.get()); - EXPECT_EQ(base::TimeDelta(), ComputeDelayTillNextTask(&lazy_now)->delay()); + EXPECT_EQ(base::TimeDelta(), ComputeDelayTillNextTask(&lazy_now).value()); } TEST_F(TaskQueueManagerTest, PostDoWorkContinuation_NoMoreWork) { Initialize(1u); LazyNow lazy_now(now_src_.get()); - PostDoWorkContinuation(base::Optional<NextTaskDelay>(), &lazy_now); + PostDoWorkContinuation(base::Optional<base::TimeDelta>(), &lazy_now); EXPECT_EQ(0u, test_task_runner_->NumPendingTasks()); EXPECT_EQ(0, immediate_do_work_posted_count()); - EXPECT_TRUE(next_delayed_do_work_time().is_null()); + EXPECT_TRUE(next_scheduled_delayed_do_work_time().is_null()); } TEST_F(TaskQueueManagerTest, PostDoWorkContinuation_ImmediateWork) { Initialize(1u); LazyNow lazy_now(now_src_.get()); - PostDoWorkContinuation(NextTaskDelay(), &lazy_now); + PostDoWorkContinuation(base::TimeDelta(), &lazy_now); EXPECT_EQ(1u, test_task_runner_->NumPendingTasks()); EXPECT_EQ(base::TimeDelta(), test_task_runner_->DelayToNextTaskTime()); EXPECT_EQ(1, immediate_do_work_posted_count()); - EXPECT_TRUE(next_delayed_do_work_time().is_null()); -} - -TEST_F(TaskQueueManagerTest, PostDoWorkContinuation_DelayedWorkInThePast) { - Initialize(1u); - - LazyNow lazy_now(now_src_.get()); - // Note this isn't supposed to happen in practice. - PostDoWorkContinuation( - NextTaskDelay(base::TimeDelta::FromSeconds(-1), - runners_[0]->GetTimeDomain(), - NextTaskDelay::AllowAnyDelayForTesting()), - &lazy_now); - - EXPECT_EQ(1u, test_task_runner_->NumPendingTasks()); - EXPECT_EQ(base::TimeDelta(), test_task_runner_->DelayToNextTaskTime()); - EXPECT_EQ(1, immediate_do_work_posted_count()); - EXPECT_TRUE(next_delayed_do_work_time().is_null()); + EXPECT_TRUE(next_scheduled_delayed_do_work_time().is_null()); } TEST_F(TaskQueueManagerTest, PostDoWorkContinuation_DelayedWork) { Initialize(1u); LazyNow lazy_now(now_src_.get()); - PostDoWorkContinuation(NextTaskDelay(base::TimeDelta::FromSeconds(1), - runners_[0]->GetTimeDomain()), - &lazy_now); + PostDoWorkContinuation(base::TimeDelta::FromSeconds(1), &lazy_now); EXPECT_EQ(1u, test_task_runner_->NumPendingTasks()); EXPECT_EQ(base::TimeDelta::FromSeconds(1), test_task_runner_->DelayToNextTaskTime()); EXPECT_EQ(0, immediate_do_work_posted_count()); EXPECT_EQ(lazy_now.Now() + base::TimeDelta::FromSeconds(1), - next_delayed_do_work_time()); + next_scheduled_delayed_do_work_time()); } TEST_F(TaskQueueManagerTest, @@ -2603,35 +2447,29 @@ EXPECT_EQ(1, immediate_do_work_posted_count()); LazyNow lazy_now(now_src_.get()); - PostDoWorkContinuation(NextTaskDelay(base::TimeDelta::FromSeconds(1), - runners_[0]->GetTimeDomain()), - &lazy_now); + PostDoWorkContinuation(base::TimeDelta::FromSeconds(1), &lazy_now); // Test that a delayed task didn't get posted. EXPECT_EQ(1u, test_task_runner_->NumPendingTasks()); EXPECT_EQ(base::TimeDelta(), test_task_runner_->DelayToNextTaskTime()); EXPECT_EQ(1, immediate_do_work_posted_count()); - EXPECT_TRUE(next_delayed_do_work_time().is_null()); + EXPECT_TRUE(next_scheduled_delayed_do_work_time().is_null()); } TEST_F(TaskQueueManagerTest, PostDoWorkContinuation_DelayedWorkTimeChanges) { Initialize(1u); LazyNow lazy_now(now_src_.get()); - PostDoWorkContinuation(NextTaskDelay(base::TimeDelta::FromSeconds(1), - runners_[0]->GetTimeDomain()), - &lazy_now); + PostDoWorkContinuation(base::TimeDelta::FromSeconds(1), &lazy_now); EXPECT_TRUE(test_task_runner_->HasPendingTasks()); EXPECT_EQ(0, immediate_do_work_posted_count()); EXPECT_EQ(base::TimeDelta::FromSeconds(1), test_task_runner_->DelayToNextTaskTime()); EXPECT_EQ(lazy_now.Now() + base::TimeDelta::FromSeconds(1), - next_delayed_do_work_time()); + next_scheduled_delayed_do_work_time()); - PostDoWorkContinuation(NextTaskDelay(base::TimeDelta::FromSeconds(10), - runners_[0]->GetTimeDomain()), - &lazy_now); + PostDoWorkContinuation(base::TimeDelta::FromSeconds(10), &lazy_now); // This should have resulted in the previous task getting canceled and a new // one getting posted. @@ -2642,7 +2480,7 @@ test_task_runner_->DelayToNextTaskTime()); EXPECT_EQ(0, immediate_do_work_posted_count()); EXPECT_EQ(lazy_now.Now() + base::TimeDelta::FromSeconds(10), - next_delayed_do_work_time()); + next_scheduled_delayed_do_work_time()); } TEST_F(TaskQueueManagerTest, @@ -2650,20 +2488,18 @@ Initialize(1u); LazyNow lazy_now(now_src_.get()); - PostDoWorkContinuation(NextTaskDelay(base::TimeDelta::FromSeconds(1), - runners_[0]->GetTimeDomain()), - &lazy_now); + PostDoWorkContinuation(base::TimeDelta::FromSeconds(1), &lazy_now); now_src_->Advance(base::TimeDelta::FromSeconds(1)); lazy_now = LazyNow(now_src_.get()); - PostDoWorkContinuation(NextTaskDelay(), &lazy_now); + PostDoWorkContinuation(base::TimeDelta(), &lazy_now); // Because the delayed DoWork was pending we don't expect an immediate DoWork // to get posted. EXPECT_EQ(1u, test_task_runner_->NumPendingTasks()); EXPECT_EQ(base::TimeDelta(), test_task_runner_->DelayToNextTaskTime()); EXPECT_EQ(0, immediate_do_work_posted_count()); - EXPECT_EQ(lazy_now.Now(), next_delayed_do_work_time()); + EXPECT_EQ(lazy_now.Now(), next_scheduled_delayed_do_work_time()); } namespace { @@ -2764,98 +2600,5 @@ EXPECT_TRUE(runners_[0]->CouldTaskRun(enqueue_order)); } -TEST_F(TaskQueueManagerTest, DelayedDoWorkNotPostedForDisabledQueue) { - Initialize(1u); - - runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), - base::TimeDelta::FromMilliseconds(1)); - ASSERT_TRUE(test_task_runner_->HasPendingTasks()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds(1), - test_task_runner_->DelayToNextTaskTime()); - - std::unique_ptr<TaskQueue::QueueEnabledVoter> voter = - runners_[0]->CreateQueueEnabledVoter(); - voter->SetQueueEnabled(false); - - EXPECT_TRUE(test_task_runner_->HasPendingTasks()); - test_task_runner_->RemoveCancelledTasks(); - EXPECT_FALSE(test_task_runner_->HasPendingTasks()); - - voter->SetQueueEnabled(true); - ASSERT_TRUE(test_task_runner_->HasPendingTasks()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds(1), - test_task_runner_->DelayToNextTaskTime()); -} - -TEST_F(TaskQueueManagerTest, DisablingQueuesChangesDelayTillNextDoWork) { - Initialize(3u); - runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), - base::TimeDelta::FromMilliseconds(1)); - runners_[1]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), - base::TimeDelta::FromMilliseconds(10)); - runners_[2]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), - base::TimeDelta::FromMilliseconds(100)); - - std::unique_ptr<TaskQueue::QueueEnabledVoter> voter0 = - runners_[0]->CreateQueueEnabledVoter(); - std::unique_ptr<TaskQueue::QueueEnabledVoter> voter1 = - runners_[1]->CreateQueueEnabledVoter(); - std::unique_ptr<TaskQueue::QueueEnabledVoter> voter2 = - runners_[2]->CreateQueueEnabledVoter(); - - ASSERT_TRUE(test_task_runner_->HasPendingTasks()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds(1), - test_task_runner_->DelayToNextTaskTime()); - - voter0->SetQueueEnabled(false); - test_task_runner_->RemoveCancelledTasks(); - ASSERT_TRUE(test_task_runner_->HasPendingTasks()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds(10), - test_task_runner_->DelayToNextTaskTime()); - - voter1->SetQueueEnabled(false); - test_task_runner_->RemoveCancelledTasks(); - ASSERT_TRUE(test_task_runner_->HasPendingTasks()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds(100), - test_task_runner_->DelayToNextTaskTime()); - - voter2->SetQueueEnabled(false); - test_task_runner_->RemoveCancelledTasks(); - EXPECT_FALSE(test_task_runner_->HasPendingTasks()); -} - -TEST_F(TaskQueueManagerTest, GetNextScheduledWakeUp) { - Initialize(1u); - - EXPECT_EQ(base::nullopt, runners_[0]->GetNextScheduledWakeUp()); - - base::TimeTicks start_time = manager_->delegate()->NowTicks(); - base::TimeDelta delay1 = base::TimeDelta::FromMilliseconds(10); - base::TimeDelta delay2 = base::TimeDelta::FromMilliseconds(2); - - runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), delay1); - EXPECT_EQ(start_time + delay1, runners_[0]->GetNextScheduledWakeUp()); - - runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), delay2); - EXPECT_EQ(start_time + delay2, runners_[0]->GetNextScheduledWakeUp()); - - // We don't have wakeups scheduled for disabled queues. - std::unique_ptr<TaskQueue::QueueEnabledVoter> voter = - runners_[0]->CreateQueueEnabledVoter(); - voter->SetQueueEnabled(false); - EXPECT_EQ(base::nullopt, runners_[0]->GetNextScheduledWakeUp()); - - voter->SetQueueEnabled(true); - EXPECT_EQ(start_time + delay2, runners_[0]->GetNextScheduledWakeUp()); - - // Immediate tasks shouldn't make any difference. - runners_[0]->PostTask(FROM_HERE, base::Bind(&NopTask)); - EXPECT_EQ(start_time + delay2, runners_[0]->GetNextScheduledWakeUp()); - - // Neither should fences. - runners_[0]->InsertFence(TaskQueue::InsertFencePosition::BEGINNING_OF_TIME); - EXPECT_EQ(start_time + delay2, runners_[0]->GetNextScheduledWakeUp()); -} - } // namespace scheduler } // namespace blink
diff --git a/third_party/WebKit/Source/platform/scheduler/base/task_queue_selector_unittest.cc b/third_party/WebKit/Source/platform/scheduler/base/task_queue_selector_unittest.cc index cbc6e9d..2e12e70 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/task_queue_selector_unittest.cc +++ b/third_party/WebKit/Source/platform/scheduler/base/task_queue_selector_unittest.cc
@@ -408,8 +408,6 @@ MockObserver mock_observer; selector.SetTaskQueueSelectorObserver(&mock_observer); - EXPECT_CALL(mock_observer, OnTaskQueueEnabled(_)).Times(1); - scoped_refptr<TaskQueueImpl> task_queue(NewTaskQueueWithBlockReporting()); selector.AddQueue(task_queue.get()); std::unique_ptr<TaskQueue::QueueEnabledVoter> voter = @@ -469,8 +467,6 @@ EXPECT_FALSE(selector.SelectWorkQueueToService(&chosen_work_queue)); testing::Mock::VerifyAndClearExpectations(&mock_observer); - EXPECT_CALL(mock_observer, OnTaskQueueEnabled(_)).Times(2); - voter.reset(); selector.EnableQueue(task_queue.get());
diff --git a/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc b/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc index 161830e..919543e9 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc +++ b/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc
@@ -52,8 +52,10 @@ queue->set_scheduled_time_domain_wakeup(delayed_run_time); // If |queue| is the first wakeup then request the wakeup. - if (delayed_wakeup_queue_.min().queue == queue) - RequestWakeupAt(now, delayed_run_time); + if (delayed_wakeup_queue_.min().queue == queue) { + base::TimeDelta delay = std::max(base::TimeDelta(), delayed_run_time - now); + RequestWakeup(now, delay); + } if (observer_) observer_->OnTimeDomainHasDelayedWork(queue); @@ -73,18 +75,9 @@ return; DCHECK_NE(queue->scheduled_time_domain_wakeup(), base::TimeTicks()); - DCHECK(!delayed_wakeup_queue_.empty()); - base::TimeTicks prev_first_wakeup = delayed_wakeup_queue_.min().time; // O(log n) delayed_wakeup_queue_.erase(queue->heap_handle()); - - if (delayed_wakeup_queue_.empty()) { - CancelWakeupAt(prev_first_wakeup); - } else if (prev_first_wakeup != delayed_wakeup_queue_.min().time) { - CancelWakeupAt(prev_first_wakeup); - RequestWakeupAt(Now(), delayed_wakeup_queue_.min().time); - } } void TimeDomain::WakeupReadyDelayedQueues(LazyNow* lazy_now) {
diff --git a/third_party/WebKit/Source/platform/scheduler/base/time_domain.h b/third_party/WebKit/Source/platform/scheduler/base/time_domain.h index c2b828a8b..09e3f70a 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/time_domain.h +++ b/third_party/WebKit/Source/platform/scheduler/base/time_domain.h
@@ -111,18 +111,12 @@ virtual void OnRegisterWithTaskQueueManager( TaskQueueManager* task_queue_manager) = 0; - // The implementation will schedule task processing to run at time |run_time| - // within the TimeDomain's time line. Only called from the main thread. + // The implementaion will secedule task processing to run with |delay| with + // respect to the TimeDomain's time source. Always called on the main thread. // NOTE this is only called by ScheduleDelayedWork if the scheduled runtime // is sooner than any previously sheduled work or if there is no other // scheduled work. - virtual void RequestWakeupAt(base::TimeTicks now, - base::TimeTicks run_time) = 0; - - // The implementation will cancel a wake up previously requested by - // RequestWakeupAt. It's expected this will be a NOP for most virtual time - // domains. - virtual void CancelWakeupAt(base::TimeTicks run_time) = 0; + virtual void RequestWakeup(base::TimeTicks now, base::TimeDelta delay) = 0; // For implementation specific tracing. virtual void AsValueIntoInternal(
diff --git a/third_party/WebKit/Source/platform/scheduler/base/time_domain_unittest.cc b/third_party/WebKit/Source/platform/scheduler/base/time_domain_unittest.cc index c8b6acb..f8eaf85 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/time_domain_unittest.cc +++ b/third_party/WebKit/Source/platform/scheduler/base/time_domain_unittest.cc
@@ -53,10 +53,7 @@ void OnRegisterWithTaskQueueManager( TaskQueueManager* task_queue_manager) override {} - MOCK_METHOD2(RequestWakeupAt, - void(base::TimeTicks now, base::TimeTicks run_time)); - - MOCK_METHOD1(CancelWakeupAt, void(base::TimeTicks run_time)); + MOCK_METHOD2(RequestWakeup, void(base::TimeTicks now, base::TimeDelta delay)); void SetNow(base::TimeTicks now) { now_ = now; } @@ -92,7 +89,7 @@ TEST_F(TimeDomainTest, ScheduleDelayedWork) { base::TimeDelta delay = base::TimeDelta::FromMilliseconds(10); base::TimeTicks delayed_runtime = time_domain_->Now() + delay; - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, delayed_runtime)); + EXPECT_CALL(*time_domain_.get(), RequestWakeup(_, delay)); base::TimeTicks now = time_domain_->Now(); time_domain_->ScheduleDelayedWork(task_queue_.get(), now + delay, now); @@ -103,9 +100,6 @@ TaskQueue* next_task_queue; EXPECT_TRUE(time_domain_->NextScheduledTaskQueue(&next_task_queue)); EXPECT_EQ(task_queue_.get(), next_task_queue); - Mock::VerifyAndClearExpectations(time_domain_.get()); - - EXPECT_CALL(*time_domain_.get(), CancelWakeupAt(_)).Times(AnyNumber()); } TEST_F(TimeDomainTest, ScheduleDelayedWorkSupersedesPreviousWakeup) { @@ -113,7 +107,7 @@ base::TimeDelta delay2 = base::TimeDelta::FromMilliseconds(100); base::TimeTicks delayed_runtime1 = time_domain_->Now() + delay1; base::TimeTicks delayed_runtime2 = time_domain_->Now() + delay2; - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, delayed_runtime1)); + EXPECT_CALL(*time_domain_.get(), RequestWakeup(_, delay1)); base::TimeTicks now = time_domain_->Now(); time_domain_->ScheduleDelayedWork(task_queue_.get(), delayed_runtime1, now); @@ -125,17 +119,14 @@ // Now scheduler a later wakeup, which should replace the previously requested // one. - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, delayed_runtime2)); + EXPECT_CALL(*time_domain_.get(), RequestWakeup(_, delay2)); time_domain_->ScheduleDelayedWork(task_queue_.get(), delayed_runtime2, now); EXPECT_TRUE(time_domain_->NextScheduledRunTime(&next_scheduled_runtime)); EXPECT_EQ(delayed_runtime2, next_scheduled_runtime); - Mock::VerifyAndClearExpectations(time_domain_.get()); - - EXPECT_CALL(*time_domain_.get(), CancelWakeupAt(_)).Times(AnyNumber()); } -TEST_F(TimeDomainTest, RequestWakeupAt_OnlyCalledForEarlierTasks) { +TEST_F(TimeDomainTest, RequestWakeup_OnlyCalledForEarlierTasks) { scoped_refptr<internal::TaskQueueImpl> task_queue2 = make_scoped_refptr( new internal::TaskQueueImpl(nullptr, time_domain_.get(), TaskQueue::Spec(TaskQueue::QueueType::TEST), @@ -156,27 +147,23 @@ base::TimeDelta delay3 = base::TimeDelta::FromMilliseconds(30); base::TimeDelta delay4 = base::TimeDelta::FromMilliseconds(1); - // RequestWakeupAt should always be called if there are no other wakeups. + // RequestWakeup should always be called if there are no other wakeups. + EXPECT_CALL(*time_domain_.get(), RequestWakeup(_, delay1)); base::TimeTicks now = time_domain_->Now(); - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, now + delay1)); time_domain_->ScheduleDelayedWork(task_queue_.get(), now + delay1, now); Mock::VerifyAndClearExpectations(time_domain_.get()); - // RequestWakeupAt should not be called when scheduling later tasks. - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, _)).Times(0); + // RequestWakeup should not be called when scheduling later tasks. + EXPECT_CALL(*time_domain_.get(), RequestWakeup(_, _)).Times(0); time_domain_->ScheduleDelayedWork(task_queue2.get(), now + delay2, now); time_domain_->ScheduleDelayedWork(task_queue3.get(), now + delay3, now); - // RequestWakeupAt should be called when scheduling earlier tasks. + // RequestWakeup should be called when scheduling earlier tasks. Mock::VerifyAndClearExpectations(time_domain_.get()); - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, now + delay4)); + EXPECT_CALL(*time_domain_.get(), RequestWakeup(_, delay4)); time_domain_->ScheduleDelayedWork(task_queue4.get(), now + delay4, now); - Mock::VerifyAndClearExpectations(time_domain_.get()); - - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, _)); - EXPECT_CALL(*time_domain_.get(), CancelWakeupAt(_)).Times(2); task_queue2->UnregisterTaskQueue(); task_queue3->UnregisterTaskQueue(); task_queue4->UnregisterTaskQueue(); @@ -188,40 +175,31 @@ TaskQueue::Spec(TaskQueue::QueueType::TEST), "test.category", "test.category")); + EXPECT_CALL(*time_domain_.get(), RequestWakeup(_, _)).Times(1); base::TimeTicks now = time_domain_->Now(); - base::TimeTicks wakeup1 = now + base::TimeDelta::FromMilliseconds(10); - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, wakeup1)).Times(1); - time_domain_->ScheduleDelayedWork(task_queue_.get(), wakeup1, now); - base::TimeTicks wakeup2 = now + base::TimeDelta::FromMilliseconds(100); - time_domain_->ScheduleDelayedWork(task_queue2_.get(), wakeup2, now); + time_domain_->ScheduleDelayedWork( + task_queue_.get(), now + base::TimeDelta::FromMilliseconds(10), now); + time_domain_->ScheduleDelayedWork( + task_queue2_.get(), now + base::TimeDelta::FromMilliseconds(100), now); TaskQueue* next_task_queue; EXPECT_TRUE(time_domain_->NextScheduledTaskQueue(&next_task_queue)); EXPECT_EQ(task_queue_.get(), next_task_queue); - testing::Mock::VerifyAndClearExpectations(time_domain_.get()); - - EXPECT_CALL(*time_domain_.get(), CancelWakeupAt(wakeup1)).Times(1); - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, wakeup2)).Times(1); - time_domain_->UnregisterQueue(task_queue_.get()); task_queue_ = scoped_refptr<internal::TaskQueueImpl>(); EXPECT_TRUE(time_domain_->NextScheduledTaskQueue(&next_task_queue)); EXPECT_EQ(task_queue2_.get(), next_task_queue); - testing::Mock::VerifyAndClearExpectations(time_domain_.get()); - - EXPECT_CALL(*time_domain_.get(), CancelWakeupAt(wakeup2)).Times(1); - time_domain_->UnregisterQueue(task_queue2_.get()); EXPECT_FALSE(time_domain_->NextScheduledTaskQueue(&next_task_queue)); } TEST_F(TimeDomainTest, WakeupReadyDelayedQueues) { base::TimeDelta delay = base::TimeDelta::FromMilliseconds(50); + EXPECT_CALL(*time_domain_.get(), RequestWakeup(_, delay)); base::TimeTicks now = time_domain_->Now(); base::TimeTicks delayed_runtime = now + delay; - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, delayed_runtime)); time_domain_->ScheduleDelayedWork(task_queue_.get(), delayed_runtime, now); base::TimeTicks next_run_time; @@ -241,62 +219,17 @@ TEST_F(TimeDomainTest, CancelDelayedWork) { base::TimeTicks now = time_domain_->Now(); - base::TimeTicks run_time = now + base::TimeDelta::FromMilliseconds(20); - - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, run_time)); - time_domain_->ScheduleDelayedWork(task_queue_.get(), run_time, now); + time_domain_->ScheduleDelayedWork( + task_queue_.get(), now + base::TimeDelta::FromMilliseconds(20), now); TaskQueue* next_task_queue; EXPECT_TRUE(time_domain_->NextScheduledTaskQueue(&next_task_queue)); EXPECT_EQ(task_queue_.get(), next_task_queue); - EXPECT_CALL(*time_domain_.get(), CancelWakeupAt(run_time)); time_domain_->CancelDelayedWork(task_queue_.get()); EXPECT_FALSE(time_domain_->NextScheduledTaskQueue(&next_task_queue)); } -TEST_F(TimeDomainTest, CancelDelayedWork_TwoQueues) { - scoped_refptr<internal::TaskQueueImpl> task_queue2 = make_scoped_refptr( - new internal::TaskQueueImpl(nullptr, time_domain_.get(), - TaskQueue::Spec(TaskQueue::QueueType::TEST), - "test.category", "test.category")); - - base::TimeTicks now = time_domain_->Now(); - base::TimeTicks run_time1 = now + base::TimeDelta::FromMilliseconds(20); - base::TimeTicks run_time2 = now + base::TimeDelta::FromMilliseconds(40); - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, run_time1)); - time_domain_->ScheduleDelayedWork(task_queue_.get(), run_time1, now); - Mock::VerifyAndClearExpectations(time_domain_.get()); - - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, _)).Times(0); - time_domain_->ScheduleDelayedWork(task_queue2.get(), run_time2, now); - Mock::VerifyAndClearExpectations(time_domain_.get()); - - TaskQueue* next_task_queue; - EXPECT_TRUE(time_domain_->NextScheduledTaskQueue(&next_task_queue)); - EXPECT_EQ(task_queue_.get(), next_task_queue); - - base::TimeTicks next_run_time; - ASSERT_TRUE(time_domain_->NextScheduledRunTime(&next_run_time)); - EXPECT_EQ(run_time1, next_run_time); - - EXPECT_CALL(*time_domain_.get(), CancelWakeupAt(run_time1)); - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, run_time2)); - time_domain_->CancelDelayedWork(task_queue_.get()); - EXPECT_TRUE(time_domain_->NextScheduledTaskQueue(&next_task_queue)); - EXPECT_EQ(task_queue2.get(), next_task_queue); - - ASSERT_TRUE(time_domain_->NextScheduledRunTime(&next_run_time)); - EXPECT_EQ(run_time2, next_run_time); - - Mock::VerifyAndClearExpectations(time_domain_.get()); - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, _)).Times(AnyNumber()); - EXPECT_CALL(*time_domain_.get(), CancelWakeupAt(_)).Times(AnyNumber()); - - // Tidy up. - task_queue2->UnregisterTaskQueue(); -} - namespace { class MockObserver : public TimeDomain::Observer { public: @@ -324,7 +257,7 @@ TEST_F(TimeDomainWithObserverTest, OnTimeDomainHasDelayedWork) { EXPECT_CALL(*observer_, OnTimeDomainHasDelayedWork(task_queue_.get())); - EXPECT_CALL(*time_domain_.get(), RequestWakeupAt(_, _)); + EXPECT_CALL(*time_domain_.get(), RequestWakeup(_, _)); base::TimeTicks now = time_domain_->Now(); time_domain_->ScheduleDelayedWork( task_queue_.get(), now + base::TimeDelta::FromMilliseconds(10), now);
diff --git a/third_party/WebKit/Source/platform/scheduler/base/virtual_time_domain.cc b/third_party/WebKit/Source/platform/scheduler/base/virtual_time_domain.cc index 56c7d37..a90b3be2 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/virtual_time_domain.cc +++ b/third_party/WebKit/Source/platform/scheduler/base/virtual_time_domain.cc
@@ -34,17 +34,13 @@ return now_; } -void VirtualTimeDomain::RequestWakeupAt(base::TimeTicks now, - base::TimeTicks run_time) { +void VirtualTimeDomain::RequestWakeup(base::TimeTicks now, + base::TimeDelta delay) { // We don't need to do anything here because the caller of AdvanceTo is // responsible for calling TaskQueueManager::MaybeScheduleImmediateWork if // needed. } -void VirtualTimeDomain::CancelWakeupAt(base::TimeTicks run_time) { - // We ignore this because RequestWakeupAt is a NOP. -} - base::Optional<base::TimeDelta> VirtualTimeDomain::DelayTillNextTask( LazyNow* lazy_now) { return base::nullopt;
diff --git a/third_party/WebKit/Source/platform/scheduler/base/virtual_time_domain.h b/third_party/WebKit/Source/platform/scheduler/base/virtual_time_domain.h index a1503f0..a841226 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/virtual_time_domain.h +++ b/third_party/WebKit/Source/platform/scheduler/base/virtual_time_domain.h
@@ -32,8 +32,7 @@ protected: void OnRegisterWithTaskQueueManager( TaskQueueManager* task_queue_manager) override; - void RequestWakeupAt(base::TimeTicks now, base::TimeTicks run_time) override; - void CancelWakeupAt(base::TimeTicks run_time) override; + void RequestWakeup(base::TimeTicks now, base::TimeDelta delay) override; void AsValueIntoInternal( base::trace_event::TracedValue* state) const override;
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/auto_advancing_virtual_time_domain.cc b/third_party/WebKit/Source/platform/scheduler/renderer/auto_advancing_virtual_time_domain.cc index 57a44ed..04dd639 100644 --- a/third_party/WebKit/Source/platform/scheduler/renderer/auto_advancing_virtual_time_domain.cc +++ b/third_party/WebKit/Source/platform/scheduler/renderer/auto_advancing_virtual_time_domain.cc
@@ -24,18 +24,14 @@ return base::TimeDelta(); // Makes DoWork post an immediate continuation. } -void AutoAdvancingVirtualTimeDomain::RequestWakeupAt(base::TimeTicks now, - base::TimeTicks run_time) { +void AutoAdvancingVirtualTimeDomain::RequestWakeup(base::TimeTicks now, + base::TimeDelta delay) { // Avoid posting pointless DoWorks. I.e. if the time domain has more then one // scheduled wake up then we don't need to do anything. if (can_advance_virtual_time_ && NumberOfScheduledWakeups() == 1u) RequestDoWork(); } -void AutoAdvancingVirtualTimeDomain::CancelWakeupAt(base::TimeTicks run_time) { - // We ignore this because RequestWakeupAt doesn't post a delayed task. -} - void AutoAdvancingVirtualTimeDomain::SetCanAdvanceVirtualTime( bool can_advance_virtual_time) { can_advance_virtual_time_ = can_advance_virtual_time;
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/auto_advancing_virtual_time_domain.h b/third_party/WebKit/Source/platform/scheduler/renderer/auto_advancing_virtual_time_domain.h index 9cdd5d8..0ca186d 100644 --- a/third_party/WebKit/Source/platform/scheduler/renderer/auto_advancing_virtual_time_domain.h +++ b/third_party/WebKit/Source/platform/scheduler/renderer/auto_advancing_virtual_time_domain.h
@@ -28,8 +28,7 @@ // TimeDomain implementation: base::Optional<base::TimeDelta> DelayTillNextTask(LazyNow* lazy_now) override; - void RequestWakeupAt(base::TimeTicks now, base::TimeTicks run_time) override; - void CancelWakeupAt(base::TimeTicks run_time) override; + void RequestWakeup(base::TimeTicks now, base::TimeDelta delay) override; const char* GetName() const override; // Controls whether or not virtual time is allowed to advance, when the
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler_unittest.cc b/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler_unittest.cc index 79cef1c..d5fdb53 100644 --- a/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler_unittest.cc +++ b/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler_unittest.cc
@@ -979,41 +979,5 @@ base::TimeDelta::FromMilliseconds(300))); } -TEST_F(TaskQueueThrottlerTest, DisabledQueueThenEnabledQueue) { - std::vector<base::TimeTicks> run_times; - - scoped_refptr<TaskQueue> second_queue = - scheduler_->NewTimerTaskRunner(TaskQueue::QueueType::TEST); - - task_queue_throttler_->IncreaseThrottleRefCount(timer_queue_.get()); - task_queue_throttler_->IncreaseThrottleRefCount(second_queue.get()); - - timer_queue_->PostDelayedTask(FROM_HERE, - base::Bind(&TestTask, &run_times, clock_.get()), - base::TimeDelta::FromMilliseconds(100)); - second_queue->PostDelayedTask(FROM_HERE, - base::Bind(&TestTask, &run_times, clock_.get()), - base::TimeDelta::FromMilliseconds(200)); - - std::unique_ptr<TaskQueue::QueueEnabledVoter> voter = - timer_queue_->CreateQueueEnabledVoter(); - voter->SetQueueEnabled(false); - - clock_->Advance(base::TimeDelta::FromMilliseconds(250)); - - mock_task_runner_->RunUntilIdle(); - - EXPECT_THAT(run_times, ElementsAre(base::TimeTicks() + - base::TimeDelta::FromMilliseconds(1000))); - - voter->SetQueueEnabled(true); - mock_task_runner_->RunUntilIdle(); - - EXPECT_THAT( - run_times, - ElementsAre(base::TimeTicks() + base::TimeDelta::FromMilliseconds(1000), - base::TimeTicks() + base::TimeDelta::FromMilliseconds(2000))); -} - } // namespace scheduler } // namespace blink
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.cc b/third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.cc index 348d034..a859d09 100644 --- a/third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.cc +++ b/third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.cc
@@ -17,16 +17,12 @@ return "ThrottledTimeDomain"; } -void ThrottledTimeDomain::RequestWakeupAt(base::TimeTicks now, - base::TimeTicks run_time) { +void ThrottledTimeDomain::RequestWakeup(base::TimeTicks now, + base::TimeDelta delay) { // We assume the owner (i.e. TaskQueueThrottler) will manage wakeups on our // behalf. } -void ThrottledTimeDomain::CancelWakeupAt(base::TimeTicks run_time) { - // We ignore this because RequestWakeupAt is a NOP. -} - base::Optional<base::TimeDelta> ThrottledTimeDomain::DelayTillNextTask( LazyNow* lazy_now) { base::TimeTicks next_run_time;
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.h b/third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.h index 4faaf5b..c9d73fa 100644 --- a/third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.h +++ b/third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.h
@@ -21,8 +21,7 @@ // TimeDomain implementation: const char* GetName() const override; - void RequestWakeupAt(base::TimeTicks now, base::TimeTicks run_time) override; - void CancelWakeupAt(base::TimeTicks run_time) override; + void RequestWakeup(base::TimeTicks now, base::TimeDelta delay) override; base::Optional<base::TimeDelta> DelayTillNextTask(LazyNow* lazy_now) override; using TimeDomain::WakeupReadyDelayedQueues;
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/common.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/common.py index 17b8b89..156d740 100644 --- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/common.py +++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/common.py
@@ -12,6 +12,7 @@ WPT_DEST_NAME = 'wpt' CSS_DEST_NAME = 'csswg-test' +WPT_GH_REPO_URL = 'git@github.com:w3c/web-platform-tests.git' # TODO(qyearsley): This directory should be able to be constructed with # WebKitFinder and WPT_DEST_NAME, plus the string "external". @@ -21,6 +22,7 @@ WPT_REPO_URL = 'https://chromium.googlesource.com/external/w3c/web-platform-tests.git' CSS_REPO_URL = 'https://chromium.googlesource.com/external/w3c/csswg-test.git' + _log = logging.getLogger(__name__)
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/local_wpt.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/local_wpt.py index 0081f9e..6c9b0ec 100644 --- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/local_wpt.py +++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/local_wpt.py
@@ -8,12 +8,9 @@ from webkitpy.common.system.executive import ScriptError from webkitpy.w3c.chromium_commit import ChromiumCommit -from webkitpy.w3c.common import WPT_REPO_URL, CHROMIUM_WPT_DIR +from webkitpy.w3c.common import WPT_GH_REPO_URL, CHROMIUM_WPT_DIR -WPT_SSH_URL = 'git@github.com:w3c/web-platform-tests.git' -REMOTE_NAME = 'github' - _log = logging.getLogger(__name__) @@ -34,14 +31,11 @@ def fetch(self): if self.host.filesystem.exists(self.path): _log.info('WPT checkout exists at %s, fetching latest', self.path) - self.run(['git', 'fetch', '--all']) + self.run(['git', 'fetch', 'origin']) self.run(['git', 'checkout', 'origin/master']) else: - _log.info('Cloning %s into %s', WPT_REPO_URL, self.path) - self.host.executive.run_command(['git', 'clone', WPT_REPO_URL, self.path]) - - if REMOTE_NAME not in self.run(['git', 'remote']): - self.run(['git', 'remote', 'add', REMOTE_NAME, WPT_SSH_URL]) + _log.info('Cloning %s into %s', WPT_GH_REPO_URL, self.path) + self.host.executive.run_command(['git', 'clone', WPT_GH_REPO_URL, self.path]) def run(self, command, **kwargs): """Runs a command in the local WPT directory.""" @@ -97,7 +91,7 @@ self.run(['git', 'apply', '-'], input=patch) self.run(['git', 'add', '.']) self.run(['git', 'commit', '--author', author, '-am', message]) - self.run(['git', 'push', '-f', REMOTE_NAME, self.branch_name]) + self.run(['git', 'push', '-f', 'origin', self.branch_name]) return self.branch_name
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/local_wpt_unittest.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/local_wpt_unittest.py index e8c8fd2..dbb0aab0 100644 --- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/local_wpt_unittest.py +++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/local_wpt_unittest.py
@@ -22,10 +22,8 @@ local_wpt.fetch() self.assertEqual(host.executive.calls, [ - ['git', 'fetch', '--all'], + ['git', 'fetch', 'origin'], ['git', 'checkout', 'origin/master'], - ['git', 'remote'], - ['git', 'remote', 'add', 'github', 'git@github.com:w3c/web-platform-tests.git'] ]) def test_fetch_when_wpt_dir_does_not_exist(self): @@ -35,8 +33,9 @@ local_wpt = LocalWPT(host) local_wpt.fetch() - self.assertEqual(len(host.executive.calls), 3) - self.assertEqual(host.executive.calls[0][1], 'clone') + self.assertEqual(host.executive.calls, [ + ['git', 'clone', 'git@github.com:w3c/web-platform-tests.git', '/tmp/wpt'], + ]) def test_constructor(self): host = MockHost() @@ -84,9 +83,7 @@ local_branch_name = local_wpt.create_branch_with_patch('message', 'patch', 'author') self.assertEqual(local_branch_name, 'chromium-export-try') self.assertEqual(host.executive.calls, [ - ['git', 'clone', 'https://chromium.googlesource.com/external/w3c/web-platform-tests.git', '/tmp/wpt'], - ['git', 'remote'], - ['git', 'remote', 'add', 'github', 'git@github.com:w3c/web-platform-tests.git'], + ['git', 'clone', 'git@github.com:w3c/web-platform-tests.git', '/tmp/wpt'], ['git', 'reset', '--hard', 'HEAD'], ['git', 'clean', '-fdx'], ['git', 'checkout', 'origin/master'], @@ -96,4 +93,4 @@ ['git', 'apply', '-'], ['git', 'add', '.'], ['git', 'commit', '--author', 'author', '-am', 'message'], - ['git', 'push', '-f', 'github', 'chromium-export-try']]) + ['git', 'push', '-f', 'origin', 'chromium-export-try']])
diff --git a/third_party/WebKit/public/platform/scheduler/base/task_queue.h b/third_party/WebKit/public/platform/scheduler/base/task_queue.h index c57d237..07335fb0 100644 --- a/third_party/WebKit/public/platform/scheduler/base/task_queue.h +++ b/third_party/WebKit/public/platform/scheduler/base/task_queue.h
@@ -150,9 +150,8 @@ // NOTE: this must be called on the thread this TaskQueue was created by. virtual bool HasPendingImmediateWork() const = 0; - // Returns requested run time of next scheduled wakeup for a delayed task - // which is not ready to run. If there are no such tasks or the queue is - // disabled (by a QueueEnabledVoter) it returns base::nullopt. + // Returns requested run time of next delayed task, which is not ready + // to run. If there are no such tasks, returns base::nullopt. // NOTE: this must be called on the thread this TaskQueue was created by. virtual base::Optional<base::TimeTicks> GetNextScheduledWakeUp() = 0;
diff --git a/third_party/widevine/cdm/BUILD.gn b/third_party/widevine/cdm/BUILD.gn index eebe2ab5..34ae399f 100644 --- a/third_party/widevine/cdm/BUILD.gn +++ b/third_party/widevine/cdm/BUILD.gn
@@ -37,6 +37,7 @@ widevine_cdm_binary_files = [ "win/$widevine_arch/widevinecdm.dll", "win/$widevine_arch/widevinecdm.dll.lib", + "win/$widevine_arch/widevinecdm.dll.sig", ] widevine_cdm_manifest_file = [ "win/$widevine_arch/manifest.json" ] } else if (is_mac) {
diff --git a/ui/aura/mus/window_tree_client.cc b/ui/aura/mus/window_tree_client.cc index d5cef71..08e7b02 100644 --- a/ui/aura/mus/window_tree_client.cc +++ b/ui/aura/mus/window_tree_client.cc
@@ -178,6 +178,7 @@ tree_(nullptr), in_destructor_(false), weak_factory_(this) { + DCHECK(delegate_); // Allow for a null request in tests. if (request.is_pending()) binding_.Bind(std::move(request));
diff --git a/ui/base/cocoa/touch_bar_forward_declarations.h b/ui/base/cocoa/touch_bar_forward_declarations.h index 52280c2..0e33268 100644 --- a/ui/base/cocoa/touch_bar_forward_declarations.h +++ b/ui/base/cocoa/touch_bar_forward_declarations.h
@@ -24,6 +24,12 @@ typedef NSString* NSTouchBarItemIdentifier; typedef NSString* NSTouchBarCustomizationIdentifier; +static const NSTouchBarItemIdentifier NSTouchBarItemIdentifierFixedSpaceSmall = + @"NSTouchBarItemIdentifierFixedSpaceSmall"; + +static const NSTouchBarItemIdentifier NSTouchBarItemIdentifierFlexibleSpace = + @"NSTouchBarItemIdentifierFlexibleSpace"; + @interface NSTouchBar : NSObject<NSCoding> - (instancetype)init NS_DESIGNATED_INITIALIZER; @@ -104,6 +110,10 @@ @protocol NSTouchBarDelegate; @class NSTouchBarItem; +@interface NSWindow (TouchBarSDK) +@property(strong, readonly) NSTouchBar* touchBar; +@end + #endif // MAC_OS_X_VERSION_10_12_1 #endif // UI_BASE_COCOA_TOUCH_BAR_FORWARD_DECLARATIONS_H_
diff --git a/ui/events/blink/blink_event_util.cc b/ui/events/blink/blink_event_util.cc index 6378ce2..ac5a14aa 100644 --- a/ui/events/blink/blink_event_util.cc +++ b/ui/events/blink/blink_event_util.cc
@@ -670,11 +670,17 @@ gesture.data.scrollBegin.pointerCount = details.touch_points(); gesture.data.scrollBegin.deltaXHint = details.scroll_x_hint(); gesture.data.scrollBegin.deltaYHint = details.scroll_y_hint(); + gesture.data.scrollBegin.deltaHintUnits = + static_cast<blink::WebGestureEvent::ScrollUnits>( + details.scroll_begin_units()); break; case ET_GESTURE_SCROLL_UPDATE: gesture.setType(WebInputEvent::GestureScrollUpdate); gesture.data.scrollUpdate.deltaX = details.scroll_x(); gesture.data.scrollUpdate.deltaY = details.scroll_y(); + gesture.data.scrollUpdate.deltaUnits = + static_cast<blink::WebGestureEvent::ScrollUnits>( + details.scroll_update_units()); gesture.data.scrollUpdate.previousUpdateInSequencePrevented = details.previous_scroll_update_in_sequence_prevented(); break; @@ -750,10 +756,12 @@ wheel_event->y += delta.y(); wheel_event->x *= scale; wheel_event->y *= scale; - wheel_event->deltaX *= scale; - wheel_event->deltaY *= scale; - wheel_event->wheelTicksX *= scale; - wheel_event->wheelTicksY *= scale; + if (!wheel_event->scrollByPage) { + wheel_event->deltaX *= scale; + wheel_event->deltaY *= scale; + wheel_event->wheelTicksX *= scale; + wheel_event->wheelTicksY *= scale; + } } else if (blink::WebInputEvent::isMouseEventType(event.type())) { blink::WebMouseEvent* mouse_event = new blink::WebMouseEvent; scaled_event.reset(mouse_event); @@ -788,12 +796,18 @@ gesture_event->y *= scale; switch (gesture_event->type()) { case blink::WebInputEvent::GestureScrollUpdate: - gesture_event->data.scrollUpdate.deltaX *= scale; - gesture_event->data.scrollUpdate.deltaY *= scale; + if (gesture_event->data.scrollUpdate.deltaUnits != + blink::WebGestureEvent::ScrollUnits::Page) { + gesture_event->data.scrollUpdate.deltaX *= scale; + gesture_event->data.scrollUpdate.deltaY *= scale; + } break; case blink::WebInputEvent::GestureScrollBegin: - gesture_event->data.scrollBegin.deltaXHint *= scale; - gesture_event->data.scrollBegin.deltaYHint *= scale; + if (gesture_event->data.scrollBegin.deltaHintUnits != + blink::WebGestureEvent::ScrollUnits::Page) { + gesture_event->data.scrollBegin.deltaXHint *= scale; + gesture_event->data.scrollBegin.deltaYHint *= scale; + } break; case blink::WebInputEvent::GesturePinchUpdate:
diff --git a/ui/events/blink/blink_event_util_unittest.cc b/ui/events/blink/blink_event_util_unittest.cc index 1bee5915..260f636 100644 --- a/ui/events/blink/blink_event_util_unittest.cc +++ b/ui/events/blink/blink_event_util_unittest.cc
@@ -8,6 +8,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "third_party/WebKit/public/platform/WebGestureEvent.h" #include "third_party/WebKit/public/platform/WebInputEvent.h" +#include "third_party/WebKit/public/platform/WebMouseWheelEvent.h" #include "ui/events/gesture_event_details.h" namespace ui { @@ -28,6 +29,108 @@ EXPECT_TRUE(ScaleWebInputEvent(event, 2.f)); } +TEST(BlinkEventUtilTest, NonPaginatedWebMouseWheelEvent) { + blink::WebMouseWheelEvent event(blink::WebInputEvent::MouseWheel, + blink::WebInputEvent::NoModifiers, + blink::WebInputEvent::TimeStampForTesting); + event.deltaX = 1.f; + event.deltaY = 1.f; + event.wheelTicksX = 1.f; + event.wheelTicksY = 1.f; + event.scrollByPage = false; + std::unique_ptr<blink::WebInputEvent> webEvent = + ScaleWebInputEvent(event, 2.f); + EXPECT_TRUE(webEvent); + blink::WebMouseWheelEvent* mouseWheelEvent = + static_cast<blink::WebMouseWheelEvent*>(webEvent.get()); + EXPECT_EQ(2.f, mouseWheelEvent->deltaX); + EXPECT_EQ(2.f, mouseWheelEvent->deltaY); + EXPECT_EQ(2.f, mouseWheelEvent->wheelTicksX); + EXPECT_EQ(2.f, mouseWheelEvent->wheelTicksY); +} + +TEST(BlinkEventUtilTest, PaginatedWebMouseWheelEvent) { + blink::WebMouseWheelEvent event(blink::WebInputEvent::MouseWheel, + blink::WebInputEvent::NoModifiers, + blink::WebInputEvent::TimeStampForTesting); + event.deltaX = 1.f; + event.deltaY = 1.f; + event.wheelTicksX = 1.f; + event.wheelTicksY = 1.f; + event.scrollByPage = true; + std::unique_ptr<blink::WebInputEvent> webEvent = + ScaleWebInputEvent(event, 2.f); + EXPECT_TRUE(webEvent); + blink::WebMouseWheelEvent* mouseWheelEvent = + static_cast<blink::WebMouseWheelEvent*>(webEvent.get()); + EXPECT_EQ(1.f, mouseWheelEvent->deltaX); + EXPECT_EQ(1.f, mouseWheelEvent->deltaY); + EXPECT_EQ(1.f, mouseWheelEvent->wheelTicksX); + EXPECT_EQ(1.f, mouseWheelEvent->wheelTicksY); +} + +TEST(BlinkEventUtilTest, NonPaginatedScrollBeginEvent) { + ui::GestureEventDetails details(ui::ET_GESTURE_SCROLL_BEGIN, 1, 1); + details.set_device_type(ui::GestureDeviceType::DEVICE_TOUCHSCREEN); + auto event = + CreateWebGestureEvent(details, base::TimeTicks(), gfx::PointF(1.f, 1.f), + gfx::PointF(1.f, 1.f), 0, 0U); + std::unique_ptr<blink::WebInputEvent> webEvent = + ScaleWebInputEvent(event, 2.f); + EXPECT_TRUE(webEvent); + blink::WebGestureEvent* gestureEvent = + static_cast<blink::WebGestureEvent*>(webEvent.get()); + EXPECT_EQ(2.f, gestureEvent->data.scrollBegin.deltaXHint); + EXPECT_EQ(2.f, gestureEvent->data.scrollBegin.deltaYHint); +} + +TEST(BlinkEventUtilTest, PaginatedScrollBeginEvent) { + ui::GestureEventDetails details(ui::ET_GESTURE_SCROLL_BEGIN, 1, 1, + ui::GestureEventDetails::ScrollUnits::PAGE); + details.set_device_type(ui::GestureDeviceType::DEVICE_TOUCHSCREEN); + auto event = + CreateWebGestureEvent(details, base::TimeTicks(), gfx::PointF(1.f, 1.f), + gfx::PointF(1.f, 1.f), 0, 0U); + std::unique_ptr<blink::WebInputEvent> webEvent = + ScaleWebInputEvent(event, 2.f); + EXPECT_TRUE(webEvent); + blink::WebGestureEvent* gestureEvent = + static_cast<blink::WebGestureEvent*>(webEvent.get()); + EXPECT_EQ(1.f, gestureEvent->data.scrollBegin.deltaXHint); + EXPECT_EQ(1.f, gestureEvent->data.scrollBegin.deltaYHint); +} + +TEST(BlinkEventUtilTest, NonPaginatedScrollUpdateEvent) { + ui::GestureEventDetails details(ui::ET_GESTURE_SCROLL_UPDATE, 1, 1); + details.set_device_type(ui::GestureDeviceType::DEVICE_TOUCHSCREEN); + auto event = + CreateWebGestureEvent(details, base::TimeTicks(), gfx::PointF(1.f, 1.f), + gfx::PointF(1.f, 1.f), 0, 0U); + std::unique_ptr<blink::WebInputEvent> webEvent = + ScaleWebInputEvent(event, 2.f); + EXPECT_TRUE(webEvent); + blink::WebGestureEvent* gestureEvent = + static_cast<blink::WebGestureEvent*>(webEvent.get()); + EXPECT_EQ(2.f, gestureEvent->data.scrollUpdate.deltaX); + EXPECT_EQ(2.f, gestureEvent->data.scrollUpdate.deltaY); +} + +TEST(BlinkEventUtilTest, PaginatedScrollUpdateEvent) { + ui::GestureEventDetails details(ui::ET_GESTURE_SCROLL_UPDATE, 1, 1, + ui::GestureEventDetails::ScrollUnits::PAGE); + details.set_device_type(ui::GestureDeviceType::DEVICE_TOUCHSCREEN); + auto event = + CreateWebGestureEvent(details, base::TimeTicks(), gfx::PointF(1.f, 1.f), + gfx::PointF(1.f, 1.f), 0, 0U); + std::unique_ptr<blink::WebInputEvent> webEvent = + ScaleWebInputEvent(event, 2.f); + EXPECT_TRUE(webEvent); + blink::WebGestureEvent* gestureEvent = + static_cast<blink::WebGestureEvent*>(webEvent.get()); + EXPECT_EQ(1.f, gestureEvent->data.scrollUpdate.deltaX); + EXPECT_EQ(1.f, gestureEvent->data.scrollUpdate.deltaY); +} + TEST(BlinkEventUtilTest, TouchEventCoalescing) { blink::WebTouchPoint touch_point; touch_point.id = 1;
diff --git a/ui/events/gesture_event_details.cc b/ui/events/gesture_event_details.cc index 8fceecb7a..3203fd4 100644 --- a/ui/events/gesture_event_details.cc +++ b/ui/events/gesture_event_details.cc
@@ -21,7 +21,8 @@ GestureEventDetails::GestureEventDetails(ui::EventType type, float delta_x, - float delta_y) + float delta_y, + ScrollUnits units) : type_(type), device_type_(GestureDeviceType::DEVICE_UNKNOWN), touch_points_(1) { @@ -31,11 +32,13 @@ case ui::ET_GESTURE_SCROLL_BEGIN: data_.scroll_begin.x_hint = delta_x; data_.scroll_begin.y_hint = delta_y; + data_.scroll_begin.delta_hint_units = units; break; case ui::ET_GESTURE_SCROLL_UPDATE: data_.scroll_update.x = delta_x; data_.scroll_update.y = delta_y; + data_.scroll_update.delta_units = units; break; case ui::ET_SCROLL_FLING_START:
diff --git a/ui/events/gesture_event_details.h b/ui/events/gesture_event_details.h index 377c423..2b066d5 100644 --- a/ui/events/gesture_event_details.h +++ b/ui/events/gesture_event_details.h
@@ -17,9 +17,22 @@ struct EVENTS_BASE_EXPORT GestureEventDetails { public: + // The definition of ScrollUnits below is consistent with that in + // //src/third_party/WebKit/public/platform/WebGestureEvent.h + enum class ScrollUnits { + PRECISE_PIXELS = 0, // generated by high precision devices. + PIXELS, // large pixel jump duration; should animate to delta. + PAGE // page (visible viewport) based scrolling. + }; + GestureEventDetails(); explicit GestureEventDetails(EventType type); - GestureEventDetails(EventType type, float delta_x, float delta_y); + + // ScrollUnits::PRECISE_PIXELS is the default in WebGestureEvent.h too. + GestureEventDetails(EventType type, + float delta_x, + float delta_y, + ScrollUnits units = ScrollUnits::PRECISE_PIXELS); // The caller is responsible for ensuring that the gesture data from |other| // is compatible and sufficient for that expected by gestures of |type|. @@ -58,6 +71,11 @@ return data_.scroll_begin.y_hint; } + ScrollUnits scroll_begin_units() const { + DCHECK_EQ(ET_GESTURE_SCROLL_BEGIN, type_); + return data_.scroll_begin.delta_hint_units; + } + float scroll_x() const { DCHECK_EQ(ET_GESTURE_SCROLL_UPDATE, type_); return data_.scroll_update.x; @@ -68,6 +86,11 @@ return data_.scroll_update.y; } + ScrollUnits scroll_update_units() const { + DCHECK_EQ(ET_GESTURE_SCROLL_UPDATE, type_); + return data_.scroll_update.delta_units; + } + float velocity_x() const { DCHECK_EQ(ET_SCROLL_FLING_START, type_); return data_.fling_velocity.x; @@ -162,11 +185,13 @@ // the x/y values from the first scroll_update. float x_hint; float y_hint; + ScrollUnits delta_hint_units; } scroll_begin; struct { // SCROLL delta. float x; float y; + ScrollUnits delta_units; // Whether any previous scroll update in the current scroll sequence was // suppressed because the underlying touch was consumed. bool previous_update_in_sequence_prevented;
diff --git a/ui/gfx/color_transform.cc b/ui/gfx/color_transform.cc index 933252a..1d562d484 100644 --- a/ui/gfx/color_transform.cc +++ b/ui/gfx/color_transform.cc
@@ -11,6 +11,7 @@ #include "base/logging.h" #include "base/memory/ptr_util.h" +#include "base/strings/stringprintf.h" #include "third_party/qcms/src/qcms.h" #include "ui/gfx/color_space.h" #include "ui/gfx/icc_profile.h" @@ -250,8 +251,9 @@ // Return true if this is a null transform. virtual bool IsNull() { return false; } - - virtual void Transform(ColorTransform::TriStim* color, size_t num) = 0; + virtual void Transform(ColorTransform::TriStim* color, size_t num) const = 0; + virtual bool CanAppendShaderSource() { return false; } + virtual void AppendShaderSource(std::string* result) { NOTREACHED(); } private: DISALLOW_COPY_AND_ASSIGN(ColorTransformStep); @@ -264,11 +266,16 @@ Intent intent); ~ColorTransformInternal() override; - // Perform transformation of colors, |colors| is both input and output. - void Transform(TriStim* colors, size_t num) override { + gfx::ColorSpace GetSrcColorSpace() const override { return src_; }; + gfx::ColorSpace GetDstColorSpace() const override { return dst_; }; + + void Transform(TriStim* colors, size_t num) const override { for (const auto& step : steps_) step->Transform(colors, num); } + bool CanGetShaderSource() const override; + std::string GetShaderSource() const override; + bool IsIdentity() const override { return steps_.empty(); } size_t NumberOfStepsForTesting() const override { return steps_.size(); } private: @@ -283,13 +290,23 @@ ScopedQcmsProfile GetQCMSProfileIfNecessary(const ColorSpace& color_space); std::list<std::unique_ptr<ColorTransformStep>> steps_; + gfx::ColorSpace src_; + gfx::ColorSpace dst_; }; +#define SRC(...) \ + do { \ + *result += std::string(" ") + base::StringPrintf(__VA_ARGS__) + \ + std::string("\n"); \ + } while (0) + class ColorTransformNull : public ColorTransformStep { public: ColorTransformNull* GetNull() override { return this; } bool IsNull() override { return true; } - void Transform(ColorTransform::TriStim* color, size_t num) override {} + void Transform(ColorTransform::TriStim* color, size_t num) const override {} + bool CanAppendShaderSource() override { return true; } + void AppendShaderSource(std::string* result) override {} }; class ColorTransformMatrix : public ColorTransformStep { @@ -311,11 +328,24 @@ return SkMatrixIsApproximatelyIdentity(matrix_.matrix()); } - void Transform(ColorTransform::TriStim* colors, size_t num) override { + void Transform(ColorTransform::TriStim* colors, size_t num) const override { for (size_t i = 0; i < num; i++) matrix_.TransformPoint(colors + i); } + bool CanAppendShaderSource() override { return true; } + + void AppendShaderSource(std::string* result) override { + const SkMatrix44& m = matrix_.matrix(); + SRC("color = mat3(%1.8e, %1.8e, %1.8e, %1.8e, %1.8e, %1.8e, %1.8e, %1.8e, " + "%1.8e) * color;", + m.get(0, 0), m.get(1, 0), m.get(2, 0), // column 1 + m.get(0, 1), m.get(1, 1), m.get(2, 1), // column 2 + m.get(0, 2), m.get(1, 2), m.get(2, 2)); // column 3 + SRC("color += vec3(%1.8e, %1.8e, %1.8e);", m.get(0, 3), m.get(1, 3), + m.get(2, 3)); + } + private: class Transform matrix_; }; @@ -331,7 +361,7 @@ } ColorTransformFromLinear* GetFromLinear() override { return this; } bool IsNull() override { return transfer_ == ColorSpace::TransferID::LINEAR; } - void Transform(ColorTransform::TriStim* colors, size_t num) override { + void Transform(ColorTransform::TriStim* colors, size_t num) const override { if (fn_valid_) { for (size_t i = 0; i < num; i++) { colors[i].set_x(EvalSkTransferFn(fn_, colors[i].x())); @@ -399,7 +429,7 @@ return c.x() * 0.2627f + c.y() * 0.6780f + c.z() * 0.0593f; } - ColorTransform::TriStim ClipToWhite(ColorTransform::TriStim& c) { + static ColorTransform::TriStim ClipToWhite(ColorTransform::TriStim& c) { float maximum = max(max(c.x(), c.y()), c.z()); if (maximum > 1.0f) { float l = Luma(c); @@ -412,7 +442,7 @@ return c; } - void Transform(ColorTransform::TriStim* colors, size_t num) override { + void Transform(ColorTransform::TriStim* colors, size_t num) const override { if (fn_valid_) { for (size_t i = 0; i < num; i++) { colors[i].set_x(EvalSkTransferFn(fn_, colors[i].x())); @@ -476,7 +506,7 @@ bool IsNull() override { return null_; } - void Transform(ColorTransform::TriStim* RYB, size_t num) override { + void Transform(ColorTransform::TriStim* RYB, size_t num) const override { for (size_t i = 0; i < num; i++) { float U, V; float B_Y = RYB[i].z() - RYB[i].y(); @@ -514,7 +544,7 @@ bool IsNull() override { return null_; } - void Transform(ColorTransform::TriStim* YUV, size_t num) override { + void Transform(ColorTransform::TriStim* YUV, size_t num) const override { if (null_) return; for (size_t i = 0; i < num; i++) { @@ -587,6 +617,11 @@ steps_.push_back( base::MakeUnique<ColorTransformMatrix>(Invert(GetTransferMatrix(from)))); + // If the target color space is not defined, just apply the adjust and + // tranfer matrices. + if (!to.IsValid()) + return; + SkColorSpaceTransferFn to_linear_fn; bool to_linear_fn_valid = from.GetTransferFunction(&to_linear_fn); steps_.push_back(base::MakeUnique<ColorTransformToLinear>( @@ -640,7 +675,7 @@ return true; return false; } - void Transform(ColorTransform::TriStim* colors, size_t num) override { + void Transform(ColorTransform::TriStim* colors, size_t num) const override { CHECK(sizeof(ColorTransform::TriStim) == sizeof(float[3])); // QCMS doesn't like numbers outside 0..1 for (size_t i = 0; i < num; i++) { @@ -688,37 +723,56 @@ return ScopedQcmsProfile(qcms_profile_create_rgb_with_gamma(w, xyz, 1.0f)); } -ColorTransformInternal::ColorTransformInternal(const ColorSpace& from, - const ColorSpace& to, - Intent intent) { +ColorTransformInternal::ColorTransformInternal(const ColorSpace& src, + const ColorSpace& dst, + Intent intent) + : src_(src), dst_(dst) { // If no source color space is specified, do no transformation. - // TODO(ccameron): We may want to assume sRGB at some point in the future. - if (!from.IsValid()) + // TODO(ccameron): We may want dst assume sRGB at some point in the future. + if (!src_.IsValid()) return; - ScopedQcmsProfile from_profile = GetQCMSProfileIfNecessary(from); - ScopedQcmsProfile to_profile = GetQCMSProfileIfNecessary(to); - bool has_from_profile = !!from_profile; - bool has_to_profile = !!to_profile; + ScopedQcmsProfile src_profile = GetQCMSProfileIfNecessary(src_); + ScopedQcmsProfile dst_profile = GetQCMSProfileIfNecessary(dst_); + bool has_src_profile = !!src_profile; + bool has_dst_profile = !!dst_profile; - if (from_profile) { + if (src_profile) { steps_.push_back(base::MakeUnique<QCMSColorTransform>( - std::move(from_profile), GetXYZD50Profile())); + std::move(src_profile), GetXYZD50Profile())); } AppendColorSpaceToColorSpaceTransform( - has_from_profile ? ColorSpace::CreateXYZD50() : from, - has_to_profile ? ColorSpace::CreateXYZD50() : to, intent); + has_src_profile ? ColorSpace::CreateXYZD50() : src_, + has_dst_profile ? ColorSpace::CreateXYZD50() : dst_, intent); - if (to_profile) { + if (dst_profile) { steps_.push_back(base::MakeUnique<QCMSColorTransform>( - GetXYZD50Profile(), std::move(to_profile))); + GetXYZD50Profile(), std::move(dst_profile))); } if (intent != Intent::TEST_NO_OPT) Simplify(); } +std::string ColorTransformInternal::GetShaderSource() const { + std::string result; + result += "vec3 DoColorConversion(vec3 color) {\n"; + for (const auto& step : steps_) + step->AppendShaderSource(&result); + result += " return color;\n"; + result += "}\n"; + return result; +} + +bool ColorTransformInternal::CanGetShaderSource() const { + for (const auto& step : steps_) { + if (!step->CanAppendShaderSource()) + return false; + } + return true; +} + ColorTransformInternal::~ColorTransformInternal() {} void ColorTransformInternal::Simplify() {
diff --git a/ui/gfx/color_transform.h b/ui/gfx/color_transform.h index 5368b7a..8120a7b 100644 --- a/ui/gfx/color_transform.h +++ b/ui/gfx/color_transform.h
@@ -22,9 +22,19 @@ ColorTransform(); virtual ~ColorTransform(); + virtual gfx::ColorSpace GetSrcColorSpace() const = 0; + virtual gfx::ColorSpace GetDstColorSpace() const = 0; // Perform transformation of colors, |colors| is both input and output. - virtual void Transform(TriStim* colors, size_t num) = 0; + virtual void Transform(TriStim* colors, size_t num) const = 0; + + // Return GLSL shader source that defines a function DoColorConversion that + // converts a vec3 according to this transform. + virtual bool CanGetShaderSource() const = 0; + virtual std::string GetShaderSource() const = 0; + + // Returns true if this transform is the identity. + virtual bool IsIdentity() const = 0; virtual size_t NumberOfStepsForTesting() const = 0;