[ios] Adds protocols for native app launching

Adds protocols for native app launching.

BUG=662573
Committed: https://crrev.com/50d00a67e659fd519e1ed60f132becd9e65b4a79
Cr-Commit-Position: refs/heads/master@{#431555}

[sczs1, 2016-11-01 23:01:36]

Patchset #1 (id:1) has been deleted

[sczs1, 2016-11-04 22:51:23]

Description was changed from

==========
Upstreaming WIP

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

to

==========
Adds protocols for native app launching

BUG= 662573
==========

[sczs1, 2016-11-04 22:51:23]

sczs@google.com changed reviewers:
+ rohitrao@chromium.org

[sczs1, 2016-11-04 22:51:41]

Description was changed from

==========
Adds protocols for native app launching

BUG= 662573
==========

to

==========
Adds protocols for native app launching.

BUG= 662573
==========

[sczs1, 2016-11-04 22:57:11]

Hi Rohit, could you PTAL.

I won't land this until the downstream classes conform to the new protocols (In
case any changes are needed).

[rohitrao (ping after 24h), 2016-11-08 12:39:18]

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
File
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h
(right):

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:5:
#ifndef IOS_INTERNAL_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_METADATA_H_
Header guard needs to be updated.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:11:
// TODO(sczs) Import native_app_infobar_delegate once rest of
Is this cruft now?  Can it be deleted?

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:81:
// from a user interface other than a Google App Launcher infobar.
Remove "Google App Launcher" from this comment.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:87:
// Returns the additionalData dictionary in GIPApp for unit testing.
This should not be in the public API.  Remove this method completely.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:90:
// TODO(sczs) Remove comments once rest of native_app_launcher lands upstream.
Can this be removed now?

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:103:
#endif  //
IOS_INTERNAL_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_METADATA_H_
Update header guard.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
File
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h
(right):

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h:5:
#ifndef
IOS_INTERNAL_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_WHITELIST_MANAGER_H_
Update header guards.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h:8:
#import <Foundation/Foundation.h>
Blank line after this import.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h:30:
// Returns an autoreleased NSArray of NativeAppMetadata objects which
Typo: which -> for which.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h:39:
#endif  //
IOS_INTERNAL_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_WHITELIST_MANAGER_H_
Update header guard.

[sczs1, 2016-11-09 01:14:52]

Addressed the feedback and made some small refactoring changes.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
File
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h
(right):

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:5:
#ifndef IOS_INTERNAL_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_METADATA_H_
On 2016/11/08 12:39:18, rohitrao wrote:
> Header guard needs to be updated.

Done.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:11:
// TODO(sczs) Import native_app_infobar_delegate once rest of
On 2016/11/08 12:39:18, rohitrao wrote:
> Is this cruft now?  Can it be deleted?

We can't forward declare enumerations that are nested in a Class. So I can't
declare the 2 methods that depend on NativeAppInfoBarDelegate until the class is
upstream. 
Only NativeAppNavigationController uses those methods, so for now that Class is
still importint GoogleNativeAppMetadata.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:81:
// from a user interface other than a Google App Launcher infobar.
On 2016/11/08 12:39:18, rohitrao wrote:
> Remove "Google App Launcher" from this comment.

Done.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:87:
// Returns the additionalData dictionary in GIPApp for unit testing.
On 2016/11/08 12:39:18, rohitrao wrote:
> This should not be in the public API.  Remove this method completely.

Done.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:103:
#endif  //
IOS_INTERNAL_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_METADATA_H_
On 2016/11/08 12:39:18, rohitrao wrote:
> Update header guard.

Done.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
File
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h
(right):

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h:5:
#ifndef
IOS_INTERNAL_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_WHITELIST_MANAGER_H_
On 2016/11/08 12:39:18, rohitrao wrote:
> Update header guards.

Done.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h:8:
#import <Foundation/Foundation.h>
On 2016/11/08 12:39:18, rohitrao wrote:
> Blank line after this import.

Done.

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h:30:
// Returns an autoreleased NSArray of NativeAppMetadata objects which
On 2016/11/08 12:39:18, rohitrao wrote:
> Typo: which -> for which.

"which" is already being used

