Correct iOS build for latest Xcode beta.

Correct iOS build for latest Xcode beta.

- NSObject's NS_DESIGNATED_INITIALIZER -init method requires
   an -init NS_UNAVAILABLE implementation.
- Correct nonnull API usage.
- Cast to NSInteger for comparisons with NSNotFound.
- Account for dylib -> tbd change in latest Xcode.

BUG=498581, 498825, 498841
Committed: https://crrev.com/5a862993ea3a0c7593ac58971015f8122bda7a56
Cr-Commit-Position: refs/heads/master@{#333756}

[justincohen, 2015-06-10 12:25:56]

justincohen@chromium.org changed reviewers:
+ droger@chromium.org, sdefresne@chromium.org

[justincohen, 2015-06-10 12:25:57]

Do these look correct?  I'm not sure if this is the right approach.

https://codereview.chromium.org/1171203004/diff/20001/ios/web/shell/view_cont...
File ios/web/shell/view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/20001/ios/web/shell/view_cont...
ios/web/shell/view_controller.mm:133: [NSURLCache
setSharedURLCache:[[[NSURLCache alloc] init] autorelease]];
droger@ is this correct?

[sdefresne, 2015-06-10 12:33:20]

https://codereview.chromium.org/1171203004/diff/20001/ios/net/request_tracker.mm
File ios/net/request_tracker.mm (right):

https://codereview.chromium.org/1171203004/diff/20001/ios/net/request_tracker...
ios/net/request_tracker.mm:37: DCHECK_EQ(NSNotFound, (NSInteger)[factories_
indexOfObject:factory]);
static_cast<NSInteger>(...)

https://codereview.chromium.org/1171203004/diff/20001/ios/web/browsing_data_m...
File ios/web/browsing_data_managers/crw_cookie_browsing_data_manager.h (right):

https://codereview.chromium.org/1171203004/diff/20001/ios/web/browsing_data_m...
ios/web/browsing_data_managers/crw_cookie_browsing_data_manager.h:18: -
(instancetype)init UNAVAILABLE_ATTRIBUTE;
s/UNAVAILABLE_ATTRIBUTE/NS_UNAVAILABLE/

https://codereview.chromium.org/1171203004/diff/20001/ios/web/web_state/ui/cr...
File ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/20001/ios/web/web_state/ui/cr...
ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm:75: baseURL:[[NSURL
alloc] initWithString:@""]];
Need to be autoreleased otherwise you're leaking the NSURL.

[droger, 2015-06-10 12:34:09]

https://codereview.chromium.org/1171203004/diff/20001/ios/web/shell/view_cont...
File ios/web/shell/view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/20001/ios/web/shell/view_cont...
ios/web/shell/view_controller.mm:133: [NSURLCache
setSharedURLCache:[[[NSURLCache alloc] init] autorelease]];
On 2015/06/10 12:25:57, justincohen wrote:
> droger@ is this correct?

No. We don't want a cache.
If we can't pass nil, we need to provide a custom implementation of NSURLCache
that always returns nil in |- cachedResponseForRequest:|.

https://codereview.chromium.org/1171203004/diff/20001/ios/web/web_state/ui/cr...
File ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/20001/ios/web/web_state/ui/cr...
ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm:75: baseURL:[[NSURL
alloc] initWithString:@""]];
Simpler:
[NSURL URLWithString:@""]

We need to check if pdfs still work with this change.

[justincohen, 2015-06-10 15:05:45]

PTAL.

I still need to clean up the comments on the NSNotFound, file a bug, and file a
bug for the #if 0'd test.  

I want to get clearance on the rest of the proposal first.

Thanks!

https://codereview.chromium.org/1171203004/diff/20001/ios/web/browsing_data_m...
File ios/web/browsing_data_managers/crw_cookie_browsing_data_manager.h (right):

https://codereview.chromium.org/1171203004/diff/20001/ios/web/browsing_data_m...
ios/web/browsing_data_managers/crw_cookie_browsing_data_manager.h:18: -
(instancetype)init UNAVAILABLE_ATTRIBUTE;
On 2015/06/10 12:33:20, sdefresne wrote:
> s/UNAVAILABLE_ATTRIBUTE/NS_UNAVAILABLE/

