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

Issue 2884423003: Use scroll-boundary-behavior to control overscroll-refresh/glow on android. (Closed)

Created:
6 months ago by sunyunjia
Modified:
2 months, 2 weeks ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-api_chromium.org, blink-reviews-css, blink-reviews-dom_chromium.org, blink-reviews-frames_chromium.org, blink-reviews-style_chromium.org, cc-bugs_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, dglazkov+blink, dtapuska+chromiumwatch_chromium.org, eae+blinkwatch, jam, kenneth.christiansen, kinuko+watch, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, piman+watch_chromium.org, rwlbuis, sof
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Use scroll-boundary-behavior to control overscroll-refresh/glow on android. Overscroll navigation is a feature of Chrome. Users can pull verticallly to refresh the page on android, or swipe horizontally to browse history on desktop platform. Overscroll-glow is a visual effect indicating user that his scroll has passed the extent of the scroller. However, for some mobile web applications, navigation or glow does not make sense. This patch allows users to use a css attribute: scroll-boundary-behavior on the viewport defining element in order to control overscroll-navigation and overscroll-glow. On main thread, we set this property in OverscrollController, and on the compositor thread, we plumb this property to LayerTreeImpl. When overscroll happens, scroll-boundary-behavior is bundled in did_overscroll_params and is passed to OverscrollControllerAndroid::Overscrolled(). Thus, we make the decision of whether to pull-to-refresh and whether to glow on Overscrolled() as opposed to immediately after ScrollUpdateAck. Spec: https://wicg.github.io/scroll-boundary-behavior/ Design Doc: https://docs.google.com/a/google.com/document/d/1cqyeFamVmSu5EN5sBpQFB3Madm-41BtJEMDEbW1xdh4/edit?usp=sharing BUG=656801 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2884423003 Cr-Commit-Position: refs/heads/master@{#498816} Committed: https://chromium.googlesource.com/chromium/src/+/bbea8a99d2643b97d9dfe9d368cba6d129850a28

Patch Set 1 #

Patch Set 2 : Added missing files #

Total comments: 20

Patch Set 3 : Fixed the comments #

Total comments: 7

Patch Set 4 : update more tests #

Patch Set 5 : Update constructor #

Patch Set 6 : update more tests #

Patch Set 7 : Remove the test-expectations as they are still under flags #

Patch Set 8 : Update the test #

Total comments: 17

Patch Set 9 : rebase #

Patch Set 10 : Fixing comments #

Patch Set 11 : format #

Total comments: 12

Patch Set 12 : Fixing on android side #

Patch Set 13 : rebase #

Patch Set 14 : rebase #

Total comments: 2

Patch Set 15 : Added a WebFrameTest to distinguish frame vs iframe settings #

Patch Set 16 : Fixing the device-crash by re-org unique_ptrs #

Patch Set 17 : rebase #

Total comments: 4

Patch Set 18 : Update WebFrameTest #

Patch Set 19 : rebase #

Patch Set 20 : Update input_messages with the order in the enum. #

Total comments: 7

Patch Set 21 : update the tests and canpropagate #

Patch Set 22 : rebase and update comments #

Total comments: 4

Patch Set 23 : Use the state in overscroll_refresh instead. #

Patch Set 24 : Update the tests. #

Patch Set 25 : Adjust ScrollManager #

Patch Set 26 : Update ScrollManager to pass the test. #

Total comments: 6

Patch Set 27 : Fix nits and rebase #

Total comments: 21

Patch Set 28 : Update tests on android side. #

Patch Set 29 : rebase #

Total comments: 2

Patch Set 30 : Mark OverscrollRefresh() as protected. #

Total comments: 14

Patch Set 31 : fixing comments #

Patch Set 32 : Rebase. #

Total comments: 4

Patch Set 33 : Update the members in the test. #

