[Autofill] Add Details Section

chrome/browser/ui/cocoa/autofill/autofill_details_container.mm

+// Copyright (c) 2013 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/autofill/autofill_details_container.h"
+
+#include "chrome/browser/ui/autofill/autofill_dialog_controller.h"
+#import "chrome/browser/ui/cocoa/autofill/autofill_section_container.h"
+
+@implementation AutofillDetailsContainer
+
+- (id)initWithController:(autofill::AutofillDialogController*)controller {
+  if (self = [super init]) {
+    controller_ = controller;
+  }
+  return self;
+}
+
+- (void)addSection:(autofill::DialogSection)section {
+  scoped_nsobject<AutofillSectionContainer> sectionContainer;
+  sectionContainer.reset(

[sail, 2013/05/01 17:12:36]

move into constructor

[groby-ooo-7-16, 2013/05/01 20:37:44]

I'm confused - I call this several times, so I can't just move this code into
the constructor.

I'd also like to keep those calls in loadView, since they're actually
constructing sub-views.

On 2013/05/01 17:12:36, sail wrote:

[sail, 2013/05/01 21:38:38]

Oops, sorry. I meant combine declaration and initialization.

[groby-ooo-7-16, 2013/05/01 22:36:41]

Done.

+      [[AutofillSectionContainer alloc] initWithController:controller_
+                                                forSection:section]);
+  [details_ addObject:sectionContainer];
+}
+
+- (void)loadView {
+  details_.reset([[NSMutableArray alloc] init]);
+
+  [self addSection:autofill::SECTION_EMAIL];
+  [self addSection:autofill::SECTION_CC];
+  [self addSection:autofill::SECTION_BILLING];
+  // TODO(groby): Add SECTION_CC_BILLING once toggling is enabled.
+  [self addSection:autofill::SECTION_SHIPPING];
+
+  NSSize detailSize;

[sail, 2013/05/01 17:12:36]

need to initialize

[groby-ooo-7-16, 2013/05/01 20:37:44]

Done. 

Only for my clarification: ObjC style guide says don't do that -
(http://www.corp.google.com/eng/doc/objcguide.xml#Initialization)

I assume does that only apply to objects, but not to structs? 

On 2013/05/01 17:12:36, sail wrote:

[sail, 2013/05/01 21:38:38]

That only applies to member variables.

+  for(AutofillSectionContainer* container in details_.get()) {
+    detailSize.width = NSWidth([[container view] frame]);

[sail, 2013/05/01 17:12:36]

should this be a max or something?

[groby-ooo-7-16, 2013/05/01 20:37:44]

I was torn here - I only need the width of the first one, since they're all
guaranteed to be the same. A max gives the misleading impression they're not,
this looks like I missed max.

Changed to a much simpler approach in new CL.
On 2013/05/01 17:12:36, sail wrote:

+    detailSize.height += NSHeight([[container view] frame]);
+  }
+
+  NSPoint origin = NSMakePoint(0, detailSize.height);
+
+  self.view = [[NSView alloc] init];

[sail, 2013/05/01 17:12:36]

I don't think we should use dot notation (it's pretty inconsistent with the rest
of the code)

[groby-ooo-7-16, 2013/05/01 20:37:44]

Done.

If we don't use dot notation, does it make sense to declare non-synthesized
properties as @property at all?

Also, FYI only, it's in sporadic use throughout our code - see
https://code.google.com/p/chromium/codesearch#search/&q=%22self.%22%20lang:%5...

On 2013/05/01 17:12:36, sail wrote:

+  [[self view] setFrameSize:detailSize];
+
+  for(AutofillSectionContainer* container in details_.get()) {

[sail, 2013/05/01 17:12:36]

Alternatively, you could have single loop that iterates backwards.

[groby-ooo-7-16, 2013/05/01 20:37:44]

TIL: reverseObjectEnumerator - done.
On 2013/05/01 17:12:36, sail wrote:

+    origin.y -= NSHeight([[container view] frame]);
+    [[container view] setFrameOrigin:origin];
+    [self.view addSubview:[container view]];
+  }
+}
+
+@end