Done.

[shreyasv1, 2015-06-10 15:14:34]

shreyasv@chromium.org changed reviewers:
+ shreyasv@chromium.org

[shreyasv1, 2015-06-10 15:14:35]

https://codereview.chromium.org/1171203004/diff/40001/ios/web/shell/view_cont...
File ios/web/shell/view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/40001/ios/web/shell/view_cont...
ios/web/shell/view_controller.mm:134: [NSURLCache
setSharedURLCache:[[[EmptyNSURLCache alloc] init] autorelease]];
nit: I see that [NSURLCache setSharedURLCache:[[[EmptyNSURLCache alloc] init]
autorelease]];
is being used several times in this CL. Can we move this to a helper function?

https://codereview.chromium.org/1171203004/diff/40001/ios/web/web_state/ui/cr...
File ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/40001/ios/web/web_state/ui/cr...
ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm:75: baseURL:[[NSURL
alloc] initWithString:@""]];
This is a memory leak? autorelease or wrap in a scoped_nsobject.

https://codereview.chromium.org/1171203004/diff/40001/ios/web/web_state/ui/wk...
File ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm
(right):

https://codereview.chromium.org/1171203004/diff/40001/ios/web/web_state/ui/wk...
ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm:84:
config.processPool = nil;
you can just WKProcessPool/WKPreference/WKUserContentController alloc, init here
and the test will still test the functionality.

Another clean way would be to get a WKWebViewConfiguration from another
BrowserState and assign its internal properties to this object.

If ^ is too confusing, you can just let it be and file a bug against me.

[justincohen, 2015-06-10 15:21:11]

ptal

https://codereview.chromium.org/1171203004/diff/40001/ios/web/shell/view_cont...
File ios/web/shell/view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/40001/ios/web/shell/view_cont...
ios/web/shell/view_controller.mm:134: [NSURLCache
setSharedURLCache:[[[EmptyNSURLCache alloc] init] autorelease]];
On 2015/06/10 15:14:34, shreyasv1 wrote:
> nit: I see that [NSURLCache setSharedURLCache:[[[EmptyNSURLCache alloc] init]
> autorelease]];
> is being used several times in this CL. Can we move this to a helper function?

I'll defer to droger@ on this one.  I have no clue if this is even correct.  But
it's a fair point.

https://codereview.chromium.org/1171203004/diff/40001/ios/web/web_state/ui/cr...
File ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/40001/ios/web/web_state/ui/cr...
ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm:75: baseURL:[[NSURL
alloc] initWithString:@""]];
On 2015/06/10 15:14:34, shreyasv1 wrote:
> This is a memory leak? autorelease or wrap in a scoped_nsobject.

Done

https://codereview.chromium.org/1171203004/diff/40001/ios/web/web_state/ui/wk...
File ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm
(right):

https://codereview.chromium.org/1171203004/diff/40001/ios/web/web_state/ui/wk...
ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm:84:
config.processPool = nil;
On 2015/06/10 15:14:35, shreyasv1 wrote:
> you can just WKProcessPool/WKPreference/WKUserContentController alloc, init
here
> and the test will still test the functionality.
> 
> Another clean way would be to get a WKWebViewConfiguration from another
> BrowserState and assign its internal properties to this object.
> 
> If ^ is too confusing, you can just let it be and file a bug against me.

Just did the alloc init autorelease.  wdyt?

[droger, 2015-06-10 15:59:10]

https://codereview.chromium.org/1171203004/diff/120001/ios/net/empty_nsurlcac...
File ios/net/empty_nsurlcache.h (right):

https://codereview.chromium.org/1171203004/diff/120001/ios/net/empty_nsurlcac...
ios/net/empty_nsurlcache.h:10: @interface EmptyNSURLCache : NSURLCache
Add a comment. I suggest something like:

// Dummy NSURLCache implementation that does not cache anything.

