[subresource_filter] Mac UI updated and xib replaced with code based layout.

chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm

Index: chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
diff --git a/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm b/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
index 41cd17d855985793ce4ed5ab6ed978414b0da30e..9355db223b2a595ab77ee6421fa31e4668061434 100644
--- a/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
+++ b/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
@@ -15,10 +15,12 @@
 #include "chrome/browser/plugins/plugin_finder.h"
 #include "chrome/browser/plugins/plugin_metadata.h"
 #import "chrome/browser/ui/cocoa/info_bubble_view.h"
+#import "chrome/browser/ui/cocoa/info_bubble_window.h"

[shivanisha, 2017/05/03 18:10:35]

removed this unused include

 #import "chrome/browser/ui/cocoa/l10n_util.h"
+#import "chrome/browser/ui/cocoa/location_bar/content_setting_decoration.h"
+#import "chrome/browser/ui/cocoa/subresource_filter/subresource_filter_bubble_controller.h"
 #include "chrome/browser/ui/content_settings/content_setting_bubble_model.h"
 #include "chrome/browser/ui/content_settings/content_setting_media_menu_model.h"
-#import "chrome/browser/ui/cocoa/location_bar/content_setting_decoration.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/browser/navigation_handle.h"
@@ -27,6 +29,7 @@
 #include "skia/ext/skia_utils_mac.h"
 #import "third_party/google_toolbox_for_mac/src/AppKit/GTMUILocalizerAndLayoutTweaker.h"
 #import "ui/base/cocoa/controls/hyperlink_button_cell.h"
+#import "ui/base/cocoa/window_size_constants.h"

[Robert Sesek, 2017/05/02 21:51:47]

Unused

[shivanisha, 2017/05/03 18:10:35]

removed

 #include "ui/base/l10n/l10n_util.h"
 
 using content::PluginService;
@@ -205,13 +208,15 @@ class ContentSettingBubbleWebContentsObserverBridge
   DISALLOW_COPY_AND_ASSIGN(ContentSettingBubbleWebContentsObserverBridge);
 };
 
-@interface ContentSettingBubbleController(Private)
+@interface ContentSettingBubbleController (Protected)
 - (id)initWithModel:(ContentSettingBubbleModel*)settingsBubbleModel
         webContents:(content::WebContents*)webContents
        parentWindow:(NSWindow*)parentWindow
          decoration:(ContentSettingDecoration*)decoration
          anchoredAt:(NSPoint)anchoredAt;
-- (NSString*)getNibPathForModel:(ContentSettingBubbleModel*)model;
++ (NSString*)getNibPathForModel:(ContentSettingBubbleModel*)model;
++ (ContentSettingBubbleController*)getControllerForModel:
+    (ContentSettingBubbleModel*)model;
 - (NSButton*)hyperlinkButtonWithFrame:(NSRect)frame
                                 title:(NSString*)title
                                  icon:(NSImage*)icon
@@ -219,13 +224,13 @@ class ContentSettingBubbleWebContentsObserverBridge
 - (void)initializeBlockedPluginsList;
 - (void)initializeTitle;
 - (void)initializeMessage;
+- (void)initalizeManageCheckbox;
 - (void)initializeRadioGroup;
 - (void)initializeItemList;
 - (void)initializeGeoLists;
 - (void)initializeMediaMenus;
 - (void)initializeMIDISysExLists;
 - (void)sizeToFitLoadButton;
-- (void)initManageDoneButtons;
 - (void)removeInfoButton;
 - (void)popupLinkClicked:(id)sender;
 - (void)clearGeolocationForCurrentHost:(id)sender;
