Initialize CRIWVWebView with Configuration object instead of browserState.

Initialize CRIWVWebView with Configuration object instead of
browserState.

BUG=689231
Review-Url: https://codereview.chromium.org/2679003002
Cr-Commit-Position: refs/heads/master@{#449149}
Committed: https://chromium.googlesource.com/chromium/src/+/9fc76d62aac6ec3b72751abdcbca613a3da7b81e

[michaeldo, 2017-02-06 22:35:56]

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

[Eugene But (OOO till 7-30), 2017-02-06 23:31:56]

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv.mm
File ios/web_view/internal/criwv.mm (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv.mm:44: +
(CRIWVWebView*)webViewWithFrame:(CGRect)frame {
I assume we going to remove this method. Right?

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
File ios/web_view/internal/criwv_browser_state.h (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_browser_state.h:27: CRIWVBrowserState(bool
off_the_record);
explicit

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
File ios/web_view/internal/criwv_web_client.h (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_web_client.h:31: CRIWVBrowserState* browser_state()
const;
This should be owned by WebsiteDataStore. It's ok to do the change in a separate
CL.

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
File ios/web_view/internal/criwv_web_view.mm (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_web_view.mm:20: #import
"ios/web_view/internal/criwv_website_data_store_internal.h"
nit: Import "ios/web_view/internal/criwv_website_data_store.h" as well

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_web_view.mm:48: _configuration = configuration;
[configuration copy]

It's ok to make the change in a separate CL.

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
File ios/web_view/internal/criwv_web_view_configuration.mm (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_web_view_configuration.mm:13: @synthesize
websiteDataStore = _websiteDataStore;
Please create default dataStore in init.

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
File ios/web_view/internal/criwv_web_view_internal.h (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_web_view_internal.h:16:
configuration:(CRIWVWebViewConfiguration*)configuration
This should be a public WebView method now.

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/public/criwv_w...
File ios/web_view/public/criwv_web_view_configuration.h (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/public/criwv_w...
ios/web_view/public/criwv_web_view_configuration.h:15: // Data store defining
persistance of webstie data.
s/webstie/website

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/public/criwv_w...
ios/web_view/public/criwv_web_view_configuration.h:15: // Data store defining
persistance of webstie data.
Please document which data store is default.

[michaeldo, 2017-02-06 23:53:50]

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv.mm
File ios/web_view/internal/criwv.mm (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv.mm:44: +
(CRIWVWebView*)webViewWithFrame:(CGRect)frame {
On 2017/02/06 23:31:56, Eugene But wrote:
> I assume we going to remove this method. Right?

Yes, we will, I just didn't want this CL to get too large.

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
File ios/web_view/internal/criwv_browser_state.h (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_browser_state.h:27: CRIWVBrowserState(bool
off_the_record);
On 2017/02/06 23:31:56, Eugene But wrote:
> explicit

Done.

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
File ios/web_view/internal/criwv_web_client.h (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_web_client.h:31: CRIWVBrowserState* browser_state()
const;
On 2017/02/06 23:31:56, Eugene But wrote:
> This should be owned by WebsiteDataStore. It's ok to do the change in a
separate
> CL.

Acknowledged.

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
File ios/web_view/internal/criwv_web_view.mm (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_web_view.mm:20: #import
"ios/web_view/internal/criwv_website_data_store_internal.h"
On 2017/02/06 23:31:56, Eugene But wrote:
> nit: Import "ios/web_view/internal/criwv_website_data_store.h" as well

Done.

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_web_view.mm:48: _configuration = configuration;
On 2017/02/06 23:31:56, Eugene But wrote:
> [configuration copy]
> 
> It's ok to make the change in a separate CL.

Done. Needed to upload new patch anyway.

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
File ios/web_view/internal/criwv_web_view_configuration.mm (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_web_view_configuration.mm:13: @synthesize
websiteDataStore = _websiteDataStore;
On 2017/02/06 23:31:56, Eugene But wrote:
> Please create default dataStore in init.

Done.

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
File ios/web_view/internal/criwv_web_view_internal.h (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/internal/criwv...
ios/web_view/internal/criwv_web_view_internal.h:16:
configuration:(CRIWVWebViewConfiguration*)configuration
On 2017/02/06 23:31:56, Eugene But wrote:
> This should be a public WebView method now. 

That's right, thanks for the catch.

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/public/criwv_w...
File ios/web_view/public/criwv_web_view_configuration.h (right):

https://codereview.chromium.org/2679003002/diff/1/ios/web_view/public/criwv_w...
ios/web_view/public/criwv_web_view_configuration.h:15: // Data store defining
persistance of webstie data.
On 2017/02/06 23:31:56, Eugene But wrote:
> s/webstie/website

Done.

[Eugene But (OOO till 7-30), 2017-02-07 01:27:46]

lgtm

https://codereview.chromium.org/2679003002/diff/20001/ios/web_view/internal/c...
File ios/web_view/internal/criwv_web_view_configuration.mm (right):

https://codereview.chromium.org/2679003002/diff/20001/ios/web_view/internal/c...
ios/web_view/internal/criwv_web_view_configuration.mm:20: self.websiteDataStore
= [CRIWVWebsiteDataStore defaultDataStore];
s/self.websiteDataStore/_websiteDataStore

https://google.github.io/styleguide/objcguide.xml#Avoid_Accessors_During_init...

https://codereview.chromium.org/2679003002/diff/20001/ios/web_view/public/cri...
File ios/web_view/public/criwv_web_view_configuration.h (right):

https://codereview.chromium.org/2679003002/diff/20001/ios/web_view/public/cri...
ios/web_view/public/criwv_web_view_configuration.h:13: @interface
CRIWVWebViewConfiguration : NSObject
This should conform to NSCopying. Otherwise calling copy will crash the app

[michaeldo, 2017-02-07 17:04:22]

Description was changed from

==========
Initialize CRIWVWebView with Configuration object instead of
browserState.

BUG=689231
==========

to

==========
Initialize CRIWVWebView with Configuration object instead of
browserState.

BUG=689231
==========

[michaeldo, 2017-02-08 20:40:48]

https://codereview.chromium.org/2679003002/diff/20001/ios/web_view/internal/c...
File ios/web_view/internal/criwv_web_view_configuration.mm (right):

https://codereview.chromium.org/2679003002/diff/20001/ios/web_view/internal/c...
ios/web_view/internal/criwv_web_view_configuration.mm:20: self.websiteDataStore
= [CRIWVWebsiteDataStore defaultDataStore];
On 2017/02/07 01:27:46, Eugene But wrote:
> s/self.websiteDataStore/_websiteDataStore
> 
>
https://google.github.io/styleguide/objcguide.xml#Avoid_Accessors_During_init...

Done.

https://codereview.chromium.org/2679003002/diff/20001/ios/web_view/public/cri...
File ios/web_view/public/criwv_web_view_configuration.h (right):

https://codereview.chromium.org/2679003002/diff/20001/ios/web_view/public/cri...
ios/web_view/public/criwv_web_view_configuration.h:13: @interface
CRIWVWebViewConfiguration : NSObject
On 2017/02/07 01:27:46, Eugene But wrote:
> This should conform to NSCopying. Otherwise calling copy will crash the app

Done.

[michaeldo, 2017-02-08 22:40:37]

The CQ bit was checked by michaeldo@chromium.org

[michaeldo, 2017-02-08 22:40:37]

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

[commit-bot: I haz the power, 2017-02-08 22:41:09]

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-02-09 00:05:22]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486593637470140,
"parent_rev": "93a6ee79f37afd04984937c279c3b6b87864e303", "commit_rev":
"9fc76d62aac6ec3b72751abdcbca613a3da7b81e"}

[commit-bot: I haz the power, 2017-02-09 00:05:58]

Description was changed from

==========
Initialize CRIWVWebView with Configuration object instead of
browserState.

BUG=689231
==========

to

==========
Initialize CRIWVWebView with Configuration object instead of
browserState.

BUG=689231

Review-Url: https://codereview.chromium.org/2679003002
Cr-Commit-Position: refs/heads/master@{#449149}
Committed:
https://chromium.googlesource.com/chromium/src/+/9fc76d62aac6ec3b72751abdcbca...
==========

[commit-bot: I haz the power, 2017-02-09 00:05:59]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/9fc76d62aac6ec3b72751abdcbca...