Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2878403002: Support setting mouse cursor icon in Android N. (Closed)

Created:
7 months ago by jaebaek
Modified:
4 months ago
CC:
agrieve+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, jam, nona+watch_chromium.org, shuchen+watch_chromium.org, James Su, yusukes+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Support setting mouse cursor icon in Android N. Android N supports mouse input and setting mouse cursor icon (https://developer.android.com/reference/android/view/View.html#setPointerIcon(android.view.PointerIcon)). Current Chrome does not change the mouse cursor icon even when CSS file specifies the cursor type. This CL supports the mouse cursor setup (e.g., IBeam, Hand, custom cursor from image). BUG=584424 Review-Url: https://codereview.chromium.org/2878403002 Cr-Commit-Position: refs/heads/master@{#493995} Committed: https://chromium.googlesource.com/chromium/src/+/d1b0d570f9bc9bf80d187170fbeac0d4a1f6eb50

Patch Set 1 #

Patch Set 2 : [WIP] pointer icon in android #

Patch Set 3 : Support setting pointer icon in Android N #

Patch Set 4 : Support setting pointer icon in Android N #

Patch Set 5 : Support setting pointer icon in Android N #

Total comments: 34

Patch Set 6 : Support setting pointer icon in Android N #

Patch Set 7 : Support setting pointer icon in Android N #

Total comments: 4

Patch Set 8 : Support setting mouse cursor icon in Android N #

Total comments: 15

Patch Set 9 : Support setting mouse cursor icon in Android N #

Patch Set 10 : Support setting mouse cursor icon in Android N #

Patch Set 11 : Support setting mouse cursor icon in Android N #

Total comments: 4

Patch Set 12 : Support setting mouse cursor icon in Android N #

Total comments: 2

Patch Set 13 : Support setting mouse cursor icon in Android N #

Total comments: 16

Patch Set 14 : Support setting mouse cursor icon in Android N #

Patch Set 15 : Support setting mouse cursor icon in Android N (Test) #

Patch Set 16 : Support setting mouse cursor icon in Android N #

Patch Set 17 : Support setting mouse cursor icon in Android N #

Patch Set 18 : Support setting mouse cursor icon in Android N #

Total comments: 6

Patch Set 19 : Support setting mouse cursor icon in Android N #

Total comments: 10

Patch Set 20 : Support setting mouse cursor icon in Android N (Test) #

Patch Set 21 : Support setting mouse cursor icon in Android N #

Total comments: 6

Patch Set 22 : Support setting mouse cursor icon in Android N #

Total comments: 2

Patch Set 23 : nit #

Total comments: 2

Patch Set 24 : CoordinatorLayout for PointerIcon #

Patch Set 25 : Handle the case content view is null #

Patch Set 26 : Handle the case content view is null #

Patch Set 27 : Resize views to show pointer icon #

Patch Set 28 : Resize views to show pointer icon (rebase) #

Patch Set 29 : Support setting mouse cursor icon in Android N #

Total comments: 8

Patch Set 30 : Support setting mouse cursor icon in Android N #

Patch Set 31 : Support setting mouse cursor icon in Android N #

Total comments: 12

Patch Set 32 : Override onResolvePointerIcon #

Patch Set 33 : Override onResolvePointerIcon #

Total comments: 23

Patch Set 34 : Override onResolvePointerIcon #

Patch Set 35 : Support setting mouse cursor icon in Android N #

Patch Set 36 : rebase #

Patch Set 37 : Support setting mouse cursor icon in Android N #

Patch Set 38 : Apply patch 585579 #

Patch Set 39 : rebase #

Patch Set 40 : Apply patch 585579 #

Patch Set 41 : Support setting mouse cursor icon in Android N #

Patch Set 42 : Support setting mouse cursor icon in Android N #

Patch Set 43 : Support setting mouse cursor icon in Android N #

Patch Set 44 : Support setting mouse cursor icon in Android N #

Patch Set 45 : Support setting mouse cursor icon in Android N #

Total comments: 8

Patch Set 46 : Support setting mouse cursor icon in Android N #

Patch Set 47 : rebase #

Patch Set 48 : Support setting mouse cursor icon in Android N #

Unified diffs Side-by-side diffs Delta from patch set Stats (+421 lines, -11 lines) Patch
M chrome/android/java/res/layout/main.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 4 chunks +5 lines, -5 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 3 chunks +10 lines, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/coordinator/CoordinatorLayoutForPointer.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 1 chunk +45 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/widget/OverviewListLayout.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 3 chunks +9 lines, -3 lines 0 comments Download
M chrome/android/java_sources.gni View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/android/content_view_core.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 1 chunk +4 lines, -1 line 0 comments Download
M content/public/android/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 3 chunks +6 lines, -1 line 0 comments Download
A content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 1 chunk +96 lines, -0 lines 0 comments Download
M content/shell/android/java/src/org/chromium/content_shell/ShellViewAndroidDelegate.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 2 chunks +36 lines, -0 lines 0 comments Download
M content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellActivityTestRule.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +8 lines, -0 lines 0 comments Download
M content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellTestCommon.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 2 chunks +13 lines, -0 lines 0 comments Download
M third_party/WebKit/public/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 1 chunk +6 lines, -0 lines 0 comments Download
M third_party/WebKit/public/platform/WebCursorInfo.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 1 chunk +3 lines, -0 lines 0 comments Download
M ui/android/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 2 chunks +5 lines, -1 line 0 comments Download
M ui/android/DEPS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 1 chunk +1 line, -0 lines 0 comments Download
M ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 2 chunks +143 lines, -0 lines 0 comments Download
M ui/android/view_android.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 2 chunks +9 lines, -0 lines 0 comments Download
M ui/android/view_android.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 3 chunks +20 lines, -0 lines 0 comments Download

Messages

Total messages: 145 (74 generated)
jaebaek
PTAL
7 months ago (2017-05-17 03:26:05 UTC) #7
aelias_OOO_until_Jul13
https://codereview.chromium.org/2878403002/diff/80001/content/browser/renderer_host/render_widget_host_view_android.cc File content/browser/renderer_host/render_widget_host_view_android.cc (right): https://codereview.chromium.org/2878403002/diff/80001/content/browser/renderer_host/render_widget_host_view_android.cc#newcode710 content/browser/renderer_host/render_widget_host_view_android.cc:710: if (!content_view_core_) No longer needed. https://codereview.chromium.org/2878403002/diff/80001/content/browser/renderer_host/render_widget_host_view_android.cc#newcode786 content/browser/renderer_host/render_widget_host_view_android.cc:786: LOG(WARNING) << ...
7 months ago (2017-05-17 04:47:16 UTC) #8
Jinsuk Kim
Nice to see the icons working. https://codereview.chromium.org/2878403002/diff/80001/content/browser/renderer_host/render_widget_host_view_android.cc File content/browser/renderer_host/render_widget_host_view_android.cc (right): https://codereview.chromium.org/2878403002/diff/80001/content/browser/renderer_host/render_widget_host_view_android.cc#newcode710 content/browser/renderer_host/render_widget_host_view_android.cc:710: if (!content_view_core_) No ...
7 months ago (2017-05-17 04:49:51 UTC) #9
jaebaek
PTAL For device/power_save_blocker, device/power_save_blocker/power_save_blocker_android.cc includes view_android.h To enable the custom cursor type, I cannot help ...
6 months, 4 weeks ago (2017-05-18 01:44:29 UTC) #21
Jinsuk Kim
https://codereview.chromium.org/2878403002/diff/80001/device/power_save_blocker/BUILD.gn File device/power_save_blocker/BUILD.gn (right): https://codereview.chromium.org/2878403002/diff/80001/device/power_save_blocker/BUILD.gn#newcode60 device/power_save_blocker/BUILD.gn:60: "//ui/gfx", On 2017/05/18 01:44:28, jaebaek wrote: > On 2017/05/17 ...
6 months, 4 weeks ago (2017-05-18 02:19:51 UTC) #22
aelias_OOO_until_Jul13
https://codereview.chromium.org/2878403002/diff/120001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java File ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java (right): https://codereview.chromium.org/2878403002/diff/120001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java#newcode183 ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java:183: case WebCursorInfoType.TYPE_EAST_RESIZE: On 2017/05/18 at 02:19:51, Jinsuk Kim wrote: ...
6 months, 4 weeks ago (2017-05-18 18:57:34 UTC) #29
aelias_OOO_until_Jul13
https://codereview.chromium.org/2878403002/diff/140001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java File ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java (right): https://codereview.chromium.org/2878403002/diff/140001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java#newcode190 ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java:190: case WebCursorInfoType.TYPE_WEST_RESIZE: These are web-specced to do something as ...
6 months, 4 weeks ago (2017-05-18 19:59:11 UTC) #30
boliu
hmm... test? https://codereview.chromium.org/2878403002/diff/140001/content/browser/renderer_host/render_widget_host_view_android.cc File content/browser/renderer_host/render_widget_host_view_android.cc (right): https://codereview.chromium.org/2878403002/diff/140001/content/browser/renderer_host/render_widget_host_view_android.cc#newcode725 content/browser/renderer_host/render_widget_host_view_android.cc:725: view_.OnCursorChanged(cursor_info.type == WebCursorInfo::kTypeCustom, this parameter is redundant, ...
6 months, 4 weeks ago (2017-05-19 02:27:57 UTC) #31
jaebaek
PTAL https://codereview.chromium.org/2878403002/diff/120001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java File ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java (right): https://codereview.chromium.org/2878403002/diff/120001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java#newcode183 ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java:183: case WebCursorInfoType.TYPE_EAST_RESIZE: On 2017/05/18 02:19:51, Jinsuk Kim wrote: ...
6 months, 3 weeks ago (2017-05-22 12:08:35 UTC) #37
boliu
https://codereview.chromium.org/2878403002/diff/140001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java File ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java (right): https://codereview.chromium.org/2878403002/diff/140001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java#newcode209 ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java:209: containerView.getRootView().setPointerIcon( On 2017/05/22 12:08:35, jaebaek wrote: > On 2017/05/19 ...
6 months, 3 weeks ago (2017-05-22 16:33:09 UTC) #42
aelias_OOO_until_Jul13
https://codereview.chromium.org/2878403002/diff/140001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java File ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java (right): https://codereview.chromium.org/2878403002/diff/140001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java#newcode209 ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java:209: containerView.getRootView().setPointerIcon( On 2017/05/22 at 16:33:09, boliu wrote: > It's ...
6 months, 3 weeks ago (2017-05-22 22:31:14 UTC) #43
boliu
On 2017/05/22 22:31:14, aelias wrote: > https://codereview.chromium.org/2878403002/diff/140001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java > File ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java (right): > > https://codereview.chromium.org/2878403002/diff/140001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java#newcode209 > ...
6 months, 3 weeks ago (2017-05-22 22:48:26 UTC) #44
jaebaek
On 2017/05/22 22:48:26, boliu wrote: > On 2017/05/22 22:31:14, aelias wrote: > > > https://codereview.chromium.org/2878403002/diff/140001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java ...
6 months, 3 weeks ago (2017-05-23 09:08:29 UTC) #45
jaebaek
PTAL
6 months, 3 weeks ago (2017-05-23 09:08:47 UTC) #46
chromium-reviews
I was added to this mid-review; what do you want me to look at? On ...
6 months, 3 weeks ago (2017-05-23 14:52:01 UTC) #47
boliu
On 2017/05/23 09:08:29, jaebaek wrote: > On 2017/05/22 22:48:26, boliu wrote: > > On 2017/05/22 ...
6 months, 3 weeks ago (2017-05-23 15:05:59 UTC) #48
aelias_OOO_until_Jul13
https://codereview.chromium.org/2878403002/diff/200001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java File ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java (right): https://codereview.chromium.org/2878403002/diff/200001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java#newcode157 ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java:157: case WebCursorInfoType.TYPE_EAST_WEST_RESIZE: These are supposed to look different so ...
6 months, 3 weeks ago (2017-05-23 22:43:26 UTC) #49
jaebaek
boliu@, Ah! That is a good exception case. Thank you for letting me know it. ...
6 months, 3 weeks ago (2017-05-24 11:26:15 UTC) #50
Nico
What exactly in the DEPS file do you need me for? It's mostly deps on ...
6 months, 3 weeks ago (2017-05-24 14:39:22 UTC) #51
boliu
what's with new changes in ui/base? Does that need to be part of this CL? ...
6 months, 3 weeks ago (2017-05-24 18:24:08 UTC) #52
aelias_OOO_until_Jul13
https://codereview.chromium.org/2878403002/diff/240001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java File ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java (right): https://codereview.chromium.org/2878403002/diff/240001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java#newcode227 ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java:227: assert false : "These pointer icon types are not ...
6 months, 3 weeks ago (2017-05-24 19:03:56 UTC) #53
boliu
https://codereview.chromium.org/2878403002/diff/240001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java File ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java (right): https://codereview.chromium.org/2878403002/diff/240001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java#newcode227 ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java:227: assert false : "These pointer icon types are not ...
6 months, 3 weeks ago (2017-05-24 19:14:02 UTC) #54
jaebaek
The last commit is for testing mouse cursor. I tried DOMUtils.clickNode(), MotionEvent + OnGenericMotionListener(), and ...
6 months, 2 weeks ago (2017-05-29 12:40:04 UTC) #55
jaebaek
Added an instrumentation test. PTAL.
6 months, 2 weeks ago (2017-05-30 07:17:53 UTC) #56
boliu
https://codereview.chromium.org/2878403002/diff/340001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java File content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java (right): https://codereview.chromium.org/2878403002/diff/340001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java#newcode87 content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java:87: moveCursor(11.0f, 20.0f); how are you getting these coordinates? in ...
6 months, 2 weeks ago (2017-05-30 21:30:02 UTC) #57
aelias_OOO_until_Jul13
lgtm modulo Bo's test comments and a nit https://codereview.chromium.org/2878403002/diff/340001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java File ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java (right): https://codereview.chromium.org/2878403002/diff/340001/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java#newcode246 ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java:246: public ...
6 months, 2 weeks ago (2017-05-30 23:26:35 UTC) #58
jaebaek
boliu@ Could you please check CallbackHelper part? I added CallbackHelper rather than poll, but not ...
6 months, 2 weeks ago (2017-05-31 13:29:09 UTC) #59
boliu
https://codereview.chromium.org/2878403002/diff/360001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java File content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java (right): https://codereview.chromium.org/2878403002/diff/360001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java#newcode29 content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java:29: @MinAndroidSdkLevel(Build.VERSION_CODES.N) which part of this test requires N specifically? ...
6 months, 2 weeks ago (2017-05-31 16:30:04 UTC) #60
boliu
https://codereview.chromium.org/2878403002/diff/360001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java File content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java (right): https://codereview.chromium.org/2878403002/diff/360001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java#newcode31 content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerIconTest.java:31: public class ContentViewPointerIconTest extends ContentShellTestBase { also, most of ...
6 months, 2 weeks ago (2017-05-31 16:32:47 UTC) #61
jaebaek
boliu@, still unclear in terms of Android N issue .. If ShellViewAndroidDelegate overrides onCursorChange(), the ...
6 months, 2 weeks ago (2017-06-01 06:20:56 UTC) #62
boliu
On 2017/06/01 06:20:56, jaebaek wrote: > boliu@, still unclear in terms of Android N issue ...
6 months, 2 weeks ago (2017-06-01 16:14:51 UTC) #63
jaebaek
PTAL
6 months, 2 weeks ago (2017-06-02 06:17:18 UTC) #64
jaebaek
Ah .. I am sorry .. I rebased the code and it resulted in mixed ...
6 months, 2 weeks ago (2017-06-02 06:43:09 UTC) #65
boliu
https://codereview.chromium.org/2878403002/diff/400001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java File content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java (right): https://codereview.chromium.org/2878403002/diff/400001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java#newcode34 content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java:34: @MinAndroidSdkLevel(Build.VERSION_CODES.N) drop these two and make sure test runs ...
6 months, 1 week ago (2017-06-02 15:54:41 UTC) #66
jaebaek
It works fine. https://codereview.chromium.org/2878403002/diff/400001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java File content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java (right): https://codereview.chromium.org/2878403002/diff/400001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java#newcode34 content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java:34: @MinAndroidSdkLevel(Build.VERSION_CODES.N) On 2017/06/02 15:54:41, boliu wrote: ...
6 months, 1 week ago (2017-06-03 14:24:11 UTC) #67
boliu
lgtm % nit https://codereview.chromium.org/2878403002/diff/420001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java File content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java (right): https://codereview.chromium.org/2878403002/diff/420001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java#newcode85 content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java:85: Assert.assertEquals(mActivityTestRule.getOnCursorUpdateHelper().getPointerType(), type); nit: expected first, actual ...
6 months, 1 week ago (2017-06-07 02:38:09 UTC) #68
jaebaek
tedchoc@, Could you please review the CL of the following file? chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java chrome/android/java/src/org/chromium/chrome/browser/tab/TabViewAndroidDelegate.java
6 months, 1 week ago (2017-06-07 06:27:58 UTC) #70
Ted C
https://codereview.chromium.org/2878403002/diff/440001/chrome/android/java/src/org/chromium/chrome/browser/tab/TabViewAndroidDelegate.java File chrome/android/java/src/org/chromium/chrome/browser/tab/TabViewAndroidDelegate.java (right): https://codereview.chromium.org/2878403002/diff/440001/chrome/android/java/src/org/chromium/chrome/browser/tab/TabViewAndroidDelegate.java#newcode56 chrome/android/java/src/org/chromium/chrome/browser/tab/TabViewAndroidDelegate.java:56: getContainerView().getRootView().setPointerIcon(icon); since this mutates the root view, what happens ...
6 months, 1 week ago (2017-06-07 19:27:18 UTC) #71
jaebaek
I will dig into the container view issue more. https://codereview.chromium.org/2878403002/diff/420001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java File content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java (right): https://codereview.chromium.org/2878403002/diff/420001/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java#newcode85 content/public/android/javatests/src/org/chromium/content/browser/ContentViewPointerTypeTest.java:85: ...
6 months, 1 week ago (2017-06-08 01:00:11 UTC) #72
jaebaek
I tried to identify the exact View to correctly set the pointer icon. The ancestors ...
6 months, 1 week ago (2017-06-08 05:26:08 UTC) #73
Ted C
On 2017/06/08 01:00:11, jaebaek wrote: > I will dig into the container view issue more. ...
6 months, 1 week ago (2017-06-08 14:24:58 UTC) #74
jaebaek
2017. 6. 8. 오후 11:24에 <tedchoc@chromium.org>님이 작성: On 2017/06/08 01:00:11, jaebaek wrote: > I will ...
6 months, 1 week ago (2017-06-08 15:00:22 UTC) #75
jaebaek
When extending ContentView and CompositorViewHolder to override onResolvePointerIcon(), I checked both are not invoked. I ...
6 months ago (2017-06-12 08:11:45 UTC) #76
Ted C
+mdjones Looking at CoordinatorLayout, it doesn't actually override onResolvePointerIcon, so it is relying on the ...
6 months ago (2017-06-12 18:11:15 UTC) #78
jaebaek
To exactly understand what happens in onResolvePointerIcon() of the initial CoordinatorLayout, I used java reflection ...
6 months ago (2017-06-13 06:03:02 UTC) #79
aelias_OOO_until_Jul13
I'd suggest changing the size of the snackbar view to reflect its true visible size ...
6 months ago (2017-06-13 22:34:20 UTC) #80
jaebaek
I resized BottomContainer (snackbar) and FadingBackgroundView to make the pointer icon of CompositorViewHolder selected when ...
5 months ago (2017-07-13 01:35:36 UTC) #88
Jinsuk Kim
https://codereview.chromium.org/2878403002/diff/560001/chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarLayout.java File chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarLayout.java (right): https://codereview.chromium.org/2878403002/diff/560001/chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarLayout.java#newcode249 chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarLayout.java:249: if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { Version check is repeated ...
4 months, 4 weeks ago (2017-07-17 04:45:19 UTC) #89
jaebaek
tedchoc@, could you please review the code? I resized view components listed in main.xml to ...
4 months, 4 weeks ago (2017-07-18 07:48:21 UTC) #90
Ted C
mdjones@ will be the best reviewer for the sizing changes here. In general, I think ...
4 months, 3 weeks ago (2017-07-18 16:41:18 UTC) #91
jaebaek
tedchoc@, mdjones@, aelias@, As tedchoc pointed, it worries me side effects caused by this resizing. ...
4 months, 3 weeks ago (2017-07-19 01:18:53 UTC) #92
mdjones
https://codereview.chromium.org/2878403002/diff/600001/chrome/android/java/res/layout/main.xml File chrome/android/java/res/layout/main.xml (right): https://codereview.chromium.org/2878403002/diff/600001/chrome/android/java/res/layout/main.xml#newcode26 chrome/android/java/res/layout/main.xml:26: android:layout_height="wrap_content" /> On 2017/07/19 01:18:53, jaebaek wrote: > On ...
4 months, 3 weeks ago (2017-07-19 18:40:51 UTC) #93
jaebaek
Roll back to overriding CoordinatorLayout. tedchoc@, mdjones@, could you please review? https://codereview.chromium.org/2878403002/diff/600001/chrome/android/java/res/layout/main.xml File chrome/android/java/res/layout/main.xml (right): ...
4 months, 3 weeks ago (2017-07-20 14:16:30 UTC) #94
mdjones
Looks good, just a few comments: Regarding the partly transparent views: If a view group ...
4 months, 3 weeks ago (2017-07-20 21:49:06 UTC) #95
Ted C
overall this seems an ok, but sad (because of the framework issue) approach to the ...
4 months, 3 weeks ago (2017-07-21 00:15:26 UTC) #96
jaebaek
PTAL https://codereview.chromium.org/2878403002/diff/640001/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java File chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java (right): https://codereview.chromium.org/2878403002/diff/640001/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java#newcode190 chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java:190: ContentViewCore content = getActiveContent(); On 2017/07/21 00:15:25, Ted ...
4 months, 3 weeks ago (2017-07-21 10:03:05 UTC) #97
Ted C
https://codereview.chromium.org/2878403002/diff/640001/chrome/android/java/src/org/chromium/chrome/browser/coordinator/CoordinatorLayoutForPointer.java File chrome/android/java/src/org/chromium/chrome/browser/coordinator/CoordinatorLayoutForPointer.java (right): https://codereview.chromium.org/2878403002/diff/640001/chrome/android/java/src/org/chromium/chrome/browser/coordinator/CoordinatorLayoutForPointer.java#newcode36 chrome/android/java/src/org/chromium/chrome/browser/coordinator/CoordinatorLayoutForPointer.java:36: PointerIcon icon = getChildAt(i).onResolvePointerIcon(event, pointerIndex); this doesn't actually check ...
4 months, 3 weeks ago (2017-07-21 18:45:32 UTC) #98
mdjones
https://codereview.chromium.org/2878403002/diff/640001/chrome/android/java/src/org/chromium/chrome/browser/coordinator/CoordinatorLayoutForPointer.java File chrome/android/java/src/org/chromium/chrome/browser/coordinator/CoordinatorLayoutForPointer.java (right): https://codereview.chromium.org/2878403002/diff/640001/chrome/android/java/src/org/chromium/chrome/browser/coordinator/CoordinatorLayoutForPointer.java#newcode36 chrome/android/java/src/org/chromium/chrome/browser/coordinator/CoordinatorLayoutForPointer.java:36: PointerIcon icon = getChildAt(i).onResolvePointerIcon(event, pointerIndex); On 2017/07/21 18:45:31, Ted ...
4 months, 3 weeks ago (2017-07-21 20:42:28 UTC) #99
jaebaek
Two things are updated: 1. Adding bound check in onResolvePointerIcon() of CoordinatorLayoutForPointer. 2. Changing layout_height ...
4 months, 3 weeks ago (2017-07-24 07:34:11 UTC) #100
Ted C
On 2017/07/24 07:34:11, jaebaek wrote: > Two things are updated: > > 1. Adding bound ...
4 months, 2 weeks ago (2017-07-25 20:59:03 UTC) #101
Ted C
On 2017/07/24 07:34:11, jaebaek wrote: > Two things are updated: > > 1. Adding bound ...
4 months, 2 weeks ago (2017-07-25 21:38:00 UTC) #102
jaebaek
Adopted the patch https://chromium-review.googlesource.com/c/585579 that tedchoc@ suggested. PTAL
4 months, 1 week ago (2017-08-07 08:22:19 UTC) #119
jaebaek
Rebased (on the patch https://chromium-review.googlesource.com/c/585579) PTAL
4 months, 1 week ago (2017-08-08 05:42:52 UTC) #126
Ted C
lgtm w/ the one xml change and one continuing question https://codereview.chromium.org/2878403002/diff/880001/chrome/android/java/res/layout/main.xml File chrome/android/java/res/layout/main.xml (right): https://codereview.chromium.org/2878403002/diff/880001/chrome/android/java/res/layout/main.xml#newcode26 ...
4 months ago (2017-08-09 04:30:54 UTC) #127
jaebaek
https://codereview.chromium.org/2878403002/diff/880001/chrome/android/java/res/layout/main.xml File chrome/android/java/res/layout/main.xml (right): https://codereview.chromium.org/2878403002/diff/880001/chrome/android/java/res/layout/main.xml#newcode26 chrome/android/java/res/layout/main.xml:26: android:layout_height="wrap_content" /> On 2017/08/09 04:30:54, Ted C wrote: > ...
4 months ago (2017-08-09 06:38:18 UTC) #128
Ted C
https://codereview.chromium.org/2878403002/diff/880001/chrome/android/java/res/layout/main.xml File chrome/android/java/res/layout/main.xml (right): https://codereview.chromium.org/2878403002/diff/880001/chrome/android/java/res/layout/main.xml#newcode26 chrome/android/java/res/layout/main.xml:26: android:layout_height="wrap_content" /> On 2017/08/09 06:38:17, jaebaek wrote: > On ...
4 months ago (2017-08-09 16:30:55 UTC) #129
jaebaek
tedchoc@, updated it as discussed in offline. PTAL! https://codereview.chromium.org/2878403002/diff/880001/chrome/android/java/res/layout/main.xml File chrome/android/java/res/layout/main.xml (right): https://codereview.chromium.org/2878403002/diff/880001/chrome/android/java/res/layout/main.xml#newcode26 chrome/android/java/res/layout/main.xml:26: android:layout_height="wrap_content" ...
4 months ago (2017-08-11 00:47:35 UTC) #134
Ted C
lgtm
4 months ago (2017-08-11 23:56:51 UTC) #135
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2878403002/920001
4 months ago (2017-08-12 23:37:56 UTC) #138
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2878403002/940001
4 months ago (2017-08-13 08:10:26 UTC) #142
commit-bot: I haz the power
4 months ago (2017-08-13 10:20:29 UTC) #145
Message was sent while issue was closed.
Committed patchset #48 (id:940001) as
https://chromium.googlesource.com/chromium/src/+/d1b0d570f9bc9bf80d187170fbea...

Powered by Google App Engine
This is Rietveld 0eb02b776