ui/base/test/windowed_nsnotification_observer.h - Issue 1146873002: [MacViews] Enable dragging a window by its caption/draggable areas.

Side by Side Diff: ui/base/test/windowed_nsnotification_observer.h

Issue 1146873002: [MacViews] Enable dragging a window by its caption/draggable areas. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Address comments. Created 5 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 // Copyright 2015 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #ifndef UI_GFX_TEST_WINDOWED_NSNOTIFICATION_OBSERVER_H_
	tapted 2015/06/03 07:04:48 UI_GFX_TEST -> UI_BASE_TEST UI_GFX_TEST -> UI_BASE_TEST jackhou1 2015/06/03 08:13:50 Done. Show quoted text On 2015/06/03 07:04:48, tapted wrote: > UI_GFX_TEST -> UI_BASE_TEST Done.
	6 #define UI_GFX_TEST_WINDOWED_NSNOTIFICATION_OBSERVER_H_

	7

	8 #import <Cocoa/Cocoa.h>
	tapted 2015/06/03 07:04:48 This should be <Foundation/Foundation.h> since App This should be <Foundation/Foundation.h> since AppKit isn't being used. All you "need" is NSObject (even NSString could be forward-declared, but Foundation will give it to you anyway, and you only #import NSObject.h directly in special cases. (but you'll probably need AppKit.h or Cocoa.h in the .mm for NSWorkspace and things jackhou1 2015/06/03 08:13:51 Done. Show quoted text On 2015/06/03 07:04:48, tapted wrote: > This should be <Foundation/Foundation.h> since AppKit isn't being used. All you > "need" is NSObject (even NSString could be forward-declared, but Foundation will > give it to you anyway, and you only #import NSObject.h directly in special > cases. > > (but you'll probably need AppKit.h or Cocoa.h in the .mm for NSWorkspace and > things Done.
	9

	10 #import "base/mac/scoped_nsobject.h"

	11 #include "base/memory/scoped_ptr.h"
	tapted 2015/06/03 07:04:48 nit: not needed (see below) nit: not needed (see below) jackhou1 2015/06/03 08:13:50 Done. Show quoted text On 2015/06/03 07:04:48, tapted wrote: > nit: not needed (see below) Done.
	12 #include "base/run_loop.h"
	tapted 2015/06/03 07:04:48 nit: forward-declare nit: forward-declare jackhou1 2015/06/03 08:13:51 Done. Show quoted text On 2015/06/03 07:04:48, tapted wrote: > nit: forward-declare Done.
	13

	14 // Watches for NSNotifications.
	tapted 2015/06/03 07:04:49 something more verbose? (i.e. "sell" this class - something more verbose? (i.e. "sell" this class - describe the situations where someone would use it). Should say stuff like ~immediately starts listening once created jackhou1 2015/06/03 08:13:51 Done. Show quoted text On 2015/06/03 07:04:49, tapted wrote: > something more verbose? (i.e. "sell" this class - describe the situations where > someone would use it). Should say stuff like ~immediately starts listening once > created Done.
	15 @interface WindowedNSNotificationObserver : NSObject {
	tapted 2015/06/03 07:04:48 Do you know the history behind the "Windowed" pref Do you know the history behind the "Windowed" prefix? I don't think it's right. Maybe "CocoaNotificationMessageWaiter" ? jackhou1 2015/06/03 08:13:51 This is the same as WindowedNotificationObserver i Show quoted text On 2015/06/03 07:04:48, tapted wrote: > Do you know the history behind the "Windowed" prefix? I don't think it's right. > > Maybe "CocoaNotificationMessageWaiter" ? This is the same as WindowedNotificationObserver in content/public/test/test_utils.h. tapted 2015/06/03 09:12:57 aah, sounds good. Show quoted text On 2015/06/03 08:13:51, jackhou1 wrote: > On 2015/06/03 07:04:48, tapted wrote: > > Do you know the history behind the "Windowed" prefix? I don't think it's > right. > > > > Maybe "CocoaNotificationMessageWaiter" ? > > This is the same as WindowedNotificationObserver in > content/public/test/test_utils.h. aah, sounds good.
	16 @private

	17 base::scoped_nsobject<NSString> bundleId_;

	18 BOOL notificationReceived_;

	19 scoped_ptr<base::RunLoop> runLoop_;
	tapted 2015/06/03 07:04:48 This can just be a raw pointer to base::RunLoop (s This can just be a raw pointer to base::RunLoop (see below) jackhou1 2015/06/03 08:13:51 Done. Show quoted text On 2015/06/03 07:04:48, tapted wrote: > This can just be a raw pointer to base::RunLoop (see below) Done.
	20 }

	21

	22 // Watch for a notification on the default notification center.
	tapted 2015/06/03 07:04:48 nit: "a notification" -> "an NSNotification" nit: "a notification" -> "an NSNotification" jackhou1 2015/06/03 08:13:51 Done. Show quoted text On 2015/06/03 07:04:48, tapted wrote: > nit: "a notification" -> "an NSNotification" Done.
	23 - (id)initForNotification:(NSString*)name;

	24 // Watch for a notification on the shared workspace notification center for the
	tapted 2015/06/03 07:04:48 nit: blank line before nit: blank line before jackhou1 2015/06/03 08:13:51 Done. Show quoted text On 2015/06/03 07:04:48, tapted wrote: > nit: blank line before Done.
	25 // given application.

	26 - (id)initForNotification:(NSString*)name
	tapted 2015/06/03 07:04:48 maybe `initForWorkspaceNotification`? maybe `initForWorkspaceNotification`? jackhou1 2015/06/03 08:13:51 Done. Show quoted text On 2015/06/03 07:04:48, tapted wrote: > maybe `initForWorkspaceNotification`? Done.
	27 andBundleId:(NSString*)bundleId;
	tapted 2015/06/03 07:04:48 andBundleId -> bundleId (or maybe `forBundleId` b andBundleId -> bundleId (or maybe `forBundleId` but I don't think I ever see `and` on arguments) jackhou1 2015/06/03 08:13:50 Done. Show quoted text On 2015/06/03 07:04:48, tapted wrote: > andBundleId -> bundleId (or maybe `forBundleId` but I don't think I ever see > `and` on arguments) Done.
	28 - (void)wait;
	tapted 2015/06/03 07:04:48 needs a comment. Needs to say stuff like ~immediat needs a comment. Needs to say stuff like ~immediately stops observing, so users don't think they can call wait multiple times. jackhou1 2015/06/03 08:13:51 Done. Show quoted text On 2015/06/03 07:04:48, tapted wrote: > needs a comment. Needs to say stuff like ~immediately stops observing, so users > don't think they can call wait multiple times. Done.
	29 @end

	30

	31 #endif // UI_GFX_TEST_WINDOWED_NSNOTIFICATION_OBSERVER_H_

	32
	tapted 2015/06/03 07:04:48 nit: remove extra blank line nit: remove extra blank line jackhou1 2015/06/03 08:13:51 Done. Show quoted text On 2015/06/03 07:04:48, tapted wrote: > nit: remove extra blank line Done.
OLD	NEW

« no previous file with comments | « ui/base/BUILD.gn ('k') | ui/base/test/windowed_nsnotification_observer.mm » ('j') | ui/base/test/windowed_nsnotification_observer.mm » ('J')