Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
300113009:
Fix BaseBubbleController close issue. (Closed)
|
Created:
6 years, 6 months ago by Xuefei Ren Modified:
6 years, 6 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
DescriptionFix BaseBubbleController close issue.
A bubble should disappear when we right mouse click on a blank
space. Add event monitor for NSRightMouseDownMask, and change
not to use performSelector, for sometimes when rightmouse down
happens, a menu comes out. This menu might be a Modal Dialogue Box.
So we can't perform the windowDidResignKey until the menu is closed.
BUG=378186
TEST="--gtest_filter=BaseBubbleControllerTest.*"
R=rsesek@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276875
Patch Set 1 #
Total comments: 3
Patch Set 2 : unit_test #
Total comments: 15
Patch Set 3 : style fixing #
Total comments: 4
Patch Set 4 : style fixing #
Messages
Total messages: 38 (0 generated)
It's still possible to right-click the bubble itself and do things like "inspect element" with this, right? https://codereview.chromium.org/300113009/diff/1/chrome/browser/ui/cocoa/base... File chrome/browser/ui/cocoa/base_bubble_controller.mm (right): https://codereview.chromium.org/300113009/diff/1/chrome/browser/ui/cocoa/base... chrome/browser/ui/cocoa/base_bubble_controller.mm:249: // it may pop up a menu. windowDidResignKey will not exec untill typo "until" https://codereview.chromium.org/300113009/diff/1/chrome/browser/ui/cocoa/base... chrome/browser/ui/cocoa/base_bubble_controller.mm:252: [self windowDidResignKey:note]; nit: this should be indented only 2 relative to the if
Oh, and can you add a short "TEST=" section that describes how to manually test the effect of this change? (Examples: https://codereview.chromium.org/218613009 https://codereview.chromium.org/227043012) Thanks for the patch!
ok, I'll do it ASAP. On Wed, May 28, 2014 at 4:52 PM, <thakis@chromium.org> wrote: > Oh, and can you add a short "TEST=" section that describes how to manually > test > the effect of this change? (Examples: https://codereview.chromium. > org/218613009 > https://codereview.chromium.org/227043012) > > Thanks for the patch! > > https://codereview.chromium.org/300113009/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2014/05/28 08:50:15, Nico (traveling) wrote: > It's still possible to right-click the bubble itself and do things like "inspect > element" with this, right? > > https://codereview.chromium.org/300113009/diff/1/chrome/browser/ui/cocoa/base... > File chrome/browser/ui/cocoa/base_bubble_controller.mm (right): > > https://codereview.chromium.org/300113009/diff/1/chrome/browser/ui/cocoa/base... > chrome/browser/ui/cocoa/base_bubble_controller.mm:249: // it may pop up a menu. > windowDidResignKey will not exec untill > typo "until" > > https://codereview.chromium.org/300113009/diff/1/chrome/browser/ui/cocoa/base... > chrome/browser/ui/cocoa/base_bubble_controller.mm:252: [self > windowDidResignKey:note]; > nit: this should be indented only 2 relative to the if if (event.window != window) It won't break right-click itself situation.
https://codereview.chromium.org/300113009/diff/1/chrome/browser/ui/cocoa/base... File chrome/browser/ui/cocoa/base_bubble_controller.mm (right): https://codereview.chromium.org/300113009/diff/1/chrome/browser/ui/cocoa/base... chrome/browser/ui/cocoa/base_bubble_controller.mm:249: // it may pop up a menu. windowDidResignKey will not exec untill nit: add a : after windowDidResignKey (it's part of the name)
Hi there, I was trying to write an unit test today. But I got a problem. Here it is. As you known, my patch is to solve the contextual menu problem. Before my patch, when we right-click(maybe left-click) on an object which can pop up a contextual menu, the bubble won't disappear because we use performSelector(windowDidResignKey:). windowDidResignKey: will not got a chance to run until the contextual menu is closed. When I tried to make a test, I need to test some values with EXPECT_TRUE/FASLE. The problem is, after a contextual menu pops up, my code will not run. It only can run after contextual menu is closed. So this is a Modal Dialogue Box problem. Now I don't know how to use a UNIT TEST to prove my patch. But it is easy to prove by manually operations which is not so strict. Any ideas? On Thu, May 29, 2014 at 3:47 AM, <rsesek@chromium.org> wrote: > > https://codereview.chromium.org/300113009/diff/1/chrome/ > browser/ui/cocoa/base_bubble_controller.mm > File chrome/browser/ui/cocoa/base_bubble_controller.mm (right): > > https://codereview.chromium.org/300113009/diff/1/chrome/ > browser/ui/cocoa/base_bubble_controller.mm#newcode249 > chrome/browser/ui/cocoa/base_bubble_controller.mm:249: // it may pop up > a menu. windowDidResignKey will not exec untill > nit: add a : after windowDidResignKey (it's part of the name) > > https://codereview.chromium.org/300113009/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2014/05/29 09:15:27, Xuefei Ren wrote: > Hi there, > I was trying to write an unit test today. But I got a problem. Here it > is. > As you known, my patch is to solve the contextual menu problem. Before > my patch, when we right-click(maybe left-click) on an object which can > pop up a contextual menu, the bubble won't disappear because we use > performSelector(windowDidResignKey:). windowDidResignKey: will not > got a chance to run until the contextual menu is closed. > When I tried to make a test, I need to test some values with > EXPECT_TRUE/FASLE. > The problem is, after a contextual menu pops up, my code will not run. > It only can run after contextual menu is closed. So this is a Modal > Dialogue Box > problem. > Now I don't know how to use a UNIT TEST to prove my patch. But it is easy > to prove > by manually operations which is not so strict. > Any ideas? Take a look at these examples, which test around popup menus: https://code.google.com/p/chromium/codesearch#chromium/src/ui/base/cocoa/menu... https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/... Basically, you need to schedule work run in the right run loop mode.
Oh, thanks. Great help. On Thu, May 29, 2014 at 9:19 PM, <rsesek@chromium.org> wrote: > On 2014/05/29 09:15:27, Xuefei Ren wrote: > >> Hi there, >> I was trying to write an unit test today. But I got a problem. Here it >> is. >> As you known, my patch is to solve the contextual menu problem. Before >> my patch, when we right-click(maybe left-click) on an object which can >> pop up a contextual menu, the bubble won't disappear because we use >> performSelector(windowDidResignKey:). windowDidResignKey: will not >> got a chance to run until the contextual menu is closed. >> When I tried to make a test, I need to test some values with >> EXPECT_TRUE/FASLE. >> The problem is, after a contextual menu pops up, my code will not run. >> It only can run after contextual menu is closed. So this is a Modal >> Dialogue Box >> problem. >> Now I don't know how to use a UNIT TEST to prove my patch. But it is easy >> to prove >> by manually operations which is not so strict. >> Any ideas? >> > > Take a look at these examples, which test around popup menus: > > https://code.google.com/p/chromium/codesearch#chromium/ > src/ui/base/cocoa/menu_controller_unittest.mm&q=menu_ > controller_unittest.mm&sq=package:chromium&type=cs&l=353 > https://code.google.com/p/chromium/codesearch#chromium/ > src/chrome/browser/ui/cocoa/menu_button_unittest.mm&q= > NSEventTrackingRunLoopMode&sq=package:chromium&dr=C&l=159 > > Basically, you need to schedule work run in the right run loop mode. > > https://codereview.chromium.org/300113009/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
I see you uploaded a test. Is this ready for another look?
Oh,yes,please take a look. 发自我的 iPhone > 在 2014年6月5日，3:47，rsesek@chromium.org 写道： > > I see you uploaded a test. Is this ready for another look? > > https://codereview.chromium.org/300113009/ To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
And you can assign me some bugs to me. My work is easy and I have a lot of time to do it. Thanks. On Thu, Jun 5, 2014 at 7:00 AM, Gmail <xrenishere@gmail.com> wrote: > Oh,yes,please take a look. > > 发自我的 iPhone > > > 在 2014年6月5日，3:47，rsesek@chromium.org 写道： > > > > I see you uploaded a test. Is this ready for another look? > > > > https://codereview.chromium.org/300113009/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Thanks for writing a good test! These are just some stylistic comments. If you're interested in working on more bugs, please feel free to find bugs in the bug tracker that are marked Available. https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... File chrome/browser/ui/cocoa/base_bubble_controller.mm (right): https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller.mm:249: // it may pop up a menu. windowDidResignKey: will not exec until nit: exec -> execute/run. We don't abbreviate in comments. https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller.mm:254: } nit: indent 1 more space https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... File chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm (left): https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:197: chrome::testing::NSRunLoopRunAllPending(); What's the rationale for removing these calls? https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... File chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm (right): https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:22: @private nit: @private is only indented 1 space https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:29: - (id)initWithMenu:(NSMenu*) menu AndWindow:(NSWindow*) window; nit: No space after ) for arguments. I'm mentioning it here, but please fix throughout the test. https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:29: - (id)initWithMenu:(NSMenu*) menu AndWindow:(NSWindow*) window; nit: AndWindow: -> andWindow: https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:49: [menu_ setDelegate: self]; nit: No space after : in method invocations. Please fix throughout your new test. https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:69: - (void)menuWillOpen:(NSMenu *)menu { nit: No space before * in arguments. Same with line 82. https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:73: NSArray* modes = [NSArray arrayWithObjects:NSEventTrackingRunLoopMode, You can use the ObjC array literal syntax @[ NSEventTrackingRunLoopMode, NSDefaultRunLoopMode ] https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:311: [[NSMenu alloc] initWithTitle:@""]); nit: Only indent 6 spaces total. https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:315: ContextMenuController* menu_controller = [[ContextMenuController alloc] This is leaked, so you should put it in a scoped_nsobject<>. https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:324: NSMakePoint(10, 10), nit: Only indent 6 spaces total, and you can join this line with the next. https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:328: EXPECT_TRUE([menu_controller isMenuOpen]); nit: Indent blocks 4 spaces. https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:338: // See ContextualMenuController::MenuWillOpen. This is how you name a C++ method. This should be written -[ContextMenuController menuWillOpen:].
https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... File chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm (left): https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:197: chrome::testing::NSRunLoopRunAllPending(); On 2014/06/05 14:22:50, rsesek wrote: > What's the rationale for removing these calls? Before my patch, the bubble window close depends on RegisterKeyStateEventTap which monitors NSLeftMouseDownMask. And it will run a block that uses [self performSelector:@selector(windowDidRegisterKey:). Now my patch change it in two ways: 1. Add NSRightMouseDownMask. 2. Replace performSelector with executing windowDidRegisterKey directly. [self windowDidRegisterKey] That is because when right-click with a contextual menu displayed, the windowDidRegisterKey will not run. Using chrome::testing::NSRunLoopRunAllPending() is to make sure windowDidResignKey; is executed before verifying EXPECT_TURE([window isVisible]); Since we use no PerformSelector, we don't need it anymore.
On 2014/06/05 14:22:50, rsesek wrote: > Thanks for writing a good test! These are just some stylistic comments. > > If you're interested in working on more bugs, please feel free to find bugs in > the bug tracker that are marked Available. > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > File chrome/browser/ui/cocoa/base_bubble_controller.mm (right): > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller.mm:249: // it may pop up a menu. > windowDidResignKey: will not exec until > nit: exec -> execute/run. We don't abbreviate in comments. > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller.mm:254: } > nit: indent 1 more space > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > File chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm (left): > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:197: > chrome::testing::NSRunLoopRunAllPending(); > What's the rationale for removing these calls? > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > File chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm (right): > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:22: @private > nit: @private is only indented 1 space > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:29: - > (id)initWithMenu:(NSMenu*) menu AndWindow:(NSWindow*) window; > nit: No space after ) for arguments. I'm mentioning it here, but please fix > throughout the test. > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:29: - > (id)initWithMenu:(NSMenu*) menu AndWindow:(NSWindow*) window; > nit: AndWindow: -> andWindow: > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:49: [menu_ > setDelegate: self]; > nit: No space after : in method invocations. Please fix throughout your new > test. > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:69: - > (void)menuWillOpen:(NSMenu *)menu { > nit: No space before * in arguments. Same with line 82. > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:73: NSArray* modes = > [NSArray arrayWithObjects:NSEventTrackingRunLoopMode, > You can use the ObjC array literal syntax @[ NSEventTrackingRunLoopMode, > NSDefaultRunLoopMode ] > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:311: [[NSMenu alloc] > initWithTitle:@""]); > nit: Only indent 6 spaces total. > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:315: > ContextMenuController* menu_controller = [[ContextMenuController alloc] > This is leaked, so you should put it in a scoped_nsobject<>. > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:324: NSMakePoint(10, > 10), > nit: Only indent 6 spaces total, and you can join this line with the next. > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:328: > EXPECT_TRUE([menu_controller isMenuOpen]); > nit: Indent blocks 4 spaces. > > https://codereview.chromium.org/300113009/diff/20001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:338: // See > ContextualMenuController::MenuWillOpen. > This is how you name a C++ method. This should be written > -[ContextMenuController menuWillOpen:]. A new patch is ready. Style issue is fixed.
On 2014/06/05 14:22:50, rsesek wrote: > If you're interested in working on more bugs, please feel free to find bugs in > the bug tracker that are marked Available. To expand on this more, the bug query "os=mac status=available" will show all open and unassigned Mac bugs, and then you can type cr=ui-browser- and then autocomplete will show you various UI components, and you can select ones you're interested in. https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... File chrome/browser/ui/cocoa/base_bubble_controller.mm (right): https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller.mm:254: } nit: this should line up with the |if| above on line 247. You out-dented one space rather than indenting one more space. https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... File chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm (right): https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:11: #import "chrome/browser/ui/cocoa/run_loop_testing.h" Remove this #import. https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:311: action: nil nit: align the colons and remove space after https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:336: // See -[ContextualMenuController MenuWillOpen:]. nit: lowercase menuWillOpen:
On 2014/06/06 14:00:34, rsesek wrote: > On 2014/06/05 14:22:50, rsesek wrote: > > If you're interested in working on more bugs, please feel free to find bugs in > > the bug tracker that are marked Available. > > To expand on this more, the bug query "os=mac status=available" will show all > open and unassigned Mac bugs, and then you can type cr=ui-browser- and then > autocomplete will show you various UI components, and you can select ones you're > interested in. > > https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... > File chrome/browser/ui/cocoa/base_bubble_controller.mm (right): > > https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller.mm:254: } > nit: this should line up with the |if| above on line 247. You out-dented one > space rather than indenting one more space. > > https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... > File chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm (right): > > https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:11: #import > "chrome/browser/ui/cocoa/run_loop_testing.h" > Remove this #import. > > https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:311: action: nil > nit: align the colons and remove space after > > https://codereview.chromium.org/300113009/diff/40001/chrome/browser/ui/cocoa/... > chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm:336: // See > -[ContextualMenuController MenuWillOpen:]. > nit: lowercase menuWillOpen: OK, I hope I can help. And another patch is ready.
LGTM
The CQ bit was checked by xrenishere@gmail.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xrenishere@gmail.com/300113009/60001
The CQ bit was unchecked by xrenishere@gmail.com
The CQ bit was checked by xrenishere@gmail.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xrenishere@gmail.com/300113009/60001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...) chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...) win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
The CQ bit was checked by xrenishere@gmail.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xrenishere@gmail.com/300113009/60001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
Hi thakis,sadrul,ben,sky, I'd appreciate that if any of you take a minute to review my patch. I need a LGTM from OWNER for ui/events/test/cocoa_test_event_utils.h/mm Thank you very much.
thakis should review those changes On Tue, Jun 10, 2014 at 6:37 PM, <xrenishere@gmail.com> wrote: > Hi thakis,sadrul,ben,sky, > I'd appreciate that if any of you take a minute to review my patch. I > need a > LGTM from OWNER for > ui/events/test/cocoa_test_event_utils.h/mm > > Thank you very much. > > https://codereview.chromium.org/300113009/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2014/06/11 15:46:48, sky wrote: > thakis should review those changes Scott, could you just do a stamp approval? I've reviewed the changes and they're LGTM, but I'm not an OWNER for that specific cocoa_* file.
Ok, rsesek is good enough too. LGTM
The CQ bit was checked by xrenishere@gmail.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xrenishere@gmail.com/300113009/60001
Message was sent while issue was closed.
Change committed as 276875 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