https://codereview.chromium.org/1171203004/diff/120001/ios/net/empty_nsurlcac...
ios/net/empty_nsurlcache.h:11: - (NSCachedURLResponse*)cachedResponseForRequest:
Optional:
remove this. I don't know exactly what is the recommended style, but I think
that in most case we don't redeclare in headers the methods that are inherited.

https://codereview.chromium.org/1171203004/diff/120001/ios/web/crw_browsing_d...
File ios/web/crw_browsing_data_store.mm (right):

https://codereview.chromium.org/1171203004/diff/120001/ios/web/crw_browsing_d...
ios/web/crw_browsing_data_store.mm:48: - (instancetype)init
UNAVAILABLE_ATTRIBUTE;
NS_UNAVAILABLE

https://codereview.chromium.org/1171203004/diff/120001/ios/web/crw_browsing_d...
File ios/web/crw_browsing_data_store_unittest.mm (right):

https://codereview.chromium.org/1171203004/diff/120001/ios/web/crw_browsing_d...
ios/web/crw_browsing_data_store_unittest.mm:22: - (instancetype)init
UNAVAILABLE_ATTRIBUTE;
NS_UNAVAILABLE

https://codereview.chromium.org/1171203004/diff/120001/ios/web/web_state/ui/c...
File ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/120001/ios/web/web_state/ui/c...
ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm:75: baseURL:[[[NSURL
alloc] initWithString:@""] autorelease]];
why not [NSURL URLWithString:@""] ?

https://codereview.chromium.org/1171203004/diff/120001/ios/web/web_state/ui/w...
File ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm
(right):

https://codereview.chromium.org/1171203004/diff/120001/ios/web/web_state/ui/w...
ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm:72: //
TODO(shreyasv) Disabled for usaging of nil on latest Xcode beta.
Could we just mark the test disabled?

Or better, if we can check at compile time:

#if IOS9
#define MAYBE_ConfigurationProtection DISABLED_ConfigurationProtection
#else
#define MAYBE_ConfigurationProtection ConfigurationProtection
#endif  // IOS9

[justincohen, 2015-06-10 16:02:25]

Done, PTAL!

https://codereview.chromium.org/1171203004/diff/120001/ios/net/empty_nsurlcac...
File ios/net/empty_nsurlcache.h (right):

https://codereview.chromium.org/1171203004/diff/120001/ios/net/empty_nsurlcac...
ios/net/empty_nsurlcache.h:10: @interface EmptyNSURLCache : NSURLCache
On 2015/06/10 15:59:10, droger wrote:
> Add a comment. I suggest something like:
> 
> // Dummy NSURLCache implementation that does not cache anything.

Done.

https://codereview.chromium.org/1171203004/diff/120001/ios/net/empty_nsurlcac...
ios/net/empty_nsurlcache.h:11: - (NSCachedURLResponse*)cachedResponseForRequest:
On 2015/06/10 15:59:10, droger wrote:
> Optional:
> remove this. I don't know exactly what is the recommended style, but I think
> that in most case we don't redeclare in headers the methods that are
inherited.

Done.

https://codereview.chromium.org/1171203004/diff/120001/ios/web/crw_browsing_d...
File ios/web/crw_browsing_data_store.mm (right):

https://codereview.chromium.org/1171203004/diff/120001/ios/web/crw_browsing_d...
ios/web/crw_browsing_data_store.mm:48: - (instancetype)init
UNAVAILABLE_ATTRIBUTE;
On 2015/06/10 15:59:10, droger wrote:
> NS_UNAVAILABLE

Done.

https://codereview.chromium.org/1171203004/diff/120001/ios/web/crw_browsing_d...
File ios/web/crw_browsing_data_store_unittest.mm (right):

https://codereview.chromium.org/1171203004/diff/120001/ios/web/crw_browsing_d...
ios/web/crw_browsing_data_store_unittest.mm:22: - (instancetype)init
UNAVAILABLE_ATTRIBUTE;
On 2015/06/10 15:59:10, droger wrote:
> NS_UNAVAILABLE

Done.

