DevTools: place DevTools WebContents underneath inspected WebContents.

chrome/browser/ui/cocoa/dev_tools_controller.mm

Index: chrome/browser/ui/cocoa/dev_tools_controller.mm
diff --git a/chrome/browser/ui/cocoa/dev_tools_controller.mm b/chrome/browser/ui/cocoa/dev_tools_controller.mm
index 6fb204d2f429373a7766476ec089c9d31985be52..fa5da6a5863fd0308e22dd8ea3bfe86be21c9151 100644
--- a/chrome/browser/ui/cocoa/dev_tools_controller.mm
+++ b/chrome/browser/ui/cocoa/dev_tools_controller.mm
@@ -5,6 +5,7 @@
 #import "chrome/browser/ui/cocoa/dev_tools_controller.h"
 
 #include <algorithm>
+#include <cmath>
 
 #include <Cocoa/Cocoa.h>
 
@@ -16,39 +17,102 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 #include "ui/base/cocoa/focus_tracker.h"
+#include "ui/gfx/size_conversions.h"
 
 using content::WebContents;
 
-@interface GraySplitView : NSSplitView {
-  BOOL dividerHidden_;
+@interface DevToolsContainerView : NSView {
+  NSSize topLeftContentsOffset_;
+  NSSize bottomRightContentsOffset_;
+  NSView* devToolsView_;
+  NSView* contentsView_;

[Avi (use Gerrit), 2013/12/09 16:13:36]

Comment these as being weak (ownership via -subviews).

[dgozman, 2013/12/10 14:51:47]

Done.

 }
 
-@property(assign, nonatomic) BOOL dividerHidden;
-
-- (NSColor*)dividerColor;
-- (CGFloat)dividerThickness;
+- (void)setContentsOffsetsTopLeft:(NSSize)topLeft
+                      bottomRight:(NSSize)bottomRight;
+- (void)adjustSubviews;
+- (void)resizeSubviewsWithOldSize:(NSSize)oldBoundsSize;

[Avi (use Gerrit), 2013/12/09 16:13:36]

unused; remove

[dgozman, 2013/12/10 14:51:47]

This is an NSView override. Removed from interface.

+- (void)showDevTools:(NSView*)devToolsView;
+- (void)hideDevTools;
 
 @end
 
 
-@implementation GraySplitView
+@implementation DevToolsContainerView
 
-@synthesize dividerHidden = dividerHidden_;
+- (DevToolsContainerView*) initWithFrame:(NSRect)frame {

[Avi (use Gerrit), 2013/12/09 16:13:36]

No space between ) and i.

[dgozman, 2013/12/10 14:51:47]

Done.

+  if (self = [super initWithFrame:frame]) {
+    devToolsView_ = nil;
+    contentsView_ = nil;
+    topLeftContentsOffset_ = NSZeroSize;
+    bottomRightContentsOffset_ = NSZeroSize;

[Avi (use Gerrit), 2013/12/09 16:13:36]

This initialization isn't needed; ivars start out zero-filled.

[dgozman, 2013/12/10 14:51:47]

Done.

+  }
+  return self;
+}
 
