Use UIAlertController on iOS8 for Context Menu.

Use UIAlertController on iOS8 for Context Menu.

Implemented CRUContextMenuController as a bridge backed up by
UIActionSheet on iOS7 and UIAlertController on iOS8.

BUG=410299, 437781
TEST=
Context menu works on iPhone/iPad with iOS7/iOS8 for:
- web links and images
- bookmarks
- recently visited items on NTP

Committed: https://crrev.com/4b536b914f0e62a98058d01c61608a89bfb64de6
Cr-Commit-Position: refs/heads/master@{#327109}

[Eugene But (OOO till 7-30), 2015-04-23 18:13:49]

eugenebut@chromium.org changed reviewers:
+ lliabraa@chromium.org

[lliabraa, 2015-04-23 20:06:39]

lliabraa@chromium.org changed reviewers:
+ sdefresne@chromium.org

[lliabraa, 2015-04-23 20:06:40]

lgtm

+sdefresne for OWNERS

https://codereview.chromium.org/1103743002/diff/1/ui/base/ios/cru_context_men...
File ui/base/ios/cru_context_menu_controller.mm (right):

https://codereview.chromium.org/1103743002/diff/1/ui/base/ios/cru_context_men...
ui/base/ios/cru_context_menu_controller.mm:21: // Returns the screen's height in
pixels.
s/pixels/points/

[Eugene But (OOO till 7-30), 2015-04-23 20:59:52]

https://codereview.chromium.org/1103743002/diff/1/ui/base/ios/cru_context_men...
File ui/base/ios/cru_context_menu_controller.mm (right):

https://codereview.chromium.org/1103743002/diff/1/ui/base/ios/cru_context_men...
ui/base/ios/cru_context_menu_controller.mm:21: // Returns the screen's height in
pixels.
On 2015/04/23 20:06:40, lliabraa wrote:
> s/pixels/points/

Done.

[sdefresne, 2015-04-24 08:09:34]

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
File ui/base/ios/cru_context_menu_controller.mm (right):

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:92: DCHECK([view window] || [view
isKindOfClass:[UIWindow class]]);
Either DCHECK or do early return, but not both.

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:101: #pragma mark iOS 7
nit: merge both lines into

#pragma mark - iOS 7

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:119: // are added to the menu items
area. If that's the case - elide the title to
nit: s/the case - elide the/the case, elide the/

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:120: // avoid having scrollbars for
menu items.
nit: please use either "scroll bars" or "scrollbars" consistently

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:225: #pragma mark -
nit: merge both line into

#pragma mark - iOS8 and higher

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:285: weakSelf.get()->_visible = YES;
This will crash if weakSelf is nil. So either keep a strong ref to self or use
strongSelf

base::scoped_nsobject<CRUAlertController> strongSelf([weakSelf retain]);
if (strongSelf)
  strongSelf->_visible = YES;

[Eugene But (OOO till 7-30), 2015-04-24 16:18:55]

PTAL

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
File ui/base/ios/cru_context_menu_controller.mm (right):

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:92: DCHECK([view window] || [view
isKindOfClass:[UIWindow class]]);
On 2015/04/24 08:09:33, sdefresne wrote:
> Either DCHECK or do early return, but not both.
I guess early return is safer.

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:101: #pragma mark iOS 7
On 2015/04/24 08:09:34, sdefresne wrote:
> nit: merge both lines into
> 
> #pragma mark - iOS 7

Done.

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:119: // are added to the menu items
area. If that's the case - elide the title to
On 2015/04/24 08:09:34, sdefresne wrote:
> nit: s/the case - elide the/the case, elide the/

Done.

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:120: // avoid having scrollbars for
menu items.
On 2015/04/24 08:09:34, sdefresne wrote:
> nit: please use either "scroll bars" or "scrollbars" consistently

Done.

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:225: #pragma mark -
On 2015/04/24 08:09:34, sdefresne wrote:
> nit: merge both line into
> 
> #pragma mark - iOS8 and higher

Done.

https://codereview.chromium.org/1103743002/diff/20001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:285: weakSelf.get()->_visible = YES;
On 2015/04/24 08:09:34, sdefresne wrote:
> This will crash if weakSelf is nil. So either keep a strong ref to self or use
> strongSelf
> 
> base::scoped_nsobject<CRUAlertController> strongSelf([weakSelf retain]);
> if (strongSelf)
>   strongSelf->_visible = YES;
> 
Redefined visible property as readwrite and call a method on self.

[sdefresne, 2015-04-25 11:08:17]

lgtm with nits

https://codereview.chromium.org/1103743002/diff/40001/ui/base/ios/cru_context...
File ui/base/ios/cru_context_menu_controller.mm (right):

https://codereview.chromium.org/1103743002/diff/40001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:62: // Redefinied to readwrite.
nit: s/Redefinied/Redefined/

https://codereview.chromium.org/1103743002/diff/40001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:233: // On presenting and dismissing
a dummy UIAlertController flushes a screen.
nit: // Presenting and dismissing a dummy ...

[Eugene But (OOO till 7-30), 2015-04-27 16:42:58]

Thanks for review!

https://codereview.chromium.org/1103743002/diff/40001/ui/base/ios/cru_context...
File ui/base/ios/cru_context_menu_controller.mm (right):

https://codereview.chromium.org/1103743002/diff/40001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:62: // Redefinied to readwrite.
On 2015/04/25 11:08:17, sdefresne wrote:
> nit: s/Redefinied/Redefined/

Done.

https://codereview.chromium.org/1103743002/diff/40001/ui/base/ios/cru_context...
ui/base/ios/cru_context_menu_controller.mm:233: // On presenting and dismissing
a dummy UIAlertController flushes a screen.
On 2015/04/25 11:08:16, sdefresne wrote:
> nit: // Presenting and dismissing a dummy ...

Done.

[Eugene But (OOO till 7-30), 2015-04-27 16:43:01]

The CQ bit was checked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2015-04-27 16:43:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
lliabraa@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/1103743002/#ps60001
(title: "Fixed comments typos.")

[commit-bot: I haz the power, 2015-04-27 16:43:13]

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

[commit-bot: I haz the power, 2015-04-27 16:56:33]

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

[commit-bot: I haz the power, 2015-04-27 16:56:33]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[Eugene But (OOO till 7-30), 2015-04-27 19:57:21]

The CQ bit was checked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2015-04-27 19:57:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
lliabraa@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/1103743002/#ps80001
(title: "Set visible to YES right after presenting alert controller.")

[commit-bot: I haz the power, 2015-04-27 19:58:22]

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

[commit-bot: I haz the power, 2015-04-27 20:06:48]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-04-27 20:07:48]

Patchset 5 (id:??) landed as
https://crrev.com/4b536b914f0e62a98058d01c61608a89bfb64de6
Cr-Commit-Position: refs/heads/master@{#327109}