Patch Set 34 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+687 lines, -157 lines) Patch
M cc/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 1 chunk +1 line, -0 lines 0 comments Download
M cc/input/input_handler.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +4 lines, -0 lines 0 comments Download
M cc/input/scroll_boundary_behavior.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 1 chunk +2 lines, -1 line 0 comments Download
M cc/input/scroll_state.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 1 chunk +4 lines, -0 lines 0 comments Download
M cc/input/scroll_state_data.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +7 lines, -0 lines 0 comments Download
M cc/input/scroll_state_data.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +1 line, -0 lines 0 comments Download
M cc/trees/layer_tree_host.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 3 chunks +4 lines, -0 lines 0 comments Download
M cc/trees/layer_tree_host.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 3 chunks +10 lines, -0 lines 0 comments Download
M cc/trees/layer_tree_host_impl.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 3 chunks +12 lines, -2 lines 0 comments Download
M cc/trees/layer_tree_impl.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 3 chunks +8 lines, -0 lines 0 comments Download
M cc/trees/layer_tree_impl.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 2 chunks +7 lines, -0 lines 0 comments Download
M content/browser/android/overscroll_controller_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 3 chunks +17 lines, -1 line 0 comments Download
M content/browser/android/overscroll_controller_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 3 chunks +71 lines, -18 lines 0 comments Download
A content/browser/android/overscroll_controller_android_unittest.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 1 chunk +183 lines, -0 lines 0 comments Download
M content/common/input_messages.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 2 chunks +11 lines, -0 lines 0 comments Download
M content/renderer/gpu/render_widget_compositor.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 1 chunk +3 lines, -0 lines 0 comments Download
M content/renderer/gpu/render_widget_compositor.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 2 chunks +6 lines, -0 lines 0 comments Download
M content/renderer/input/input_handler_wrapper.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +6 lines, -4 lines 0 comments Download
M content/renderer/input/input_handler_wrapper.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 1 chunk +3 lines, -1 line 0 comments Download
M content/renderer/input/render_widget_input_handler.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 2 chunks +6 lines, -5 lines 0 comments Download
M content/renderer/input/render_widget_input_handler.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 3 chunks +4 lines, -1 line 0 comments Download
M content/renderer/input/widget_input_handler_manager.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 1 chunk +6 lines, -4 lines 0 comments Download
M content/renderer/input/widget_input_handler_manager.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 1 chunk +3 lines, -1 line 0 comments Download
M content/renderer/render_view_impl.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 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/render_view_impl.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 1 chunk +3 lines, -2 lines 0 comments Download
M content/renderer/render_widget.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 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/render_widget.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 2 chunks +3 lines, -2 lines 0 comments Download
M content/renderer/render_widget_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +4 lines, -2 lines 0 comments Download
M content/test/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 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Document.cpp 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 4 chunks +21 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/exported/WebFrameTest.cpp 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 12 chunks +135 lines, -39 lines 0 comments Download
M third_party/WebKit/Source/core/exported/WebViewImpl.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 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/exported/WebViewImpl.cpp 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 1 chunk +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/loader/EmptyClients.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 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/page/ChromeClient.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 3 chunks +5 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/page/ChromeClientImpl.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 2 chunks +3 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/page/ChromeClientImpl.cpp 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 2 chunks +13 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/page/scrolling/OverscrollController.h View 1 2 3 3 chunks +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/page/scrolling/OverscrollController.cpp View 1 2 3 1 chunk +11 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/page/scrolling/RootScrollerTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 4 chunks +9 lines, -5 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 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/public/platform/WebLayerTreeView.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +5 lines, -0 lines 0 comments Download
M third_party/WebKit/public/web/WebViewClient.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 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/public/web/WebWidgetClient.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 1 chunk +2 lines, -1 line 0 comments Download
M ui/android/overscroll_glow.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 1 chunk +6 lines, -5 lines 0 comments Download
M ui/android/overscroll_glow.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 1 chunk +4 lines, -4 lines 0 comments Download
M ui/android/overscroll_refresh.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 3 chunks +16 lines, -6 lines 0 comments Download
M ui/android/overscroll_refresh.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 2 chunks +7 lines, -6 lines 0 comments Download
M ui/android/overscroll_refresh_unittest.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 8 chunks +10 lines, -9 lines 0 comments Download
M ui/events/blink/DEPS View 1 chunk +1 line, -0 lines 0 comments Download
M ui/events/blink/did_overscroll_params.h View 1 2 3 4 2 chunks +2 lines, -0 lines 0 comments Download
M ui/events/blink/input_handler_proxy.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +4 lines, -1 line 0 comments Download
M ui/events/blink/input_handler_proxy_client.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +2 lines, -1 line 0 comments Download
M ui/events/blink/input_handler_proxy_unittest.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 4 chunks +19 lines, -20 lines 0 comments Download