https://codereview.chromium.org/1171203004/diff/120001/ios/web/web_state/ui/c...
File ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/120001/ios/web/web_state/ui/c...
ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm:75: baseURL:[[[NSURL
alloc] initWithString:@""] autorelease]];
On 2015/06/10 15:59:10, droger wrote:
> why not [NSURL URLWithString:@""] ?

Done.

https://codereview.chromium.org/1171203004/diff/120001/ios/web/web_state/ui/w...
File ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm
(right):

https://codereview.chromium.org/1171203004/diff/120001/ios/web/web_state/ui/w...
ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm:72: //
TODO(shreyasv) Disabled for usaging of nil on latest Xcode beta.
On 2015/06/10 15:59:10, droger wrote:
> Could we just mark the test disabled?
> 
> Or better, if we can check at compile time:
> 
> #if IOS9
> #define MAYBE_ConfigurationProtection DISABLED_ConfigurationProtection
> #else
> #define MAYBE_ConfigurationProtection ConfigurationProtection
> #endif  // IOS9

This doesn't compile on the latest beta at all.  Leave as is.

[shreyasv1, 2015-06-10 16:04:48]

https://codereview.chromium.org/1171203004/diff/120001/ios/web/web_state/ui/w...
File ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm
(right):

https://codereview.chromium.org/1171203004/diff/120001/ios/web/web_state/ui/w...
ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm:72: //
TODO(shreyasv) Disabled for usaging of nil on latest Xcode beta.
On 2015/06/10 16:02:25, justincohen wrote:
> On 2015/06/10 15:59:10, droger wrote:
> > Could we just mark the test disabled?
> > 
> > Or better, if we can check at compile time:
> > 
> > #if IOS9
> > #define MAYBE_ConfigurationProtection DISABLED_ConfigurationProtection
> > #else
> > #define MAYBE_ConfigurationProtection ConfigurationProtection
> > #endif  // IOS9
> 
> This doesn't compile on the latest beta at all.  Leave as is.

2 points:
1) With the changes in this patch, the compile problem should not be present?
2) I am ok with disabling, @droger just iOS9 disablement would not work since
the test would fail before that as well. It's a small fix so disabling in the
interim is ok.

[droger, 2015-06-10 16:05:55]

Thanks! lgtm

[sdefresne, 2015-06-10 16:15:56]

https://codereview.chromium.org/1171203004/diff/140001/base/ios/crb_protocol_...
File base/ios/crb_protocol_observers_unittest.mm (right):

https://codereview.chromium.org/1171203004/diff/140001/base/ios/crb_protocol_...
base/ios/crb_protocol_observers_unittest.mm:6: #include
"base/ios/weak_nsobject.h"
#include "base/logging.h" so that you don't break if it stops being transitively
included!

https://codereview.chromium.org/1171203004/diff/140001/ios/crnet/crnet_enviro...
File ios/crnet/crnet_environment.mm (right):

https://codereview.chromium.org/1171203004/diff/140001/ios/crnet/crnet_enviro...
ios/crnet/crnet_environment.mm:304: [NSURLCache
setSharedURLCache:[[[EmptyNSURLCache alloc] init] autorelease]];
[EmptyNSURLCache emptyNSURLCache]

https://codereview.chromium.org/1171203004/diff/140001/ios/net/empty_nsurlcac...
File ios/net/empty_nsurlcache.h (right):

https://codereview.chromium.org/1171203004/diff/140001/ios/net/empty_nsurlcac...
ios/net/empty_nsurlcache.h:13: @end
+ (instancetype)emptyNSURLCache;

https://codereview.chromium.org/1171203004/diff/140001/ios/net/empty_nsurlcac...
File ios/net/empty_nsurlcache.mm (right):

https://codereview.chromium.org/1171203004/diff/140001/ios/net/empty_nsurlcac...
ios/net/empty_nsurlcache.mm:13: 
+ (instancetype)emptyNSURLCache {
  return [[[EmptyNSURLCache alloc] init] autoreleased];
}

