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

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

Created:
5 years, 6 months ago by erikchen
Modified:
5 years, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months ago (2014-06-09 21:01:25 UTC) #6
Avi (use Gerrit)
LGTM
5 years, 6 months ago (2014-06-09 21:04:50 UTC) #7
erikchen
mark: Looking for OWNER review of base/mac/sdk_forward_declarations.h
5 years, 6 months ago (2014-06-11 21:08:57 UTC) #8
Mark Mentovai
LGTM in base.
5 years, 6 months ago (2014-06-11 21:19:17 UTC) #9
erikchen
The CQ bit was checked by erikchen@chromium.org
5 years, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months ago (2014-06-11 21:34:07 UTC) #14
erikchen
The CQ bit was checked by erikchen@chromium.org
5 years, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months ago (2014-06-11 22:38:50 UTC) #19
erikchen
The CQ bit was checked by erikchen@chromium.org
5 years, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months ago (2014-06-13 05:45:31 UTC) #24
erikchen
The CQ bit was checked by erikchen@chromium.org
5 years, 6 months 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, 6 months ago (2014-06-14 00:47:26 UTC) #26
commit-bot: I haz the power
5 years, 6 months 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