New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 680536 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Use a layer to blink cursor in textfield

Project Member Reported by sadrul@chromium.org, Jan 12 2017

Issue description

views::Textfield (used in the omnibox, and other places) has to raster for each text blink. We can avoid this by using a 1-px wide solid-color layer for the cursor*, and toggling its visibility/opacity.

The tricky part here would be to make sure that the cursor-layer is parented to the correct layer.

* The cursor in textfield is always 1px wide.
 

Comment 1 by est...@chromium.org, Jan 12 2017

(technically, it's one dip)

The cursor is drawn in RenderText but I guess it should be easy enough to change since GetCursorBounds is already exposed. Not sure why the cursor layer should be difficult to parent correctly. Can we just add a child view that uses a layer, then get parenting logic for free?

Comment 2 by sky@chromium.org, Jan 12 2017

One thing to be careful about is that we don't end up with a bunch of z-ordering issues. Things like the find panel animate a view with a textfield. If the cursor is a layer we have to make sure it doesn't end up drawing on top of a bunch of other views.
Agreed with comment 2 but for the record I think the find-in-page should be safe from a z-ordering perspective.  The problem was seen when adding the ink drop and should already be fixed.

Comment 4 by sadrul@chromium.org, Jan 20 2017

Status: Started (was: Assigned)
Yep. I believe yiyix@ has a working prototype of this working.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0338037a696b84bb66efe37eee1cb16bdb113453

commit 0338037a696b84bb66efe37eee1cb16bdb113453
Author: yiyix <yiyix@chromium.org>
Date: Wed Jan 25 04:05:12 2017

Update SetPaintToLayer to accept LayerType

SetPaintToLayer is used to create layer for a specific view. It used to accept a
bool variable and always create a texture layer, which is very expensive because
texture layer needs to be re-raster every time. It can now accept |layer_type|
as input. Note that subsequent CLs will use this to create Views with
ui::LAYER_SOLID_COLOR layers.

BUG= 680536 

Review-Url: https://codereview.chromium.org/2639203007
Cr-Commit-Position: refs/heads/master@{#445940}

[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/frame/header_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/shelf/overflow_bubble_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/shelf/shelf_button.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/shelf/shelf_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/system/chromeos/network/network_state_list_detailed_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/system/status_area_widget_delegate.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/system/tray/throbber_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/system/tray/tray_background_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/system/tray/tray_details_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/system/tray/tray_details_view_unittest.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/system/tray/tray_item_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/system/tray/tray_popup_item_container.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/system/tray/tray_popup_utils.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/system/user/tray_user.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/system/web_notification/web_notification_tray.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/wallpaper/wallpaper_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/wm/overview/window_selector_item.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/common/wm/window_cycle_list.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/touch/touch_hud_debug.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/touch_hud/touch_hud_renderer.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ash/wm/window_mirror_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/chromeos/display/touch_calibrator/touch_calibrator_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/dropdown_bar_host.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/frame/contents_web_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/infobars/infobar_container_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/infobars/infobar_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/location_bar/bubble_icon_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/payments/payment_request_views_util.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/payments/view_stack.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/app_list/views/app_list_folder_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/app_list/views/app_list_main_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/app_list/views/app_list_main_view_unittest.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/app_list/views/app_list_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/app_list/views/apps_grid_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/app_list/views/folder_background_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/app_list/views/pulsing_block_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/app_list/views/top_icon_animation_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/arc/notification/arc_custom_notification_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/message_center/views/message_center_button_bar.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/message_center/views/message_center_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/message_center/views/notifier_settings_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/animation/ink_drop.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/animation/ink_drop_host_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/bubble/tray_bubble_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/controls/button/label_button.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/controls/button/md_text_button.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/controls/combobox/combobox.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/controls/focus_ring.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/controls/scroll_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/controls/scrollbar/cocoa_scroll_bar.mm
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/controls/scrollbar/overlay_scroll_bar.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/controls/slide_out_view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/view.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/view.h
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/view_unittest.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/view_unittest_aura.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/widget/native_widget_aura_unittest.cc
[modify] https://crrev.com/0338037a696b84bb66efe37eee1cb16bdb113453/ui/views/widget/window_reorderer_unittest.cc

Comment 6 Deleted

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/437537edc6fe22fd892be8fc83a5b303378f19b7

commit 437537edc6fe22fd892be8fc83a5b303378f19b7
Author: yiyix <yiyix@chromium.org>
Date: Tue Feb 14 02:08:50 2017

Fix crash in widget destruction

During the widget destruction, chrome crashes if this widget's |view|
visibility changes during |view|'s destruction.

During the destruction of the widget, the root view is reset to nullptr
first, then the destruction of its children views start. If |view|
updates its visibility, it needs to access the root view which is set
to nullptr earlier. This is where the crash happens.

TEST=AXViewTest.LayoutCalledInvalidateRootView

BUG= 680536 

Review-Url: https://codereview.chromium.org/2697533003
Cr-Commit-Position: refs/heads/master@{#450198}

[modify] https://crrev.com/437537edc6fe22fd892be8fc83a5b303378f19b7/ui/views/accessibility/ax_widget_obj_wrapper.cc
[modify] https://crrev.com/437537edc6fe22fd892be8fc83a5b303378f19b7/ui/views/accessibility/native_view_accessibility_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 17 2017

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49f6290b07b97c200fbf8737be29107ca99f923e

commit 49f6290b07b97c200fbf8737be29107ca99f923e
Author: yiyix <yiyix@chromium.org>
Date: Sat Feb 25 04:32:11 2017

Avoid to change layer type in InkDropHostView::AddInkDropLayer

Driving by the fact that the layer type of a view can be changed after
landing cl https://codereview.chromium.org/2639203007.

It's possible that classes that extends InkDropHostView (ex: Button)
created a LAYER_SOLID_COLOR S as its layer type, then by calling
InkDropHostView::AddInkDropLayer, S is destroyed and a new LAYER_TEXTURED
T is created by calling "SetPaintToLayer();". Add check such that no new
layer is created if the view has a layer already.

BUG= 680536 

Review-Url: https://codereview.chromium.org/2709283002
Cr-Commit-Position: refs/heads/master@{#453067}

[modify] https://crrev.com/49f6290b07b97c200fbf8737be29107ca99f923e/ui/views/animation/ink_drop_host_view.cc

Comment 10 by yiyix@chromium.org, Feb 27 2017

Status: Fixed (was: Started)
Status: Verified (was: Fixed)

Sign in to add a comment