Update the assignment operator

Update the assignment operator

The navigation_params was added a new parameter. The parameter has to be
copied correctly in assignment.

BUG=645983
Review-Url: https://codereview.chromium.org/2759723002
Cr-Commit-Position: refs/heads/master@{#457927}
Committed: https://chromium.googlesource.com/chromium/src/+/ab03e18490552251ca0e2ee6249509071f8c5b15

[sgurun-gerrit only, 2017-03-17 18:20:47]

Description was changed from

==========
Update the assignment operator

The navigation_params was added a new parameter. The parameter has to be
copied correctly in assignment.

BUG=645983
==========

to

==========
Update the assignment operator

The navigation_params was added a new parameter. The parameter has to be
copied correctly in assignment.

BUG=645983
==========

[sgurun-gerrit only, 2017-03-17 18:20:48]

sgurun@chromium.org changed reviewers:
+ jam@chromium.org

[sgurun-gerrit only, 2017-03-17 18:20:56]

On 2017/03/17 18:20:48, sgurun wrote:
> mailto:sgurun@chromium.org changed reviewers:
> + mailto:jam@chromium.org

ptal, thanks

[jam, 2017-03-17 19:16:23]

if you run this through trybots there are also reliable failures now in
LoadDataWithBaseUrlTest#testInvalidBaseUrl
LoadDataWithBaseUrlTest#testOnPageFinishedWithInvalidBaseUrlWhenInterrupted

The fix is to remove the DCHECK I added in
base::android::ScopedJavaLocalRef<jobject> CreateJavaNavigationParams(, since
the tests check that it's possible to pass invalid URLs therel. also, to change
ConvertUTF8ToJavaString(env, url.spec());
to
ConvertUTF8ToJavaString(env, url.possibly_invalid_spec());


with that, the two new failures should pass and lgtm

https://codereview.chromium.org/2759723002/diff/1/components/navigation_inter...
File components/navigation_interception/navigation_params.cc (right):

https://codereview.chromium.org/2759723002/diff/1/components/navigation_inter...
components/navigation_interception/navigation_params.cc:36: void
NavigationParams::Assign(const NavigationParams& other) {
can you just get rid of this method and the manually defined assignment
operator, and instead use the default one? i.e. in the header:

NavigationParams(const NavigationParams& other) = default;

[sgurun-gerrit only, 2017-03-17 19:56:27]

The CQ bit was checked by sgurun@chromium.org

[sgurun-gerrit only, 2017-03-17 19:56:27]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2759723002/#ps20001
(title: "Address jam review")

[sgurun-gerrit only, 2017-03-17 19:56:51]

https://codereview.chromium.org/2759723002/diff/1/components/navigation_inter...
File components/navigation_interception/navigation_params.cc (right):

https://codereview.chromium.org/2759723002/diff/1/components/navigation_inter...
components/navigation_interception/navigation_params.cc:36: void
NavigationParams::Assign(const NavigationParams& other) {
On 2017/03/17 19:16:23, jam wrote:
> can you just get rid of this method and the manually defined assignment
> operator, and instead use the default one? i.e. in the header:
> 
> NavigationParams(const NavigationParams& other) = default;

that removes a lot of boilerplate.

[commit-bot: I haz the power, 2017-03-17 19:57:08]

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

[jam1, 2017-03-17 19:59:39]

jam@google.com changed reviewers:
+ jam@google.com

[jam1, 2017-03-17 19:59:39]

https://codereview.chromium.org/2759723002/diff/20001/components/navigation_i...
File components/navigation_interception/navigation_params.h (right):

https://codereview.chromium.org/2759723002/diff/20001/components/navigation_i...
components/navigation_interception/navigation_params.h:27: NavigationParams&
operator=(const NavigationParams&) = default;
nit: i think this isn't actually used

[commit-bot: I haz the power, 2017-03-17 20:09:33]

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

[commit-bot: I haz the power, 2017-03-17 20:09:34]

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

[sgurun-gerrit only, 2017-03-17 20:09:35]

On 2017/03/17 19:59:39, jam1 wrote:
>
https://codereview.chromium.org/2759723002/diff/20001/components/navigation_i...
> File components/navigation_interception/navigation_params.h (right):
> 
>
https://codereview.chromium.org/2759723002/diff/20001/components/navigation_i...
> components/navigation_interception/navigation_params.h:27: NavigationParams&
> operator=(const NavigationParams&) = default;
> nit: i think this isn't actually used

Heh, compilation failure on mac:
n file included from
../../chrome/browser/plugins/flash_download_interception.cc:18:
../../components/navigation_interception/navigation_params.h:26:3: error:
[chromium-style] Complex constructor has an inlined body.
  NavigationParams(const NavigationParams&) = default;
  ^
1 error generated.

[sgurun-gerrit only, 2017-03-17 20:31:54]

The CQ bit was checked by sgurun@chromium.org

[sgurun-gerrit only, 2017-03-17 20:31:54]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2759723002/#ps40001
(title: "fix compile fail")

[commit-bot: I haz the power, 2017-03-17 20:32:59]

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

[jam1, 2017-03-17 21:30:12]

On 2017/03/17 20:09:35, sgurun wrote:
> On 2017/03/17 19:59:39, jam1 wrote:
> >
>
https://codereview.chromium.org/2759723002/diff/20001/components/navigation_i...
> > File components/navigation_interception/navigation_params.h (right):
> > 
> >
>
https://codereview.chromium.org/2759723002/diff/20001/components/navigation_i...
> > components/navigation_interception/navigation_params.h:27: NavigationParams&
> > operator=(const NavigationParams&) = default;
> > nit: i think this isn't actually used
> 
> Heh, compilation failure on mac:
> n file included from
> ../../chrome/browser/plugins/flash_download_interception.cc:18:
> ../../components/navigation_interception/navigation_params.h:26:3: error:
> [chromium-style] Complex constructor has an inlined body.
>   NavigationParams(const NavigationParams&) = default;
>   ^
> 1 error generated.

I meant just the assignment operator, but as is is ok too. we can remove it
later in the interest of getting this patch landed so we can update the
waterfall :)

[commit-bot: I haz the power, 2017-03-17 22:06:21]

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

[commit-bot: I haz the power, 2017-03-17 22:06:22]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jam, 2017-03-17 22:45:55]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2017-03-17 22:46:58]

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-03-17 23:03:27]

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

[commit-bot: I haz the power, 2017-03-17 23:03:28]

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_...)

[jam, 2017-03-17 23:11:10]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2017-03-17 23:12:14]

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-03-18 02:17:37]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489792270673700,
"parent_rev": "4494b892750fe71d43c2dc0cef3247a38d777b42", "commit_rev":
"ab03e18490552251ca0e2ee6249509071f8c5b15"}

[commit-bot: I haz the power, 2017-03-18 02:18:19]

Description was changed from

==========
Update the assignment operator

The navigation_params was added a new parameter. The parameter has to be
copied correctly in assignment.

BUG=645983
==========

to

==========
Update the assignment operator

The navigation_params was added a new parameter. The parameter has to be
copied correctly in assignment.

BUG=645983

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

[commit-bot: I haz the power, 2017-03-18 02:18:25]

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

[sgurun-gerrit only, 2017-03-18 02:21:22]

On 2017/03/18 02:18:25, commit-bot: I haz the power wrote:
> Committed patchset #3 (id:40001) as
>
https://chromium.googlesource.com/chromium/src/+/ab03e18490552251ca0e2ee62495...

finally :)