Objective-C readability review for Robert Sesek <rsesek@google.com>.

chrome/browser/ui/cocoa/page_info_bubble_controller.mm

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/page_info_bubble_controller.h"
 
 #include "app/l10n_util.h"
 #include "app/l10n_util_mac.h"
 #include "base/message_loop.h"
 #include "base/sys_string_conversions.h"
 #include "base/task.h"
@@ skipping 34 matching lines @@
 - (CGFloat)addSeparatorToSubviews:(NSMutableArray*)subviews
                          atOffset:(CGFloat)offset;
 - (NSPoint)anchorPointForWindowWithHeight:(CGFloat)bubbleHeight
                              parentWindow:(NSWindow*)parent;
 @end
 
 // This simple NSView subclass is used as the single subview of the page info
 // bubble's window's contentView. Drawing is flipped so that layout of the
 // sections is easier. Apple recommends flipping the coordinate origin when
 // doing a lot of text layout because it's more natural.
-@interface PageInfoContentView : NSView {
-}
+@interface PageInfoContentView : NSView
 @end
 @implementation PageInfoContentView
 - (BOOL)isFlipped {
   return YES;
 }
 @end
 
 namespace {
 
 // The width of the window, in view coordinates. The height will be determined
 // by the content.
-const NSInteger kWindowWidth = 380;
+const CGFloat kWindowWidth = 380;
 
 // Spacing in between sections.
-const NSInteger kVerticalSpacing = 10;
+const CGFloat kVerticalSpacing = 10;
 
 // Padding along on the X-axis between the window frame and content.
-const NSInteger kFramePadding = 10;
+const CGFloat kFramePadding = 10;
 
 // Spacing between the optional headline and description text views.
-const NSInteger kHeadlineSpacing = 2;
+const CGFloat kHeadlineSpacing = 2;
 
 // Spacing between the image and the text.
-const NSInteger kImageSpacing = 10;
+const CGFloat kImageSpacing = 10;
 
 // Square size of the image.
 const CGFloat kImageSize = 30;
 
 // The X position of the text fields. Variants for with and without an image.
 const CGFloat kTextXPositionNoImage = kFramePadding;
 const CGFloat kTextXPosition = kTextXPositionNoImage + kImageSize +
     kImageSpacing;
 
 // Width of the text fields.
 const CGFloat kTextWidth = kWindowWidth - (kImageSize + kImageSpacing +
     kFramePadding * 2);
 
 // Bridge that listens for change notifications from the model.
 class PageInfoModelBubbleBridge : public PageInfoModel::PageInfoModelObserver {

[pink (ping after 24hrs), 2011/01/24 19:22:23]

worth adding a DISALLOW_COPY_AND_ASSIGN on this?

[Robert Sesek, 2011/01/24 19:55:17]

Done.

  public:
   PageInfoModelBubbleBridge()
       : controller_(nil),
         ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)) {
   }
 
   // PageInfoModelObserver implementation.
   virtual void ModelChanged() {
     // Check to see if a layout has already been scheduled.
     if (!task_factory_.empty())
@@ skipping 53 matching lines @@
 
 }  // namespace browser
 
 @implementation PageInfoBubbleController
 
 @synthesize certID = certID_;
 
 - (id)initWithPageInfoModel:(PageInfoModel*)model
               modelObserver:(PageInfoModel::PageInfoModelObserver*)bridge
                parentWindow:(NSWindow*)parentWindow {
+  DCHECK(parentWindow);
+
   // Use an arbitrary height because it will be changed by the bridge.
   NSRect contentRect = NSMakeRect(0, 0, kWindowWidth, 0);
   // Create an empty window into which content is placed.
   scoped_nsobject<InfoBubbleWindow> window(
       [[InfoBubbleWindow alloc] initWithContentRect:contentRect
                                           styleMask:NSBorderlessWindowMask
                                             backing:NSBackingStoreBuffered
                                               defer:NO]);
 
   if ((self = [super initWithWindow:window.get()
@@ skipping 207 matching lines @@
 
   return NSHeight([certButton frame]);
 }
 
 // Adds the state image at a pre-determined x position and the given y. This
 // does not affect the next Y position because the image is placed next to
 // a text field that is larger and accounts for the image's size.
 - (void)addImageViewForInfo:(const PageInfoModel::SectionInfo&)info
                  toSubviews:(NSMutableArray*)subviews
                    atOffset:(CGFloat)offset {
-  NSRect frame = NSMakeRect(kFramePadding, offset, kImageSize,
-      kImageSize);
+  NSRect frame =
+      NSMakeRect(kFramePadding, offset, kImageSize, kImageSize);
   scoped_nsobject<NSImageView> imageView(
       [[NSImageView alloc] initWithFrame:frame]);
   [imageView setImageFrameStyle:NSImageFrameNone];
   [imageView setImage:model_->GetIconImage(info.icon_id)];
   [subviews addObject:imageView.get()];
 }
 
 // Adds the help center button that explains the icons. Returns the y position
 // delta for the next offset.
 - (CGFloat)addHelpButtonToSubviews:(NSMutableArray*)subviews
@@ skipping 43 matching lines @@
     LocationBarViewMac* locationBar = [controller locationBarBridge];
     if (locationBar) {
       NSPoint bubblePoint = locationBar->GetPageInfoBubblePoint();
       origin = [parent convertBaseToScreen:bubblePoint];
     }
   }
   return origin;
 }
 
 @end