Messages

Total messages: 235 (183 generated)
sunyunjia
Hi Majid, Here is the new patch that uses scroll-boundary-behavior to control pull-to-refresh on android. ...
6 months ago (2017-05-17 01:02:18 UTC) #3
majidvp
I am not a CSS parser expert so I didn't review that part too closely. ...
6 months ago (2017-05-18 15:26:10 UTC) #4
sunyunjia
Hi Dave, Can you help look at this patch? We're uncertain about the css declarations, ...
6 months ago (2017-05-19 20:14:24 UTC) #8
dtapuska
https://codereview.chromium.org/2884423003/diff/40001/cc/ipc/cc_param_traits.h File cc/ipc/cc_param_traits.h (right): https://codereview.chromium.org/2884423003/diff/40001/cc/ipc/cc_param_traits.h#newcode11 cc/ipc/cc_param_traits.h:11: #include "cc/input/scroll_boundary_behavior.h" This is an odd include. Where is ...
6 months ago (2017-05-23 17:13:40 UTC) #9
sunyunjia
Hi Majid and Dave, I added some quite basic tests for overscroll_controller_android. Please take a ...
5 months, 1 week ago (2017-06-13 15:56:21 UTC) #38
majidvp
https://codereview.chromium.org/2884423003/diff/40001/third_party/WebKit/public/platform/WebScrollBoundaryBehavior.h File third_party/WebKit/public/platform/WebScrollBoundaryBehavior.h (right): https://codereview.chromium.org/2884423003/diff/40001/third_party/WebKit/public/platform/WebScrollBoundaryBehavior.h#newcode14 third_party/WebKit/public/platform/WebScrollBoundaryBehavior.h:14: struct WebScrollBoundaryBehavior : public ScrollBoundaryBehavior { On 2017/06/13 15:56:21, ...
4 months, 1 week ago (2017-07-12 21:04:22 UTC) #45
sunyunjia
PTAL, thanks! https://codereview.chromium.org/2884423003/diff/220001/content/browser/android/overscroll_controller_android.cc File content/browser/android/overscroll_controller_android.cc (right): https://codereview.chromium.org/2884423003/diff/220001/content/browser/android/overscroll_controller_android.cc#newcode121 content/browser/android/overscroll_controller_android.cc:121: refresh_effect_(CreateRefreshEffect(overscroll_refresh_handler)) { On 2017/07/12 21:04:20, majidvp wrote: ...
4 months ago (2017-07-19 20:44:12 UTC) #52
majidvp
https://codereview.chromium.org/2884423003/diff/220001/content/browser/android/overscroll_controller_android_unittest.cc File content/browser/android/overscroll_controller_android_unittest.cc (right): https://codereview.chromium.org/2884423003/diff/220001/content/browser/android/overscroll_controller_android_unittest.cc#newcode75 content/browser/android/overscroll_controller_android_unittest.cc:75: TEST_F(OverscrollControllerAndroidUnitTest, BasicNavigation) { On 2017/07/19 20:44:11, sunyunjia wrote: > ...
4 months ago (2017-07-20 13:26:55 UTC) #55
sunyunjia
PTAL, thanks! https://codereview.chromium.org/2884423003/diff/280001/content/browser/android/overscroll_controller_android.cc File content/browser/android/overscroll_controller_android.cc (right): https://codereview.chromium.org/2884423003/diff/280001/content/browser/android/overscroll_controller_android.cc#newcode117 content/browser/android/overscroll_controller_android.cc:117: OverscrollControllerAndroid( On 2017/07/20 13:26:55, majidvp wrote: > ...
4 months ago (2017-07-21 13:52:23 UTC) #68
majidvp
I think the CL description can be improved. Here are some comments: > Overscroll-navigation is ...
4 months ago (2017-07-21 15:24:12 UTC) #73
sunyunjia
PTAL, thanks! https://codereview.chromium.org/2884423003/diff/340001/third_party/WebKit/Source/core/dom/Document.cpp File third_party/WebKit/Source/core/dom/Document.cpp (right): https://codereview.chromium.org/2884423003/diff/340001/third_party/WebKit/Source/core/dom/Document.cpp#newcode1982 third_party/WebKit/Source/core/dom/Document.cpp:1982: GetPage()->GetOverscrollController().SetScrollBoundaryBehavior( On 2017/07/21 15:24:12, majidvp wrote: > ...
3 months, 3 weeks ago (2017-07-25 20:20:56 UTC) #96
majidvp
https://codereview.chromium.org/2884423003/diff/400001/third_party/WebKit/Source/web/tests/WebFrameTest.cpp File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right): https://codereview.chromium.org/2884423003/diff/400001/third_party/WebKit/Source/web/tests/WebFrameTest.cpp#newcode10222 third_party/WebKit/Source/web/tests/WebFrameTest.cpp:10222: TEST_P(WebFrameOverscrollTest, OnlyMainFrameScrollBoundaryBehaviorHasEffect) { Please also add a test to ...
3 months, 3 weeks ago (2017-07-25 20:38:29 UTC) #97
sunyunjia
PTAL, thanks! https://codereview.chromium.org/2884423003/diff/400001/third_party/WebKit/Source/web/tests/WebFrameTest.cpp File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right): https://codereview.chromium.org/2884423003/diff/400001/third_party/WebKit/Source/web/tests/WebFrameTest.cpp#newcode10222 third_party/WebKit/Source/web/tests/WebFrameTest.cpp:10222: TEST_P(WebFrameOverscrollTest, OnlyMainFrameScrollBoundaryBehaviorHasEffect) { On 2017/07/25 20:38:29, majidvp ...
3 months, 2 weeks ago (2017-08-03 14:54:34 UTC) #102
majidvp
https://codereview.chromium.org/2884423003/diff/480001/third_party/WebKit/Source/core/input/ScrollManager.cpp File third_party/WebKit/Source/core/input/ScrollManager.cpp (right): https://codereview.chromium.org/2884423003/diff/480001/third_party/WebKit/Source/core/input/ScrollManager.cpp#newcode141 third_party/WebKit/Source/core/input/ScrollManager.cpp:141: frame_->GetDocument()->ViewportDefiningElement(nullptr)) This feels odd to me. It is not ...
3 months, 1 week ago (2017-08-08 15:16:15 UTC) #114
sunyunjia
Hi Majid, PTAL, thanks! https://codereview.chromium.org/2884423003/diff/480001/third_party/WebKit/Source/core/input/ScrollManager.cpp File third_party/WebKit/Source/core/input/ScrollManager.cpp (right): https://codereview.chromium.org/2884423003/diff/480001/third_party/WebKit/Source/core/input/ScrollManager.cpp#newcode141 third_party/WebKit/Source/core/input/ScrollManager.cpp:141: frame_->GetDocument()->ViewportDefiningElement(nullptr)) On 2017/08/08 15:16:14, majidvp ...
3 months, 1 week ago (2017-08-10 16:17:27 UTC) #125
sunyunjia
Hi Alexandre, Could you please take a look at this patch? Here are some contexts-- ...
3 months, 1 week ago (2017-08-11 17:49:01 UTC) #133
aelias_OOO_until_Jul13
Looks good overall, thanks! https://codereview.chromium.org/2884423003/diff/520001/content/browser/android/overscroll_controller_android.cc File content/browser/android/overscroll_controller_android.cc (right): https://codereview.chromium.org/2884423003/diff/520001/content/browser/android/overscroll_controller_android.cc#newcode216 content/browser/android/overscroll_controller_android.cc:216: (!scroll_update_consumed_) && nit: please remove ...
3 months, 1 week ago (2017-08-11 21:50:32 UTC) #134
majidvp
https://codereview.chromium.org/2884423003/diff/480001/third_party/WebKit/Source/core/input/ScrollManager.cpp File third_party/WebKit/Source/core/input/ScrollManager.cpp (right): https://codereview.chromium.org/2884423003/diff/480001/third_party/WebKit/Source/core/input/ScrollManager.cpp#newcode141 third_party/WebKit/Source/core/input/ScrollManager.cpp:141: frame_->GetDocument()->ViewportDefiningElement(nullptr)) On 2017/08/10 16:17:27, sunyunjia wrote: > On 2017/08/08 ...
3 months ago (2017-08-15 14:30:12 UTC) #143
sunyunjia
I figured out CanScroll() has more complicated logic than the 2-step we figured out the ...
3 months ago (2017-08-18 18:44:06 UTC) #152
sunyunjia
Hi Ted, Could you please review this patch for the content/browser/android/ and ui/android/ ? Thanks!
2 months, 4 weeks ago (2017-08-22 13:49:05 UTC) #154
dtapuska
On 2017/08/22 13:49:05, sunyunjia wrote: > Hi Ted, > > Could you please review this ...
2 months, 4 weeks ago (2017-08-22 14:50:34 UTC) #155
dtapuska
https://codereview.chromium.org/2884423003/diff/600001/content/common/input_messages.h File content/common/input_messages.h (right): https://codereview.chromium.org/2884423003/diff/600001/content/common/input_messages.h#newcode88 content/common/input_messages.h:88: kScrollBoundaryBehaviorTypeContain) Can you add a max value in the ...
2 months, 4 weeks ago (2017-08-22 14:51:04 UTC) #156
majidvp
On 2017/08/18 18:44:06, sunyunjia wrote: > I figured out CanScroll() has more complicated logic than ...
2 months, 4 weeks ago (2017-08-22 15:33:50 UTC) #157
sunyunjia
https://codereview.chromium.org/2884423003/diff/600001/content/common/input_messages.h File content/common/input_messages.h (right): https://codereview.chromium.org/2884423003/diff/600001/content/common/input_messages.h#newcode88 content/common/input_messages.h:88: kScrollBoundaryBehaviorTypeContain) On 2017/08/22 14:51:04, dtapuska wrote: > Can you ...
2 months, 4 weeks ago (2017-08-22 23:55:37 UTC) #162
sunyunjia
Hi boliu, Could you please review this patch under content/browser/android/ and ui/android ? Thanks!
2 months, 4 weeks ago (2017-08-22 23:58:46 UTC) #164
boliu
https://codereview.chromium.org/2884423003/diff/620001/content/browser/android/overscroll_controller_android.cc File content/browser/android/overscroll_controller_android.cc (left): https://codereview.chromium.org/2884423003/diff/620001/content/browser/android/overscroll_controller_android.cc#oldcode107 content/browser/android/overscroll_controller_android.cc:107: DCHECK(compositor_); ditto don't remove production checks https://codereview.chromium.org/2884423003/diff/620001/content/browser/android/overscroll_controller_android.cc File content/browser/android/overscroll_controller_android.cc ...
2 months, 3 weeks ago (2017-08-23 19:17:03 UTC) #165
sunyunjia
Hi Bo, PTAL, thanks! https://codereview.chromium.org/2884423003/diff/620001/content/browser/android/overscroll_controller_android.cc File content/browser/android/overscroll_controller_android.cc (left): https://codereview.chromium.org/2884423003/diff/620001/content/browser/android/overscroll_controller_android.cc#oldcode107 content/browser/android/overscroll_controller_android.cc:107: DCHECK(compositor_); On 2017/08/23 19:17:02, boliu ...
2 months, 3 weeks ago (2017-08-23 23:47:06 UTC) #169
boliu
lgtm % protect constructor for refresh https://codereview.chromium.org/2884423003/diff/620001/ui/android/overscroll_refresh.h File ui/android/overscroll_refresh.h (right): https://codereview.chromium.org/2884423003/diff/620001/ui/android/overscroll_refresh.h#newcode33 ui/android/overscroll_refresh.h:33: ~OverscrollRefresh(); On 2017/08/23 ...
2 months, 3 weeks ago (2017-08-24 02:33:55 UTC) #176
sunyunjia
https://codereview.chromium.org/2884423003/diff/680001/ui/android/overscroll_refresh.h File ui/android/overscroll_refresh.h (right): https://codereview.chromium.org/2884423003/diff/680001/ui/android/overscroll_refresh.h#newcode34 ui/android/overscroll_refresh.h:34: OverscrollRefresh(); On 2017/08/24 02:33:54, boliu wrote: > protected? Done.
2 months, 3 weeks ago (2017-08-24 16:42:36 UTC) #182
sunyunjia
Hi chrishtr@, could you please review the code under cc/ and third_party/WebKit/public? avi@, could you ...
2 months, 3 weeks ago (2017-08-24 16:47:38 UTC) #184
Avi (ping after 24h)
lgtm https://codereview.chromium.org/2884423003/diff/700001/content/browser/android/overscroll_controller_android.h File content/browser/android/overscroll_controller_android.h (right): https://codereview.chromium.org/2884423003/diff/700001/content/browser/android/overscroll_controller_android.h#newcode84 content/browser/android/overscroll_controller_android.h:84: // OverscrollGlowClient implementation. extra blank line before this ...
2 months, 3 weeks ago (2017-08-24 21:12:43 UTC) #188
chrishtr
+meade for style review of change to Document.cpp. Other parts of Blink look fine, modulo ...
2 months, 3 weeks ago (2017-08-25 20:55:06 UTC) #190
sunyunjia
https://codereview.chromium.org/2884423003/diff/700001/third_party/WebKit/Source/core/input/ScrollManager.cpp File third_party/WebKit/Source/core/input/ScrollManager.cpp (right): https://codereview.chromium.org/2884423003/diff/700001/third_party/WebKit/Source/core/input/ScrollManager.cpp#newcode90 third_party/WebKit/Source/core/input/ScrollManager.cpp:90: if (!element.GetLayoutBox()->GetScrollableArea()) On 2017/08/25 20:55:06, chrishtr wrote: > Why ...
2 months, 3 weeks ago (2017-08-25 21:19:44 UTC) #191
chrishtr
On 2017/08/25 at 21:19:44, sunyunjia wrote: > https://codereview.chromium.org/2884423003/diff/700001/third_party/WebKit/Source/core/input/ScrollManager.cpp > File third_party/WebKit/Source/core/input/ScrollManager.cpp (right): > > https://codereview.chromium.org/2884423003/diff/700001/third_party/WebKit/Source/core/input/ScrollManager.cpp#newcode90 ...
2 months, 3 weeks ago (2017-08-25 21:24:36 UTC) #192
chrishtr
On 2017/08/25 at 21:24:36, chrishtr wrote: > On 2017/08/25 at 21:19:44, sunyunjia wrote: > > ...
2 months, 3 weeks ago (2017-08-25 21:24:55 UTC) #193
dcheng
I guess this works out for OOPIF, since it's main-frame only, but at some point, ...
2 months, 3 weeks ago (2017-08-25 23:12:18 UTC) #194
Ted C
lgtm w/ existing comments addressed https://codereview.chromium.org/2884423003/diff/700001/content/browser/android/overscroll_controller_android.h File content/browser/android/overscroll_controller_android.h (right): https://codereview.chromium.org/2884423003/diff/700001/content/browser/android/overscroll_controller_android.h#newcode78 content/browser/android/overscroll_controller_android.h:78: // This method should ...
2 months, 3 weeks ago (2017-08-28 18:58:48 UTC) #195
aelias_OOO_until_Jul13
lgtm w/ existing comments addressed
2 months, 3 weeks ago (2017-08-28 21:23:02 UTC) #196
sunyunjia
Hi chrishtr and dcheng, For the OOPIF case, I've filed a bug as crbug.com/760209 and ...
2 months, 3 weeks ago (2017-08-29 17:32:31 UTC) #199
sunyunjia
Hi kenrb@, Could you please do owner's review for content/common/input_messages.h ? Thanks!
2 months, 2 weeks ago (2017-08-30 19:47:01 UTC) #207
kenrb
ipc lgtm
2 months, 2 weeks ago (2017-08-30 21:02:18 UTC) #208
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/2884423003/740001
2 months, 2 weeks ago (2017-08-30 21:03:55 UTC) #211
dcheng
https://codereview.chromium.org/2884423003/diff/740001/content/browser/android/overscroll_controller_android_unittest.cc File content/browser/android/overscroll_controller_android_unittest.cc (right): https://codereview.chromium.org/2884423003/diff/740001/content/browser/android/overscroll_controller_android_unittest.cc#newcode74 content/browser/android/overscroll_controller_android_unittest.cc:74: base::MakeUnique<MockCompositor>(); Doesn't compositor_ptr have to outlive the scope of ...
2 months, 2 weeks ago (2017-08-30 23:27:27 UTC) #212
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/523408)
2 months, 2 weeks ago (2017-08-30 23:40:17 UTC) #214
sunyunjia
PTAL, thanks! https://codereview.chromium.org/2884423003/diff/740001/content/browser/android/overscroll_controller_android_unittest.cc File content/browser/android/overscroll_controller_android_unittest.cc (right): https://codereview.chromium.org/2884423003/diff/740001/content/browser/android/overscroll_controller_android_unittest.cc#newcode74 content/browser/android/overscroll_controller_android_unittest.cc:74: base::MakeUnique<MockCompositor>(); On 2017/08/30 23:27:27, dcheng wrote: > ...
2 months, 2 weeks ago (2017-08-31 00:07:54 UTC) #215
dcheng
lgtm
2 months, 2 weeks ago (2017-08-31 00:58:55 UTC) #218
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/2884423003/760001
2 months, 2 weeks ago (2017-08-31 02:51:42 UTC) #223
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/530162)
2 months, 2 weeks ago (2017-08-31 03:02:25 UTC) #225
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/2884423003/780001
2 months, 2 weeks ago (2017-08-31 03:20:44 UTC) #228
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/375233)
2 months, 2 weeks ago (2017-08-31 05:35:29 UTC) #230
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/2884423003/780001
2 months, 2 weeks ago (2017-08-31 10:22:59 UTC) #232
commit-bot: I haz the power
2 months, 2 weeks ago (2017-08-31 11:19:24 UTC) #235
Message was sent while issue was closed.
Committed patchset #34 (id:780001) as
https://chromium.googlesource.com/chromium/src/+/bbea8a99d2643b97d9dfe9d368cb...

Powered by Google App Engine
This is Rietveld efc10ee0f