[Clean Skeleton] Fix menu dismiss and positions.

[Clean Skeleton] Fix menu dismiss and positions.

This CL cleans up the menu implementation.

First, it removes the burden of dismissing the menu when non-menu
screen areas are tapped from the menu view controller. The menu
presentation controller does this, having been handed a ToolbarCommands
object to use for this.

Second, TabContainerViewController implements ZoomTransitionDelegate,
passing the call into the toolbar view controller if it can. It also
uses that information to implement MenuPresentationDelegate such that
the menu is positioned with one corner over the zoom location used for
the menu.

BUG=

Review-Url: https://codereview.chromium.org/2603783002
Cr-Commit-Position: refs/heads/master@{#443561}
Committed: https://chromium.googlesource.com/chromium/src/+/e244e9ad13ddb05af872f74ec1044f5f1af42d54

[marq (ping after 24h), 2016-12-28 11:36:43]

marq@chromium.org changed reviewers:
+ edchin@chromium.org, lpromero@chromium.org

[lpromero, 2017-01-05 12:39:34]

Description was changed from

==========
[Clean Skeleton] Fix menu dismiss and positions.

This CL cleans up the menu implementation.

First, it removes the burden of disimissing the menu when non-menu
screen areas are tapped from the menu view controller. The menu
presentation controller does this, having been handed a ToolbarCommands
object to use for this.

Second, TabContainerViewController implements ZoomTransitionDelegate,
passing the call into the toolbar view controller if it can. It also
uses that information to implement MenuPresentationDelegate such that
the menu is positioned with one corner over the zoom location used for
the menu.

BUG=
==========

to

==========
[Clean Skeleton] Fix menu dismiss and positions.

This CL cleans up the menu implementation.

First, it removes the burden of dismissing the menu when non-menu
screen areas are tapped from the menu view controller. The menu
presentation controller does this, having been handed a ToolbarCommands
object to use for this.

Second, TabContainerViewController implements ZoomTransitionDelegate,
passing the call into the toolbar view controller if it can. It also
uses that information to implement MenuPresentationDelegate such that
the menu is positioned with one corner over the zoom location used for
the menu.

BUG=
==========

[lpromero, 2017-01-05 14:27:05]

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/presenters/menu_presentation_controller.h
(right):

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/presenters/menu_presentation_controller.h:22: // If
the presenting view controller confroms to the MenuPresentationDelegate
s/confroms/conforms

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/presenters/menu_presentation_controller.mm
(right):

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/presenters/menu_presentation_controller.mm:23:
@property(nonatomic) UITapGestureRecognizer* dismissRecognizer;
I'd explicitly annotate "strong".

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/presenters/menu_presentation_controller.mm:46:
self.containerView.bounds.size.height - menuSize.height / 2.0);
Shouldn’t it be (a - b)/2 instead of a - b/2?

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tab/tab_container_view_controller.mm (right):

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab/tab_container_view_controller.mm:141: //
deterimined by comparing frames, and thus is RTL-independent.
*determined

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tools/menu_view_controller.mm (right):

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tools/menu_view_controller.mm:66: self.view =
[[UIView alloc] initWithFrame:frame];
How does this work in Showcase, where the view controller is pushed on the
stack? If this view controller decides forcefully the size of its view, does it
play well in these situations?

[marq (ping after 24h), 2017-01-09 12:22:33]

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tools/menu_view_controller.mm (right):

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tools/menu_view_controller.mm:66: self.view =
[[UIView alloc] initWithFrame:frame];
On 2017/01/05 14:27:04, lpromero wrote:
> How does this work in Showcase, where the view controller is pushed on the
> stack? If this view controller decides forcefully the size of its view, does
it
> play well in these situations?

It works fine, because containing view controllers (such as a
UINavigationController) typically resize the view of a contained view
controller. The best way to think of it (to me) is that a view controller's
initial view frame is its *preferred* size, which a container or presenter is
free to ignore. In this case, the menu presenter honors the menu VC's
preference, but by default Showcase's nav controller doesn't. A view controller
should behave correctly (as much as possible) regardless of what its frame
actually is set to, although generally only containers or presenters should be
accessing a view controller's view frame.

[lpromero, 2017-01-09 12:52:38]

lgtm

[marq (ping after 24h), 2017-01-09 18:05:00]

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/presenters/menu_presentation_controller.h
(right):

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/presenters/menu_presentation_controller.h:22: // If
the presenting view controller confroms to the MenuPresentationDelegate
On 2017/01/05 14:27:04, lpromero wrote:
> s/confroms/conforms