And then replace all "[[[EmptyNSURLCache alloc] init] autoreleased]" by
"[EmptyNSURLCache emptyNSURLCache]".

https://codereview.chromium.org/1171203004/diff/140001/ios/web/web_state/ui/c...
File ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/140001/ios/web/web_state/ui/c...
ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm:75: baseURL:[[[NSURL
alloc] initWithString:@""] autorelease]];
[NSURL URLWithString:@""]

https://codereview.chromium.org/1171203004/diff/140001/ios/web/web_state/ui/w...
File ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm
(right):

https://codereview.chromium.org/1171203004/diff/140001/ios/web/web_state/ui/w...
ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm:72: //
TODO(shreyasv) Disabled for usaging of nil on latest Xcode beta.
s/usaging/using/

https://codereview.chromium.org/1171203004/diff/140001/testing/iossim/iossim.mm
File testing/iossim/iossim.mm (right):

https://codereview.chromium.org/1171203004/diff/140001/testing/iossim/iossim....
testing/iossim/iossim.mm:32: // (crbug.com/385030).
Since all builders are on Xcode 6, can we fix this?

https://codereview.chromium.org/1171203004/diff/140001/third_party/class-dump...
File third_party/class-dump/class-dump.gyp (right):

https://codereview.chromium.org/1171203004/diff/140001/third_party/class-dump...
third_party/class-dump/class-dump.gyp:176: 'xcode_version': '<!(xcodebuild
-version | grep Xcode | awk \'{print $2}\')',
xcodebuild -version | awk '/Xcode/ {print $2}'

[rohitrao (ping after 24h), 2015-06-10 16:18:21]

rohitrao@chromium.org changed reviewers:
+ rohitrao@chromium.org

[rohitrao (ping after 24h), 2015-06-10 16:18:22]

I'll follow up with erikchen about UNAVAILABLE_ATTRIBUTE, but NS_UNAVAILABLE is
fine for now.

I don't understand the dylib vs tbd thing.  Was that an intentional change?  Is
there any documentation about it?  It seems weird that we would need this kind
of gyp change.

https://codereview.chromium.org/1171203004/diff/160001/ios/web/crw_browsing_d...
File ios/web/crw_browsing_data_store.mm (right):

https://codereview.chromium.org/1171203004/diff/160001/ios/web/crw_browsing_d...
ios/web/crw_browsing_data_store.mm:48: - (instancetype)init NS_UNAVAILABLE;
Should this be in the .h file?

https://codereview.chromium.org/1171203004/diff/160001/ios/web/shell/view_con...
File ios/web/shell/view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/160001/ios/web/shell/view_con...
ios/web/shell/view_controller.mm:134: [NSURLCache
setSharedURLCache:[[[EmptyNSURLCache alloc] init] autorelease]];
Worth making an +emptyCache convenience method on EmptyNSURLCache, so we don't
need to alloc/init/autorelease everywhere?

https://codereview.chromium.org/1171203004/diff/160001/ios/web/web_state/ui/w...
File ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm
(right):

https://codereview.chromium.org/1171203004/diff/160001/ios/web/web_state/ui/w...
ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm:71: #if 0
We can check the current SDK version instead of #if 0, right?

[rohitrao (ping after 24h), 2015-06-10 16:19:36]

Oh, I would also prefer that the unavailable -init method come directly after
the designated initializer, not before.  This is because init is uninteresting,
so we should put the interesting stuff first.  I don't know if other people feel
differently.

[sdefresne, 2015-06-10 16:21:58]

On 2015/06/10 16:19:36, rohitrao (OOO until 6-22) wrote:
> Oh, I would also prefer that the unavailable -init method come directly after
> the designated initializer, not before.  This is because init is
uninteresting,
> so we should put the interesting stuff first.  I don't know if other people
feel
> differently.

I feel the same, unavailable -init should come last.

[justincohen, 2015-06-10 16:33:37]

Moved init to below designed initializer.

SDK no longer includes dylib, only .tbd files.  I haven't seen any documentation
on this.