https://codereview.chromium.org/2464963004/diff/60001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h:39:
#endif  //
IOS_INTERNAL_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_WHITELIST_MANAGER_H_
On 2016/11/08 12:39:18, rohitrao wrote:
> Update header guard.

Done.

[rohitrao (ping after 24h), 2016-11-09 02:19:22]

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
File ios/public/provider/chrome/browser/chrome_browser_provider.h (right):

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
ios/public/provider/chrome/browser/chrome_browser_provider.h:176: virtual
id<NativeAppWhitelistManager> GetNativeAppWhitelistManager() const;
Also update TestChromeBrowserProvider to override this method.  It can simply
return nil.

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
File
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h
(right):

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:30:
- (void)unsetShouldAutoOpenLinks;
Blank line above this and add a method comment.

// Resets |shouldAutoOpenLinks| to the "not set" state.

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:34:
// Stores the number of times a banner is dismissed or ignored.
Blank line above this.

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:89:
//    (NativeAppInfoBarDelegate::NativeAppControllerType)type;
Per discussion, let's add these enums to a new file in this CL.

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
File
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h
(right):

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h:39:
// Returns a block that will check for installed Apps.
Per discussion, let's have this method do the check, rather than return a block
to do it.

[sczs1, 2016-11-10 03:29:32]

Patchset #5 (id:100001) has been deleted

[sczs1, 2016-11-10 03:30:19]

HI Rohit, 

This should be ready to land could you PTAL.

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
File ios/public/provider/chrome/browser/chrome_browser_provider.h (right):

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
ios/public/provider/chrome/browser/chrome_browser_provider.h:176: virtual
id<NativeAppWhitelistManager> GetNativeAppWhitelistManager() const;
On 2016/11/09 02:19:21, rohitrao wrote:
> Also update TestChromeBrowserProvider to override this method.  It can simply
> return nil.

Done.

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
File
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h
(right):

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:30:
- (void)unsetShouldAutoOpenLinks;
On 2016/11/09 02:19:21, rohitrao wrote:
> Blank line above this and add a method comment.
> 
> // Resets |shouldAutoOpenLinks| to the "not set" state.

Done.

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:34:
// Stores the number of times a banner is dismissed or ignored.
On 2016/11/09 02:19:21, rohitrao wrote:
> Blank line above this.

Done.

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h:89:
//    (NativeAppInfoBarDelegate::NativeAppControllerType)type;
On 2016/11/09 02:19:21, rohitrao wrote:
> Per discussion, let's add these enums to a new file in this CL.

Acknowledged.

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
File
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h
(right):

https://codereview.chromium.org/2464963004/diff/80001/ios/public/provider/chr...
ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h:39:
// Returns a block that will check for installed Apps.
On 2016/11/09 02:19:21, rohitrao wrote:
> Per discussion, let's have this method do the check, rather than return a
block
> to do it.

Done.

[sczs1, 2016-11-10 03:30:28]

The CQ bit was checked by sczs@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-10 03:30:45]

Dry run: 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, 2016-11-10 04:30:31]

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

[commit-bot: I haz the power, 2016-11-10 04:30:31]

Dry run: This issue passed the CQ dry run.

[rohitrao (ping after 24h), 2016-11-11 12:17:10]

The CQ bit was checked by rohitrao@chromium.org

[rohitrao (ping after 24h), 2016-11-11 12:17:11]

lgtm

[commit-bot: I haz the power, 2016-11-11 12:17:19]

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, 2016-11-11 13:14:52]

Description was changed from

==========
Adds protocols for native app launching.

BUG= 662573
==========

to

==========
Adds protocols for native app launching.

BUG= 662573
==========

[commit-bot: I haz the power, 2016-11-11 13:14:53]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-11-11 13:17:50]

Description was changed from

==========
Adds protocols for native app launching.

BUG= 662573
==========

to

==========
Adds protocols for native app launching.

BUG= 662573

Committed: https://crrev.com/50d00a67e659fd519e1ed60f132becd9e65b4a79
Cr-Commit-Position: refs/heads/master@{#431555}
==========

[commit-bot: I haz the power, 2016-11-11 13:17:51]

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