Done.

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/presenters/menu_presentation_controller.mm
(right):

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/presenters/menu_presentation_controller.mm:23:
@property(nonatomic) UITapGestureRecognizer* dismissRecognizer;
On 2017/01/05 14:27:04, lpromero wrote:
> I'd explicitly annotate "strong".

Done.

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/presenters/menu_presentation_controller.mm:46:
self.containerView.bounds.size.height - menuSize.height / 2.0);
On 2017/01/05 14:27:04, lpromero wrote:
> Shouldn’t it be (a - b)/2 instead of a - b/2?

Yes, I am mostly innumerate.

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tab/tab_container_view_controller.mm (right):

https://codereview.chromium.org/2603783002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab/tab_container_view_controller.mm:141: //
deterimined by comparing frames, and thus is RTL-independent.
On 2017/01/05 14:27:04, lpromero wrote:
> *determined

Done.

[marq (ping after 24h), 2017-01-13 09:23:17]

The CQ bit was checked by marq@chromium.org

[marq (ping after 24h), 2017-01-13 09:23:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
lpromero@chromium.org
Link to the patchset: https://codereview.chromium.org/2603783002/#ps40001
(title: "Fix comments.")

[commit-bot: I haz the power, 2017-01-13 09:23:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-13 09:29:08]

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

[commit-bot: I haz the power, 2017-01-13 09:29:09]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[marq (ping after 24h), 2017-01-13 11:33:00]

Fix BUILD.gn.

[marq (ping after 24h), 2017-01-13 11:33:01]

The CQ bit was checked by marq@chromium.org

[marq (ping after 24h), 2017-01-13 11:33:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
lpromero@chromium.org
Link to the patchset: https://codereview.chromium.org/2603783002/#ps60001
(title: "Fix BUILD.gn.")

[commit-bot: I haz the power, 2017-01-13 11:33:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-13 11:40:58]

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

[commit-bot: I haz the power, 2017-01-13 11:40:59]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[marq (ping after 24h), 2017-01-13 11:53:30]

Fix strip include.

[marq (ping after 24h), 2017-01-13 11:53:31]

The CQ bit was checked by marq@chromium.org

[marq (ping after 24h), 2017-01-13 11:53:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
lpromero@chromium.org
Link to the patchset: https://codereview.chromium.org/2603783002/#ps80001
(title: "Fix strip include.")

[commit-bot: I haz the power, 2017-01-13 11:53:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-13 13:46:43]

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

[commit-bot: I haz the power, 2017-01-13 13:46:44]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[marq (ping after 24h), 2017-01-13 14:02:56]

The CQ bit was checked by marq@chromium.org

[commit-bot: I haz the power, 2017-01-13 14:03:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-13 15:19:42]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1484316176455320,
"parent_rev": "f4fecb63fe8f3bc859b14a8d12d4de84d0b9e7c8", "commit_rev":
"e244e9ad13ddb05af872f74ec1044f5f1af42d54"}

[commit-bot: I haz the power, 2017-01-13 15:20:12]

Description was changed from

==========
[Clean Skeleton] Fix menu dismiss and positions.

This CL cleans up the menu implementation.

First, it removes the burden of dismissing the menu when non-menu
screen areas are tapped from the menu view controller. The menu
presentation controller does this, having been handed a ToolbarCommands
object to use for this.

Second, TabContainerViewController implements ZoomTransitionDelegate,
passing the call into the toolbar view controller if it can. It also
uses that information to implement MenuPresentationDelegate such that
the menu is positioned with one corner over the zoom location used for
the menu.

BUG=
==========

to

==========
[Clean Skeleton] Fix menu dismiss and positions.

This CL cleans up the menu implementation.

First, it removes the burden of dismissing the menu when non-menu
screen areas are tapped from the menu view controller. The menu
presentation controller does this, having been handed a ToolbarCommands
object to use for this.

Second, TabContainerViewController implements ZoomTransitionDelegate,
passing the call into the toolbar view controller if it can. It also
uses that information to implement MenuPresentationDelegate such that
the menu is positioned with one corner over the zoom location used for
the menu.

BUG=

Review-Url: https://codereview.chromium.org/2603783002
Cr-Commit-Position: refs/heads/master@{#443561}
Committed:
https://chromium.googlesource.com/chromium/src/+/e244e9ad13ddb05af872f74ec104...
==========

[commit-bot: I haz the power, 2017-01-13 15:20:13]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/e244e9ad13ddb05af872f74ec104...