Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
314393003:
mac: Add overscroll animator slider (Closed)
|
Created:
6 years, 6 months ago by erikchen Modified:
6 years, 6 months ago CC:
chromium-reviews, darin-cc_chromium.org, jam Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
Descriptionmac: 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. #
Messages
Total messages: 27 (0 generated)
avi: PTAL https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... 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... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:210: [[NSAnimationContext currentContext] setCompletionHandler:^{ This API is only available in 10.7+, but the relevant gesture events are also only available in 10.7+. I assume that this will cause compilation problems on our 10.6 machines. How would you like me to handle this problem? https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:244: [NSAnimationContext beginGrouping]; Everything but the block can be shared with -completeOverscroll: but I'm not sure whether passing a block would freak out compilers on 10.6. Let me know if you want to see a refactor here.
https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... File content/browser/web_contents/web_contents_view_overscroll_animator_mac.h (right): https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_mac.h:18: - (void)beginOverscrollLeft:(BOOL)left; I would much rather see a two-value enum. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_mac.h:26: - (void)supplyNavigationSnapshot:(NSImage*)image; I don't understand how this goes. Is the WebContentsView supposed to poll with -needsNavigationSnapshot? This strikes me as a callback situation, where the animator somehow asks, "hey, gimme a snapshot", rather than the WCV knowing that it needs to ask. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_mac.h:31: // implementor must guarantee that the frame of the RenderWidetHost's NativeView s/RenderWidetHost/RenderWidgetHost/ https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_mac.h:41: // |progress| >= 1, then the overscroll is considered completed. These usually are 0-1 ranged. Why 0-2? https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... File content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h (right): https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h:11: #include "content/browser/web_contents/web_contents_view_overscroll_animator_mac.h" C headers, then C++ headers, then Chrome headers. <Cocoa/Cocoa.h> goes in its own section before all the others. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h:20: // time, it's frame in screen coordinates is the same as SliderView's frame in s/it's/its/ https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h:35: // exactly covering middleView_. I must admit that I don't get these two comments. During an overscroll animation to the left, why would things be moving to the right, and vice-versa? https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h:40: BOOL slidingLeft_; Same comment about an enum for here. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... 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... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:9: #import <QuartzCore/QuartzCore.h> This import gets its own section. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:18: // progress_ = 1 to progress = 2.0; either progress_ or progress. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:54: // http://crbug.com/264207) unbalanced parenthesis. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:67: namespace content { put this in an anon namespace https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:195: DCHECK_GT(progress, kMinProgress - 0.00001); Why not DCHECK_LE and DCHECK_GE? https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:210: [[NSAnimationContext currentContext] setCompletionHandler:^{ On 2014/06/06 22:44:56, erikchen1 wrote: > This API is only available in 10.7+, but the relevant gesture events are also > only available in 10.7+. I assume that this will cause compilation problems on > our 10.6 machines. How would you like me to handle this problem? If you're guaranteed that you never will come through here on 10.6, add this API to sdk_forward_declarations.h, and then you can use it here. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:244: [NSAnimationContext beginGrouping]; On 2014/06/06 22:44:56, erikchen1 wrote: > Everything but the block can be shared with -completeOverscroll: but I'm not > sure whether passing a block would freak out compilers on 10.6. Let me know if > you want to see a refactor here. Yes, there's duplication, but I don't see a ton of it. I wouldn't worry.
avi: PTAL That was an incredibly fast review! I don't expect it from my reviewers, but I do appreciate it when it happens. :) https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... File content/browser/web_contents/web_contents_view_overscroll_animator_mac.h (right): https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_mac.h:18: - (void)beginOverscrollLeft:(BOOL)left; On 2014/06/06 23:09:44, Avi wrote: > I would much rather see a two-value enum. Done. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_mac.h:26: - (void)supplyNavigationSnapshot:(NSImage*)image; On 2014/06/06 23:09:44, Avi wrote: > I don't understand how this goes. Is the WebContentsView supposed to poll with > -needsNavigationSnapshot? This strikes me as a callback situation, where the > animator somehow asks, "hey, gimme a snapshot", rather than the WCV knowing that > it needs to ask. I've reworked the methods in question to clarify the situation. Starting an overscroll now requires a direction and an NSImage. A single method -needsNavigationSnapshot determines whether NSImage should be non-nil. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_mac.h:31: // implementor must guarantee that the frame of the RenderWidetHost's NativeView On 2014/06/06 23:09:44, Avi wrote: > s/RenderWidetHost/RenderWidgetHost/ Done. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_mac.h:41: // |progress| >= 1, then the overscroll is considered completed. On 2014/06/06 23:09:44, Avi wrote: > These usually are 0-1 ranged. Why 0-2? I could keep progress from 0-1, and compare against "0.5", but I've got another implementation of WebContentsOverscrollAnimator waiting in the wings which uses the progress value differently. (It basically caps progress at 1, on a scale from 0-2). In the context of both implementations of WebContentsOverscrollAnimator, 0-2 made more sense. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... File content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h (right): https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h:11: #include "content/browser/web_contents/web_contents_view_overscroll_animator_mac.h" On 2014/06/06 23:09:44, Avi wrote: > C headers, then C++ headers, then Chrome headers. <Cocoa/Cocoa.h> goes in its > own section before all the others. I've moved <Cocoa/Cocoa.h> to the top, and kept the order of the rest the same, as per your instructions. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h:20: // time, it's frame in screen coordinates is the same as SliderView's frame in On 2014/06/06 23:09:44, Avi wrote: > s/it's/its/ Done. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h:35: // exactly covering middleView_. On 2014/06/06 23:09:44, Avi wrote: > I must admit that I don't get these two comments. During an overscroll animation > to the left, why would things be moving to the right, and vice-versa? After the rename to use directions instead of left/right, the comments make a lot more sense. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h:40: BOOL slidingLeft_; On 2014/06/06 23:09:44, Avi wrote: > Same comment about an enum for here. Ditto. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... 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... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:9: #import <QuartzCore/QuartzCore.h> On 2014/06/06 23:09:45, Avi wrote: > This import gets its own section. Done. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:18: // progress_ = 1 to progress = 2.0; On 2014/06/06 23:09:45, Avi wrote: > either progress_ or progress. I switched to "progress" for both. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:54: // http://crbug.com/264207) On 2014/06/06 23:09:45, Avi wrote: > unbalanced parenthesis. removed. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:67: namespace content { On 2014/06/06 23:09:45, Avi wrote: > put this in an anon namespace that's doesn't work, as this class has a forward declaration in the header file, and anonymous namespaces in header files are not a good idea. I've changed the namespace to not pollute the content:: namespace https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:195: DCHECK_GT(progress, kMinProgress - 0.00001); On 2014/06/06 23:09:45, Avi wrote: > Why not DCHECK_LE and DCHECK_GE? Equality comparisons aren't particularly useful for floating point arithmetic. I've changed the checks to LE and GE, but I've kept the small delta. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:210: [[NSAnimationContext currentContext] setCompletionHandler:^{ On 2014/06/06 23:09:45, Avi wrote: > On 2014/06/06 22:44:56, erikchen1 wrote: > > This API is only available in 10.7+, but the relevant gesture events are also > > only available in 10.7+. I assume that this will cause compilation problems on > > our 10.6 machines. How would you like me to handle this problem? > > If you're guaranteed that you never will come through here on 10.6, add this API > to sdk_forward_declarations.h, and then you can use it here. Done.
Not a full review, but a comment about something that came immediately to mind. I'll look at the full thing later. https://codereview.chromium.org/314393003/diff/1/content/browser/web_contents... 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... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:195: DCHECK_GT(progress, kMinProgress - 0.00001); On 2014/06/07 00:34:04, erikchen1 wrote: > Equality comparisons aren't particularly useful for floating point arithmetic. They are. Your values, kMax|MinProgress are 0 and 2. They are exactly representable in a float. Floating point arithmetic is tricky, but not that tricky. > I've changed the checks to LE and GE, but I've kept the small delta. I'm not convinced that's necessary.
Thank you for the description re-writes. It makes perfect sense now. https://codereview.chromium.org/314393003/diff/40001/base/mac/sdk_forward_dec... File base/mac/sdk_forward_declarations.h (right): https://codereview.chromium.org/314393003/diff/40001/base/mac/sdk_forward_dec... base/mac/sdk_forward_declarations.h:82: - (void)setCompletionHandler:(void (^)(void))completionHandler; You can put this in as a property: @property(copy) void(^completionHandler)(void) https://codereview.chromium.org/314393003/diff/40001/content/browser/web_cont... File content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm (right): https://codereview.chromium.org/314393003/diff/40001/content/browser/web_cont... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:194: DCHECK_GE(progress, kMinProgress - 0.00001); Why do you insist on that .00001? Please remove it.
https://codereview.chromium.org/314393003/diff/40001/base/mac/sdk_forward_dec... File base/mac/sdk_forward_declarations.h (right): https://codereview.chromium.org/314393003/diff/40001/base/mac/sdk_forward_dec... base/mac/sdk_forward_declarations.h:82: - (void)setCompletionHandler:(void (^)(void))completionHandler; On 2014/06/09 15:29:00, Avi wrote: > You can put this in as a property: > > @property(copy) void(^completionHandler)(void) Done. https://codereview.chromium.org/314393003/diff/40001/content/browser/web_cont... File content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm (right): https://codereview.chromium.org/314393003/diff/40001/content/browser/web_cont... content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm:194: DCHECK_GE(progress, kMinProgress - 0.00001); On 2014/06/09 15:29:00, Avi wrote: > Why do you insist on that .00001? Please remove it. I haven't had a chance to update this CL since your comment on Friday. I've removed the 0.00001.
LGTM
mark: Looking for OWNER review of base/mac/sdk_forward_declarations.h
LGTM in base.
The CQ bit was checked by erikchen@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erikchen@chromium.org/314393003/60001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_aosp on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/8...) android_chromium_gn_compile_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...) android_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...) android_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...) chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...) ios_dbg_simulator on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...) ios_rel_device on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...) ios_rel_device_ninja on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...) linux_chromium_chromeos_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...) linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...) linux_chromium_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...) linux_chromium_gn_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...) linux_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...) mac_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...) mac_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...) win_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...) win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...) win_chromium_x64_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...) linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15658) win_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18793)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15665) win_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18799)
The CQ bit was checked by erikchen@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erikchen@chromium.org/314393003/60001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_aosp on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/8...) android_chromium_gn_compile_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...) android_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...) android_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...) chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...) ios_dbg_simulator on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...) ios_rel_device on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...) ios_rel_device_ninja on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...) linux_chromium_chromeos_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...) linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...) linux_chromium_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...) linux_chromium_gn_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...) linux_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...) mac_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...) mac_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...) win_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...) win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...) win_chromium_x64_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...) linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15681) win_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18815)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15726) win_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18858)
The CQ bit was checked by erikchen@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erikchen@chromium.org/314393003/60001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_aosp on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/8...) android_chromium_gn_compile_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...) android_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...) android_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...) chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...) ios_dbg_simulator on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...) ios_rel_device on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...) ios_rel_device_ninja on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...) linux_chromium_chromeos_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...) linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...) linux_chromium_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...) linux_chromium_gn_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...) linux_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...) mac_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...) mac_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...) linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/16442)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios_dbg_simulator on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...) ios_rel_device on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...) ios_rel_device_ninja on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...) mac_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...) mac_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/16456)
The CQ bit was checked by erikchen@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/erikchen@chromium.org/314393003/80001
Message was sent while issue was closed.
Change committed as 277196 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