-- (NSColor*)dividerColor {
-  return [NSColor darkGrayColor];
+- (void)setContentsOffsetsTopLeft:(NSSize)topLeft
+                      bottomRight:(NSSize)bottomRight {
+  topLeftContentsOffset_ = topLeft;
+  bottomRightContentsOffset_ = bottomRight;
 }
 
-- (CGFloat)dividerThickness {
-  return dividerHidden_ ? 0 : [super dividerThickness];
+- (void)resizeSubviewsWithOldSize:(NSSize)oldBoundsSize {

[Avi (use Gerrit), 2013/12/09 16:13:36]

unused; remove

[dgozman, 2013/12/10 14:51:47]

This is NSView override invoked when view is resized.

[Avi (use Gerrit), 2013/12/10 16:32:40]

Oh.

+  [self adjustSubviews];
+}
+
+- (void)showDevTools:(NSView*)devToolsView {
+  NSArray* subviews = [self subviews];
+  DCHECK_EQ([subviews count], 1u);

[Avi (use Gerrit), 2013/12/09 16:13:36]

In DCHECK land, expected values are the first param. Switch the params here and
in several other places in this file.

[dgozman, 2013/12/10 14:51:47]

Done.

+  contentsView_ = [subviews objectAtIndex:0];
+  devToolsView_ = devToolsView;
+  // Place DevTools under contents.
+  [self addSubview:devToolsView positioned:NSWindowBelow relativeTo:nil];
+}
+
+- (void)hideDevTools {
+  DCHECK_EQ([[self subviews] count], 2u);
+  [devToolsView_ removeFromSuperview];
+  contentsView_ = nil;
+  devToolsView_ = nil;
+}
+
+- (void)adjustSubviews {
+  if (![[self subviews] count])
+    return;
+
+  if (!devToolsView_) {
+    DCHECK_EQ([[self subviews] count], 1u);
+    NSView* contents = [[self subviews] objectAtIndex:0];
+    [contents setFrame:[self bounds]];
+    return;
+  }
+
+  DCHECK_EQ([[self subviews] count], 2u);
+  NSRect bounds = [self bounds];
+
+  [devToolsView_ setFrame:bounds];
+
+  CGFloat contentsWidth = std::max(0.f, NSWidth(bounds) -
+      topLeftContentsOffset_.width - bottomRightContentsOffset_.width);
+  CGFloat contentsHeight = std::max(0.f, NSHeight(bounds) -
+      topLeftContentsOffset_.height - bottomRightContentsOffset_.height);
+  CGFloat contentsTop =
+      std::min(topLeftContentsOffset_.height, NSHeight(bounds));
+  CGFloat contentsLeft =
+      std::min(topLeftContentsOffset_.width, NSWidth(bounds));
+
+  [contentsView_ setFrame:NSMakeRect(
+      contentsLeft,
+      contentsTop,
+      contentsWidth,
+      contentsHeight)];

[Avi (use Gerrit), 2013/12/09 16:13:36]

Weird wrapping; can you get this to be on one line?

[dgozman, 2013/12/10 14:51:47]

Done.

 }
 
 @end
 
 @interface DevToolsController (Private)
-- (void)showDevToolsContainer;
-- (void)hideDevToolsContainer;
-- (void)updateDevToolsSplitPosition;
+- (void)showDevToolsView;
+- (void)hideDevToolsView;
 @end
 
 
@@ -56,29 +120,16 @@ using content::WebContents;
 
 - (id)init {
   if ((self = [super init])) {
-    splitView_.reset([[GraySplitView alloc] initWithFrame:NSZeroRect]);
-    [splitView_ setDividerStyle:NSSplitViewDividerStyleThin];
-    [splitView_ setVertical:NO];
-    [splitView_ setAutoresizingMask:NSViewWidthSizable|NSViewHeightSizable];
-    [splitView_ setDelegate:self];
-    [splitView_ setDividerHidden:NO];
-
-    dockSide_ = DEVTOOLS_DOCK_SIDE_BOTTOM;
+    devToolsContainerView_.reset(
+        [[DevToolsContainerView alloc] initWithFrame:NSZeroRect]);
+    [devToolsContainerView_
+        setAutoresizingMask:NSViewWidthSizable|NSViewHeightSizable];
   }
   return self;
 }
 
-- (void)dealloc {
-  [splitView_ setDelegate:nil];
-  [super dealloc];
-}
-
 - (NSView*)view {
-  return splitView_.get();
-}
-
-- (NSSplitView*)splitView {
-  return splitView_.get();
+  return devToolsContainerView_.get();
 }
 
 - (void)updateDevToolsForWebContents:(WebContents*)contents
@@ -86,145 +137,55 @@ using content::WebContents;
   DevToolsWindow* newDevToolsWindow = contents ?
       DevToolsWindow::GetDockedInstanceForInspectedTab(contents) : NULL;
 
-  // Fast return in case of the same window having same orientation.
-  if (devToolsWindow_ == newDevToolsWindow) {
-    if (!newDevToolsWindow ||
-        (newDevToolsWindow->dock_side() == dockSide_)) {
-      return;
-    }
-  }
-
-  // Store last used position.
-  if (devToolsWindow_) {
-    NSArray* subviews = [splitView_ subviews];
-    DCHECK_EQ([subviews count], 2u);
-    NSView* devToolsView = [subviews objectAtIndex:1];
-    if (dockSide_ == DEVTOOLS_DOCK_SIDE_RIGHT)
-      devToolsWindow_->SetWidth(NSWidth([devToolsView frame]));
-    else if (dockSide_ == DEVTOOLS_DOCK_SIDE_BOTTOM)
-      devToolsWindow_->SetHeight(NSHeight([devToolsView frame]));
-  }
+  bool shouldHide = devToolsWindow_ && devToolsWindow_ != newDevToolsWindow;
+  bool shouldShow = newDevToolsWindow && devToolsWindow_ != newDevToolsWindow;
 
-  if (devToolsWindow_)
-    [self hideDevToolsContainer];
+  if (shouldHide)
+    [self hideDevToolsView];
 
   devToolsWindow_ = newDevToolsWindow;
-
   if (devToolsWindow_) {
-    dockSide_ = devToolsWindow_->dock_side();
-    [self showDevToolsContainer];
+    gfx::Insets insets = devToolsWindow_->GetContentsInsets();;

[Avi (use Gerrit), 2013/12/09 16:13:36]

Too many semicolons.

[dgozman, 2013/12/10 14:51:47]

Done.

+    devToolsWindow_->web_contents()->GetView()->SetOverlayView(
+        contents->GetView(), gfx::Point(insets.left(), insets.top()));

[Avi (use Gerrit), 2013/12/09 16:13:36]

Yikes. SetOverlayView takes a flipped Y? Can you add a comment to the interface
in content, as well as a comment here?

[dgozman, 2013/12/10 14:51:47]

SetOverlayView takes the usual coordinates (top-left corner is (0;0)).
DevToolsWindow operates has the same idea, so we just directly pass left/top to
SetOverlayView.

+
+    // Swap top and bottom offsets for NSView geometry.

[Avi (use Gerrit), 2013/12/09 16:13:36]

:(

If you are calling parameters "top left" and "right bottom" then you should
actually pass in the top left and right bottom values, and do the appropriate
flipping inside the function.

If you swap them here and lie in the names of the parameters, you're only asking
for trouble.

[dgozman, 2013/12/10 14:51:47]

Ok, so the right way is to always use logical top, and only flip when passing to
Cocoa? Done.

[Avi (use Gerrit), 2013/12/10 16:32:40]

Well, if you have a parameter called "top left" then it should be the top left.
Otherwise it's really confusing.

+    [devToolsContainerView_
+        setContentsOffsetsTopLeft:NSMakeSize(insets.left(), insets.bottom())
+                      bottomRight:NSMakeSize(insets.right(), insets.top())];

[Avi (use Gerrit), 2013/12/09 16:13:36]

Oh wow, this is awkward. Cocoa's equivalent to gfx::Insets is NSEdgeInsets, but
it's 10.7 so you can't use it. You might as well use gfx::Insets for the
DevToolsContainerView and make less busy-work for yourself.

And do the flipping inside of DevToolsContainerView, not here!

[dgozman, 2013/12/10 14:51:47]

Done.

+  } else {
+    [devToolsContainerView_
+        setContentsOffsetsTopLeft:NSZeroSize

[Avi (use Gerrit), 2013/12/09 16:13:36]

Surely this fits on the previous line.

[dgozman, 2013/12/10 14:51:47]

Done.

+                      bottomRight:NSZeroSize];
   }
+
+  if (shouldShow)
+    [self showDevToolsView];
+
+  [devToolsContainerView_ adjustSubviews];
+  if (shouldHide || shouldShow)
+    [[devToolsContainerView_ window] disableScreenUpdatesUntilFlush];
 }
 
-- (void)showDevToolsContainer {
-  NSArray* subviews = [splitView_ subviews];
-  DCHECK_EQ([subviews count], 1u);
-  WebContents* devToolsContents = devToolsWindow_->web_contents();
+- (void)showDevToolsView {
   focusTracker_.reset(
-      [[FocusTracker alloc] initWithWindow:[splitView_ window]]);
+      [[FocusTracker alloc] initWithWindow:[devToolsContainerView_ window]]);
 
-  // |devToolsView| is a TabContentsViewCocoa object, whose ViewID was
+  // |devToolsView| is a WebContentsViewCocoa object, whose ViewID was

[Avi (use Gerrit), 2013/12/09 16:13:36]

:)

   // set to VIEW_ID_TAB_CONTAINER initially, so we need to change it to
   // VIEW_ID_DEV_TOOLS_DOCKED here.
-  NSView* devToolsView = devToolsContents->GetView()->GetNativeView();
+  NSView* devToolsView =
+      devToolsWindow_->web_contents()->GetView()->GetNativeView();
   view_id_util::SetID(devToolsView, VIEW_ID_DEV_TOOLS_DOCKED);
-  [splitView_ addSubview:devToolsView];
 
-  BOOL isVertical = devToolsWindow_->dock_side() == DEVTOOLS_DOCK_SIDE_RIGHT;
-  [splitView_ setVertical:isVertical];
-  [self updateDevToolsSplitPosition];
+  [devToolsContainerView_ showDevTools:devToolsView];
 }
 
-- (void)hideDevToolsContainer {
-  NSArray* subviews = [splitView_ subviews];
-  DCHECK_EQ([subviews count], 2u);
-  NSView* oldDevToolsContentsView = [subviews objectAtIndex:1];
-  [oldDevToolsContentsView removeFromSuperview];
-  [splitView_ adjustSubviews];
-  [focusTracker_ restoreFocusInWindow:[splitView_ window]];
+- (void)hideDevToolsView {
+  devToolsWindow_->web_contents()->GetView()->RemoveOverlayView();
+  [devToolsContainerView_ hideDevTools];
+  [focusTracker_ restoreFocusInWindow:[devToolsContainerView_ window]];
   focusTracker_.reset();
 }
 
-- (void)updateDevToolsSplitPosition {
-  NSArray* subviews = [splitView_ subviews];
-
-  // It seems as if |-setPosition:ofDividerAtIndex:| should do what's needed,
-  // but I can't figure out how to use it. Manually resize web and devtools.
-  // TODO(alekseys): either make setPosition:ofDividerAtIndex: work or to add a
-  // category on NSSplitView to handle manual resizing.
-  NSView* webView = [subviews objectAtIndex:0];
-  NSRect webFrame = [webView frame];
-  NSView* devToolsView = [subviews objectAtIndex:1];
-  NSRect devToolsFrame = [devToolsView frame];
-
-  BOOL noDivider = devToolsWindow_->dock_side() == DEVTOOLS_DOCK_SIDE_MINIMIZED;
-  [splitView_ setDividerHidden:noDivider];
-
-  if (devToolsWindow_->dock_side() == DEVTOOLS_DOCK_SIDE_RIGHT) {
-    CGFloat size = devToolsWindow_->GetWidth(NSWidth([splitView_ frame]));
-    devToolsFrame.size.width = size;
-    webFrame.size.width =
-        NSWidth([splitView_ frame]) - ([splitView_ dividerThickness] + size);
-  } else {
-    CGFloat size =
-        devToolsWindow_->dock_side() == DEVTOOLS_DOCK_SIDE_MINIMIZED ?
-            devToolsWindow_->GetMinimizedHeight() :
-            devToolsWindow_->GetHeight(NSHeight([splitView_ frame]));
-    devToolsFrame.size.height = size;
-    webFrame.size.height =
-        NSHeight([splitView_ frame]) - ([splitView_ dividerThickness] + size);
-  }
-
-  [[splitView_ window] disableScreenUpdatesUntilFlush];
-  [webView setFrame:webFrame];
-  [devToolsView setFrame:devToolsFrame];
-
-  [splitView_ adjustSubviews];
-}
-
-// NSSplitViewDelegate protocol.
-- (BOOL)splitView:(NSSplitView *)splitView
-    shouldAdjustSizeOfSubview:(NSView *)subview {
-  // Return NO for the devTools view to indicate that it should not be resized
-  // automatically. It preserves the height set by the user and also keeps
-  // view height the same while changing tabs when one of the tabs shows infobar
-  // and others are not.
-  if ([[splitView_ subviews] indexOfObject:subview] == 1)
-    return NO;
-  return YES;
-}
-
-- (NSRect)splitView:(NSSplitView*)splitView
-      effectiveRect:(NSRect)proposedEffectiveRect
-       forDrawnRect:(NSRect)drawnRect
-   ofDividerAtIndex:(NSInteger)dividerIndex {
-  if (devToolsWindow_->dock_side() == DEVTOOLS_DOCK_SIDE_MINIMIZED) {
-    return NSZeroRect;
-  } else {
-    return proposedEffectiveRect;
-  }
-}
-
-- (CGFloat)splitView:(NSSplitView*)splitView
-    constrainMaxCoordinate:(CGFloat)proposedMax
-               ofSubviewAt:(NSInteger)dividerIndex {
-  if ([splitView_ isVertical]) {
-    return NSWidth([splitView_ frame]) - [splitView_ dividerThickness] -
-        devToolsWindow_->GetMinimumWidth();
-  } else {
-    return NSHeight([splitView_ frame]) - [splitView_ dividerThickness] -
-        devToolsWindow_->GetMinimumHeight();
-  }
-}
-
-- (CGFloat)splitView:(NSSplitView *)splitView
-    constrainSplitPosition:(CGFloat)proposedPosition
-               ofSubviewAt:(NSInteger)dividerIndex {
-  return round(proposedPosition);
-}
-
--(void)splitViewWillResizeSubviews:(NSNotification *)notification {
-  [[splitView_ window] disableScreenUpdatesUntilFlush];
-}
-
 @end