@@ -240,13 +245,16 @@ showForModel:(ContentSettingBubbleModel*)contentSettingBubbleModel
 parentWindow:(NSWindow*)parentWindow
   decoration:(ContentSettingDecoration*)decoration
   anchoredAt:(NSPoint)anchor {
-  // Autoreleases itself on bubble close.
-  return [[ContentSettingBubbleController alloc]
-      initWithModel:contentSettingBubbleModel
-        webContents:webContents
-       parentWindow:parentWindow
-         decoration:decoration
-         anchoredAt:anchor];
+  ContentSettingBubbleController* controller =
+      [self getControllerForModel:contentSettingBubbleModel];
+
+  DCHECK(controller);
+
+  return [controller initWithModel:contentSettingBubbleModel
+                       webContents:webContents
+                      parentWindow:parentWindow
+                        decoration:decoration
+                        anchoredAt:anchor];
 }
 
 struct ContentTypeToNibPath {
@@ -278,7 +286,8 @@ const ContentTypeToNibPath kNibPaths[] = {
   observerBridge_.reset(
     new ContentSettingBubbleWebContentsObserverBridge(webContents, self));
 
-  NSString* nibPath = [self getNibPathForModel:model.get()];
+  NSString* nibPath =
+      [ContentSettingBubbleController getNibPathForModel:model.get()];
 
   DCHECK_NE(0u, [nibPath length]);
 
@@ -292,7 +301,36 @@ const ContentTypeToNibPath kNibPaths[] = {
   return self;
 }
 
-- (NSString*)getNibPathForModel:(ContentSettingBubbleModel*)model {
+- (id)initWithModel:(ContentSettingBubbleModel*)contentSettingBubbleModel
+        webContents:(content::WebContents*)webContents
+             window:(NSWindow*)window
+       parentWindow:(NSWindow*)parentWindow
+         decoration:(ContentSettingDecoration*)decoration
+         anchoredAt:(NSPoint)anchoredAt {
+  // This method takes ownership of |contentSettingBubbleModel| in all cases.
+  std::unique_ptr<ContentSettingBubbleModel> model(contentSettingBubbleModel);
+  DCHECK(model.get());
+  observerBridge_.reset(
+      new ContentSettingBubbleWebContentsObserverBridge(webContents, self));
+
+  contentSettingBubbleModel_ = std::move(model);
+
+  if ((self = [super initWithWindow:window
+                       parentWindow:parentWindow
+                         anchoredAt:anchoredAt])) {
+    decoration_ = decoration;
+    [self showWindow:nil];
+    NSRect newWindowFrame =
+        [window frameRectForContentRect:NSMakeRect(196, 376, 316, 134)];

[Robert Sesek, 2017/05/02 21:51:47]

Why is this done here?

[shivanisha, 2017/05/03 18:10:35]

Ah, remnant of earlier patch. Thanks, removed.

+
+    [window setFrame:newWindowFrame
+             display:YES
+             animate:[[self window] isVisible]];
+  }
+  return self;
+}
+
++ (NSString*)getNibPathForModel:(ContentSettingBubbleModel*)model {
   NSString* nibPath = @"";
 
   ContentSettingSimpleBubbleModel* simple_bubble = model->AsSimpleBubbleModel();
@@ -310,14 +348,27 @@ const ContentTypeToNibPath kNibPaths[] = {
   if (model->AsMediaStreamBubbleModel())
     nibPath = @"ContentBlockedMedia";
 
-  if (model->AsSubresourceFilterBubbleModel())
-    nibPath = @"ContentSubresourceFilter";
-
   if (model->AsDownloadsBubbleModel())
     nibPath = @"ContentBlockedDownloads";
   return nibPath;
 }
 
++ (ContentSettingBubbleController*)getControllerForModel:

[Robert Sesek, 2017/05/02 21:51:47]

naming: call this allocControllerForModel: to make it explicit that this is
returning a strong reference

[shivanisha, 2017/05/03 18:10:35]

done

+    (ContentSettingBubbleModel*)model {
+  // Check if the view is expressed in xib file or not.
+  NSString* nibPath = [self getNibPathForModel:model];
+
+  // Autoreleases itself on bubble close.
+
+  if ([nibPath length] > 0u)
+    return [ContentSettingBubbleController alloc];
+
+  if (model->AsSubresourceFilterBubbleModel())
+    return [SubresourceFilterBubbleController alloc];
+
+  return nil;
+}
+
 - (void)initializeTitle {
   if (!titleLabel_)
     return;
@@ -355,6 +406,24 @@ const ContentTypeToNibPath kNibPaths[] = {
   [messageLabel_ setFrame:messageFrame];
 }
 
+- (void)initalizeManageCheckbox {

[Robert Sesek, 2017/05/02 21:51:47]

spelling: initialize

[shivanisha, 2017/05/03 18:10:35]

done. Also moved this to the subclass

+  if (!manageCheckbox_)
+    return;
+
+  NSString* label = base::SysUTF16ToNSString(
+      contentSettingBubbleModel_->bubble_content().manage_text);
+  [manageCheckbox_ setTitle:label];
+
+  CGFloat deltaY =
+      [GTMUILocalizerAndLayoutTweaker sizeToFitView:manageCheckbox_].height;
+  NSRect windowFrame = [[self window] frame];
+  windowFrame.size.height += deltaY;
+  [[self window] setFrame:windowFrame display:NO];
+  NSRect manageCheckboxFrame = [manageCheckbox_ frame];
+  manageCheckboxFrame.origin.y -= deltaY;
+  [manageCheckbox_ setFrame:manageCheckboxFrame];
+}
+
 - (void)initializeRadioGroup {
   // NOTE! Tags in the xib files must match the order of the radio buttons
   // passed in the radio_group and be 1-based, not 0-based.
@@ -769,12 +838,26 @@ const ContentTypeToNibPath kNibPaths[] = {
 - (void)initManageDoneButtons {
   const ContentSettingBubbleModel::BubbleContent& content =
       contentSettingBubbleModel_->bubble_content();
-  [manageButton_ setTitle:base::SysUTF16ToNSString(content.manage_text)];
-  [GTMUILocalizerAndLayoutTweaker sizeToFitView:[manageButton_ superview]];
+
+  CGFloat requiredWidthForManageButton = 0.0;
+  if (manageButton_) {
+    [manageButton_ setTitle:base::SysUTF16ToNSString(content.manage_text)];
+    [GTMUILocalizerAndLayoutTweaker sizeToFitView:[manageButton_ superview]];
+    requiredWidthForManageButton =
+        NSMaxX([manageButton_ frame]) + kManageDonePadding;
+  }
+
+  if (!manageButton_ && !doneButton_)
+    return;
+
+  NSString* doneLabel = base::SysUTF16ToNSString(content.done_button_text);
+  if ([doneLabel length] > 0u)
+    [doneButton_ setTitle:doneLabel];
 
   CGFloat actualWidth = NSWidth([[[self window] contentView] frame]);
-  CGFloat requiredWidth = NSMaxX([manageButton_ frame]) + kManageDonePadding +
-      NSWidth([[doneButton_ superview] frame]) - NSMinX([doneButton_ frame]);
+  CGFloat requiredWidth = requiredWidthForManageButton +
+                          NSWidth([[doneButton_ superview] frame]) -
+                          NSMinX([doneButton_ frame]);
   if (requiredWidth <= actualWidth || !doneButton_ || !manageButton_)
     return;
 
@@ -797,11 +880,13 @@ const ContentTypeToNibPath kNibPaths[] = {
 
   // Adapt window size to bottom buttons. Do this before all other layouting.
   if ((simple_bubble && !simple_bubble->bubble_content().manage_text.empty()) ||
-      contentSettingBubbleModel_->AsDownloadsBubbleModel())
+      contentSettingBubbleModel_->AsDownloadsBubbleModel() ||
+      contentSettingBubbleModel_->AsSubresourceFilterBubbleModel())

[Robert Sesek, 2017/05/02 21:51:47]

nit: add braces since the condition is multi-line

[shivanisha, 2017/05/03 18:10:35]

done

     [self initManageDoneButtons];
 
   [self initializeTitle];
   [self initializeMessage];
+  [self initalizeManageCheckbox];
 
   // Note that the per-content-type methods and |initializeRadioGroup| below
   // must be kept in the correct order, as they make interdependent adjustments
@@ -902,6 +987,11 @@ const ContentTypeToNibPath kNibPaths[] = {
   [self close];
 }
 
+- (IBAction)manageCheckboxChecked:(id)sender {
+  bool is_checked = [sender state] == NSOnState;

[Robert Sesek, 2017/05/02 21:51:47]

naming: isChecked (use camelCase in ObjC methods)

[shivanisha, 2017/05/03 18:10:35]

done

+  contentSettingBubbleModel_->OnManageCheckboxChecked(is_checked);
+}
+
 - (IBAction)mediaMenuChanged:(id)sender {
   NSPopUpButton* button = static_cast<NSPopUpButton*>(sender);
   auto it = mediaMenus_.find(sender);