To ensure that the location bar will not reach the minimum width.

To ensure that the location bar will not reach the minimum width.

Fix the width of location bar will be less than the minimum width, when
dragging the browserActionsContainer to the left.

Before the browserActionsContainer will translate on x-axis, sending a
notification to see whether it will be allowed. This will give the
toolbar controller a chance to check if the width of location bar will
reach the minimum by dragging the browserActionsContainer.

BUG=329842
TEST=manually tested.

Committed: https://crrev.com/e9cc4e8457ee68f6bbd621e0a2aea262343026ec
Cr-Commit-Position: refs/heads/master@{#302209}

[Malcolm, 2014-10-16 03:31:23]

malcolm.2.wang@gmail.com changed reviewers:
+ rsesek@chromium.org, thakis@chromium.org

[Malcolm, 2014-10-16 03:31:23]

Please review this CL, thanks!

[Nico, 2014-10-27 20:46:25]

thakis@chromium.org changed reviewers:
+ yoz@chromium.org

[Nico, 2014-10-27 20:46:27]

yoz, can you look at this?

[Yoyo Zhou, 2014-10-27 23:45:26]

https://codereview.chromium.org/636283004/diff/1/chrome/browser/ui/cocoa/tool...
File chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm (right):

https://codereview.chromium.org/636283004/diff/1/chrome/browser/ui/cocoa/tool...
chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm:592:
name:kBrowseActionContainerWillTranslateOnXNotification
typo: kBrowserAction...

https://codereview.chromium.org/636283004/diff/1/chrome/browser/ui/cocoa/tool...
chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm:647: CGFloat
locationBarWidth = NSWidth([locationBar_ frame]);
I'm not very familiar with this code, so please bear with me. Is there some
reason we can't use maintainMinimumLocationBarWidth?

[Malcolm, 2014-10-28 06:56:09]

Hi Yoyo, I've updated the CL and added some comments, please review it, thanks!

https://codereview.chromium.org/636283004/diff/1/chrome/browser/ui/cocoa/tool...
File chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm (right):

https://codereview.chromium.org/636283004/diff/1/chrome/browser/ui/cocoa/tool...
chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm:592:
name:kBrowseActionContainerWillTranslateOnXNotification
Oops, I lost the 'r'.
On 2014/10/27 23:45:26, Yoyo Zhou wrote:
> typo: kBrowserAction...

https://codereview.chromium.org/636283004/diff/1/chrome/browser/ui/cocoa/tool...
chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm:647: CGFloat
locationBarWidth = NSWidth([locationBar_ frame]);
The -maintainMinimumLocationBarWidth only adjusts the location bar to guarantee
the minimum size of it, not the frame of browserActionsContainerView. So if we
just call -maintainMinimumLocationBarWidth, the browserActionsContainerView will
still overlap the locationbar, although the locationbar will be minimum size.
Actually, the frame of browserActionsContainerView is changed in the internal of
it (-mouseDragged:). By setting the property "canDragLeft" here can tell the
browserActionsContainerView that whether the dragging is allowed in
-mouseDragged:. If not, the -setFrame will not executed in -mouseDragged: of
browserActionsContainerView.
On 2014/10/27 23:45:26, Yoyo Zhou wrote:
> I'm not very familiar with this code, so please bear with me. Is there some
> reason we can't use maintainMinimumLocationBarWidth?

[Yoyo Zhou, 2014-10-29 21:17:10]

LGTM

https://chromiumcodereview.appspot.com/636283004/diff/20001/chrome/browser/ui...
File chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm
(right):

https://chromiumcodereview.appspot.com/636283004/diff/20001/chrome/browser/ui...
chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm:18:
NSString* const kBrowserActionContainerWillTranslateOnXNotification =
Should this name be more consistent with the others? e.g.
kBrowserActionGrippyWillDragNotification?

[Robert Sesek, 2014-10-29 21:21:34]

https://chromiumcodereview.appspot.com/636283004/diff/20001/chrome/browser/ui...
File chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm
(right):

https://chromiumcodereview.appspot.com/636283004/diff/20001/chrome/browser/ui...
chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm:18:
NSString* const kBrowserActionContainerWillTranslateOnXNotification =
On 2014/10/29 21:17:10, Yoyo Zhou wrote:
> Should this name be more consistent with the others? e.g.
> kBrowserActionGrippyWillDragNotification?

Yes, I'd recommend that.

https://chromiumcodereview.appspot.com/636283004/diff/20001/chrome/browser/ui...
chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm:129:
[NSDictionary dictionaryWithObjectsAndKeys:
This can be simplified to @{ kTranslationWithDelta : @(dx) }

[Malcolm, 2014-10-30 10:08:05]

Hi Robert&Yoyo,

I updated the CL, following by your comments, thank you for reviewing it!

Malcolm Wang

https://chromiumcodereview.appspot.com/636283004/diff/20001/chrome/browser/ui...
File chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm
(right):

https://chromiumcodereview.appspot.com/636283004/diff/20001/chrome/browser/ui...
chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm:18:
NSString* const kBrowserActionContainerWillTranslateOnXNotification =
I will change the naming.
On 2014/10/29 21:21:33, Robert Sesek wrote:
> On 2014/10/29 21:17:10, Yoyo Zhou wrote:
> > Should this name be more consistent with the others? e.g.
> > kBrowserActionGrippyWillDragNotification?
> 
> Yes, I'd recommend that.

https://chromiumcodereview.appspot.com/636283004/diff/20001/chrome/browser/ui...
chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm:129:
[NSDictionary dictionaryWithObjectsAndKeys:
Absolutely!
On 2014/10/29 21:21:34, Robert Sesek wrote:
> This can be simplified to @{ kTranslationWithDelta : @(dx) }

[Malcolm, 2014-10-30 10:09:20]

The CQ bit was checked by malcolm.2.wang@gmail.com

[commit-bot: I haz the power, 2014-10-30 10:09:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/636283004/40001

[commit-bot: I haz the power, 2014-10-30 10:12:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-30 10:12:52]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Malcolm, 2014-10-30 11:24:18]

chromium_presubmit failed, I missed the LGTM frome OWNER. Robert, would you
please review it again, thanks! :)

[Robert Sesek, 2014-10-30 13:13:25]

LGTM

[Malcolm, 2014-10-31 01:25:08]

The CQ bit was checked by malcolm.2.wang@gmail.com

[commit-bot: I haz the power, 2014-10-31 01:26:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/636283004/40001

[commit-bot: I haz the power, 2014-10-31 01:51:38]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-31 01:52:35]

Patchset 3 (id:??) landed as
https://crrev.com/e9cc4e8457ee68f6bbd621e0a2aea262343026ec
Cr-Commit-Position: refs/heads/master@{#302209}