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

Issue 314393003: mac: Add overscroll animator slider (Closed)

Created:
5 years, 1 month ago by erikchen
Modified:
5 years, 1 month ago
CC:
chromium-reviews, darin-cc_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

mac: Add overscroll animator slider. An overscroll animator performs animations related to overscrolling for the WebContentsView. I added a protocol for overscroll animators on mac, and wrote a single implementation OverscrollAnimatorSliderView. This implementation performs an overscroll animation by sliding the current RenderWidgetHostView off the screen to the right (for backwards navigation), or by sliding an image of the next page onto the screen from the right (for forwards navigation). Neither the implementation nor the protocol are hooked into WebContentsView yet. BUG=381466 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277196

Patch Set 1 #

Total comments: 32

Patch Set 2 : Respond to comments from avi. #

Total comments: 4

Patch Set 3 : Respond to comments from avi. #

Patch Set 4 : Rebase against top of tree. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+380 lines, -0 lines) Patch
M base/mac/sdk_forward_declarations.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
A content/browser/web_contents/web_contents_view_overscroll_animator_mac.h View 1 1 chunk +61 lines, -0 lines 0 comments Download
A content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h View 1 1 chunk +59 lines, -0 lines 0 comments Download
A content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm View 1 2 1 chunk +259 lines, -0 lines 0 comments Download

Messages

Total messages: 27 (0 generated)
erikchen
avi: PTAL https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm File content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm (right): https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm#newcode210 content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:210: [[NSAnimationContext currentContext] setCompletionHandler:^{ This API is only ...
5 years, 1 month ago (2014-06-06 22:44:57 UTC) #1
Avi (use Gerrit)
https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents/web_contents_view_overscroll_animator_mac.h File content/browser/web_contents/web_contents_view_overscroll_animator_mac.h (right): https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents/web_contents_view_overscroll_animator_mac.h#newcode18 content/browser/web_contents/web_contents_view_overscroll_animator_mac.h:18: - (void)beginOverscrollLeft:(BOOL)left; I would much rather see a two-value ...
5 years, 1 month ago (2014-06-06 23:09:45 UTC) #2
erikchen
avi: PTAL That was an incredibly fast review! I don't expect it from my reviewers, ...
5 years, 1 month ago (2014-06-07 00:34:04 UTC) #3
Avi (use Gerrit)
Not a full review, but a comment about something that came immediately to mind. I'll ...
5 years, 1 month ago (2014-06-07 02:57:32 UTC) #4
Avi (use Gerrit)
Thank you for the description re-writes. It makes perfect sense now. https://codereview.chromium.org/314393003/diff/40001/base/mac/sdk_forward_declarations.h File base/mac/sdk_forward_declarations.h (right): ...
5 years, 1 month ago (2014-06-09 15:29:00 UTC) #5
erikchen
https://codereview.chromium.org/314393003/diff/40001/base/mac/sdk_forward_declarations.h File base/mac/sdk_forward_declarations.h (right): https://codereview.chromium.org/314393003/diff/40001/base/mac/sdk_forward_declarations.h#newcode82 base/mac/sdk_forward_declarations.h:82: - (void)setCompletionHandler:(void (^)(void))completionHandler; On 2014/06/09 15:29:00, Avi wrote: > ...
5 years, 1 month ago (2014-06-09 21:01:25 UTC) #6
Avi (use Gerrit)
LGTM
5 years, 1 month ago (2014-06-09 21:04:50 UTC) #7
erikchen
mark: Looking for OWNER review of base/mac/sdk_forward_declarations.h
5 years, 1 month ago (2014-06-11 21:08:57 UTC) #8
Mark Mentovai
LGTM in base.
5 years, 1 month ago (2014-06-11 21:19:17 UTC) #9
erikchen
The CQ bit was checked by erikchen@chromium.org
5 years, 1 month ago (2014-06-11 21:19:47 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erikchen@chromium.org/314393003/60001
5 years, 1 month ago (2014-06-11 21:22:23 UTC) #11
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_aosp on tryserver.chromium ...
5 years, 1 month ago (2014-06-11 21:31:05 UTC) #12
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
5 years, 1 month ago (2014-06-11 21:34:06 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/26629) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15665) win_gpu ...
5 years, 1 month ago (2014-06-11 21:34:07 UTC) #14
erikchen
The CQ bit was checked by erikchen@chromium.org
5 years, 1 month ago (2014-06-11 21:38:29 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erikchen@chromium.org/314393003/60001
5 years, 1 month ago (2014-06-11 21:41:30 UTC) #16
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_aosp on tryserver.chromium ...
5 years, 1 month ago (2014-06-11 22:36:55 UTC) #17
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
5 years, 1 month ago (2014-06-11 22:38:49 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/26667) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15726) win_gpu ...
5 years, 1 month ago (2014-06-11 22:38:50 UTC) #19
erikchen
The CQ bit was checked by erikchen@chromium.org
5 years, 1 month ago (2014-06-12 22:57:22 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erikchen@chromium.org/314393003/60001
5 years, 1 month ago (2014-06-12 23:00:43 UTC) #21
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_aosp on tryserver.chromium ...
5 years, 1 month ago (2014-06-13 05:43:32 UTC) #22
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
5 years, 1 month ago (2014-06-13 05:45:30 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: ios_dbg_simulator on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/builds/161011) ios_rel_device on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds/150391) ios_rel_device_ninja ...
5 years, 1 month ago (2014-06-13 05:45:31 UTC) #24
erikchen
The CQ bit was checked by erikchen@chromium.org
5 years, 1 month ago (2014-06-14 00:46:09 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erikchen@chromium.org/314393003/80001
5 years, 1 month ago (2014-06-14 00:47:26 UTC) #26
commit-bot: I haz the power
5 years, 1 month ago (2014-06-14 08:08:06 UTC) #27
Message was sent while issue was closed.
Change committed as 277196

Powered by Google App Engine
This is Rietveld 408576698