https://codereview.chromium.org/1171203004/diff/140001/base/ios/crb_protocol_...
File base/ios/crb_protocol_observers_unittest.mm (right):

https://codereview.chromium.org/1171203004/diff/140001/base/ios/crb_protocol_...
base/ios/crb_protocol_observers_unittest.mm:6: #include
"base/ios/weak_nsobject.h"
On 2015/06/10 16:15:55, sdefresne wrote:
> #include "base/logging.h" so that you don't break if it stops being
transitively
> included!

Done.

https://codereview.chromium.org/1171203004/diff/140001/ios/crnet/crnet_enviro...
File ios/crnet/crnet_environment.mm (right):

https://codereview.chromium.org/1171203004/diff/140001/ios/crnet/crnet_enviro...
ios/crnet/crnet_environment.mm:304: [NSURLCache
setSharedURLCache:[[[EmptyNSURLCache alloc] init] autorelease]];
On 2015/06/10 16:15:55, sdefresne wrote:
> [EmptyNSURLCache emptyNSURLCache]

Done.

https://codereview.chromium.org/1171203004/diff/140001/ios/net/empty_nsurlcac...
File ios/net/empty_nsurlcache.h (right):

https://codereview.chromium.org/1171203004/diff/140001/ios/net/empty_nsurlcac...
ios/net/empty_nsurlcache.h:13: @end
On 2015/06/10 16:15:55, sdefresne wrote:
> + (instancetype)emptyNSURLCache;

Done.

https://codereview.chromium.org/1171203004/diff/140001/ios/net/empty_nsurlcac...
File ios/net/empty_nsurlcache.mm (right):

https://codereview.chromium.org/1171203004/diff/140001/ios/net/empty_nsurlcac...
ios/net/empty_nsurlcache.mm:13: 
On 2015/06/10 16:15:55, sdefresne wrote:
> + (instancetype)emptyNSURLCache {
>   return [[[EmptyNSURLCache alloc] init] autoreleased];
> }
> 
> And then replace all "[[[EmptyNSURLCache alloc] init] autoreleased]" by
> "[EmptyNSURLCache emptyNSURLCache]".

Done.

https://codereview.chromium.org/1171203004/diff/140001/ios/web/web_state/ui/c...
File ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/140001/ios/web/web_state/ui/c...
ios/web/web_state/ui/crw_ui_simple_web_view_controller.mm:75: baseURL:[[[NSURL
alloc] initWithString:@""] autorelease]];
On 2015/06/10 16:15:56, sdefresne wrote:
> [NSURL URLWithString:@""]

Done.

https://codereview.chromium.org/1171203004/diff/140001/ios/web/web_state/ui/w...
File ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm
(right):

https://codereview.chromium.org/1171203004/diff/140001/ios/web/web_state/ui/w...
ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm:72: //
TODO(shreyasv) Disabled for usaging of nil on latest Xcode beta.
On 2015/06/10 16:15:56, sdefresne wrote:
> s/usaging/using/

Done.

https://codereview.chromium.org/1171203004/diff/140001/testing/iossim/iossim.mm
File testing/iossim/iossim.mm (right):

https://codereview.chromium.org/1171203004/diff/140001/testing/iossim/iossim....
testing/iossim/iossim.mm:32: // (crbug.com/385030).
On 2015/06/10 16:15:56, sdefresne wrote:
> Since all builders are on Xcode 6, can we fix this?

I'd like to save that in a followup CL.  I'll file a bug.

https://codereview.chromium.org/1171203004/diff/140001/third_party/class-dump...
File third_party/class-dump/class-dump.gyp (right):

https://codereview.chromium.org/1171203004/diff/140001/third_party/class-dump...
third_party/class-dump/class-dump.gyp:176: 'xcode_version': '<!(xcodebuild
-version | grep Xcode | awk \'{print $2}\')',
On 2015/06/10 16:15:56, sdefresne wrote:
> xcodebuild -version | awk '/Xcode/ {print $2}'

Done.

https://codereview.chromium.org/1171203004/diff/160001/ios/web/crw_browsing_d...
File ios/web/crw_browsing_data_store.mm (right):

https://codereview.chromium.org/1171203004/diff/160001/ios/web/crw_browsing_d...
ios/web/crw_browsing_data_store.mm:48: - (instancetype)init NS_UNAVAILABLE;
On 2015/06/10 16:18:22, rohitrao (OOO until 6-22) wrote:
> Should this be in the .h file?

Why?  It doesn't need to be?

https://codereview.chromium.org/1171203004/diff/160001/ios/web/shell/view_con...
File ios/web/shell/view_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/160001/ios/web/shell/view_con...
ios/web/shell/view_controller.mm:134: [NSURLCache
setSharedURLCache:[[[EmptyNSURLCache alloc] init] autorelease]];
On 2015/06/10 16:18:22, rohitrao (OOO until 6-22) wrote:
> Worth making an +emptyCache convenience method on EmptyNSURLCache, so we don't
> need to alloc/init/autorelease everywhere?

Done.

https://codereview.chromium.org/1171203004/diff/160001/ios/web/web_state/ui/w...
File ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm
(right):

https://codereview.chromium.org/1171203004/diff/160001/ios/web/web_state/ui/w...
ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm:71: #if 0
On 2015/06/10 16:18:22, rohitrao (OOO until 6-22) wrote:
> We can check the current SDK version instead of #if 0, right?

shreyasv is OK with fixing it shortly.  It's a small fix.

[justincohen, 2015-06-10 16:33:57]

PTAL!

[sdefresne, 2015-06-10 16:57:28]

lgtm

https://codereview.chromium.org/1171203004/diff/200001/ios/chrome/browser/inf...
File ios/chrome/browser/infobars/infobar_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/200001/ios/chrome/browser/inf...
ios/chrome/browser/infobars/infobar_controller.mm:19: - (instancetype)init {
nit: I don't really mind, but we were asked to try to keep implementation and
declaration in the same order, so maybe move below the other initializer (here
and everywhere else too).

https://codereview.chromium.org/1171203004/diff/200001/ios/web/web_state/ui/c...
File ios/web/web_state/ui/crw_web_controller_unittest.mm (right):

https://codereview.chromium.org/1171203004/diff/200001/ios/web/web_state/ui/c...
ios/web/web_state/ui/crw_web_controller_unittest.mm:843:
webView:static_cast<WKWebView*>(
nit: this is ugly, can you use temporary variables for the two casted values?

[justincohen, 2015-06-10 17:06:48]

https://codereview.chromium.org/1171203004/diff/200001/ios/chrome/browser/inf...
File ios/chrome/browser/infobars/infobar_controller.mm (right):

https://codereview.chromium.org/1171203004/diff/200001/ios/chrome/browser/inf...
ios/chrome/browser/infobars/infobar_controller.mm:19: - (instancetype)init {
On 2015/06/10 16:57:28, sdefresne wrote:
> nit: I don't really mind, but we were asked to try to keep implementation and
> declaration in the same order, so maybe move below the other initializer (here
> and everywhere else too).

Done.

https://codereview.chromium.org/1171203004/diff/200001/ios/web/web_state/ui/c...
File ios/web/web_state/ui/crw_web_controller_unittest.mm (right):

https://codereview.chromium.org/1171203004/diff/200001/ios/web/web_state/ui/c...
ios/web/web_state/ui/crw_web_controller_unittest.mm:843:
webView:static_cast<WKWebView*>(
On 2015/06/10 16:57:28, sdefresne wrote:
> nit: this is ugly, can you use temporary variables for the two casted values?

Done.

[justincohen, 2015-06-10 17:07:00]

The CQ bit was checked by justincohen@chromium.org

[justincohen, 2015-06-10 17:07:01]

The patchset sent to the CQ was uploaded after l-g-t-m from droger@chromium.org,
sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/1171203004/#ps220001
(title: "Nits")

[commit-bot: I haz the power, 2015-06-10 17:07:36]

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

[commit-bot: I haz the power, 2015-06-10 17:38:27]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2015-06-10 17:39:19]

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