Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(27)

Issue 2759723002: Update the assignment operator (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months ago by sgurun-gerrit only
Modified:
5 months ago
Reviewers:
jam, jam1
CC:
chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 2

Patch Set 2 : Address jam review #

Total comments: 1

Patch Set 3 : fix compile fail #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -24 lines) Patch
M components/navigation_interception/navigation_params.h View 1 2 2 chunks +2 lines, -3 lines 0 comments Download
M components/navigation_interception/navigation_params.cc View 1 2 2 chunks +1 line, -18 lines 0 comments Download
M components/navigation_interception/navigation_params_android.cc View 1 1 chunk +1 line, -3 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 29 (14 generated)
sgurun-gerrit only
On 2017/03/17 18:20:48, sgurun wrote: > mailto:sgurun@chromium.org changed reviewers: > + mailto:jam@chromium.org ptal, thanks
5 months ago (2017-03-17 18:20:56 UTC) #3
jam
if you run this through trybots there are also reliable failures now in LoadDataWithBaseUrlTest#testInvalidBaseUrl LoadDataWithBaseUrlTest#testOnPageFinishedWithInvalidBaseUrlWhenInterrupted ...
5 months ago (2017-03-17 19:16:23 UTC) #4
sgurun-gerrit only
https://codereview.chromium.org/2759723002/diff/1/components/navigation_interception/navigation_params.cc File components/navigation_interception/navigation_params.cc (right): https://codereview.chromium.org/2759723002/diff/1/components/navigation_interception/navigation_params.cc#newcode36 components/navigation_interception/navigation_params.cc:36: void NavigationParams::Assign(const NavigationParams& other) { On 2017/03/17 19:16:23, jam ...
5 months ago (2017-03-17 19:56:51 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2759723002/20001
5 months ago (2017-03-17 19:57:08 UTC) #8
jam1
https://codereview.chromium.org/2759723002/diff/20001/components/navigation_interception/navigation_params.h File components/navigation_interception/navigation_params.h (right): https://codereview.chromium.org/2759723002/diff/20001/components/navigation_interception/navigation_params.h#newcode27 components/navigation_interception/navigation_params.h:27: NavigationParams& operator=(const NavigationParams&) = default; nit: i think this ...
5 months ago (2017-03-17 19:59:39 UTC) #10
commit-bot: I haz the power
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_compile_dbg_ng/builds/376494)
5 months ago (2017-03-17 20:09:34 UTC) #12
sgurun-gerrit only
On 2017/03/17 19:59:39, jam1 wrote: > https://codereview.chromium.org/2759723002/diff/20001/components/navigation_interception/navigation_params.h > File components/navigation_interception/navigation_params.h (right): > > https://codereview.chromium.org/2759723002/diff/20001/components/navigation_interception/navigation_params.h#newcode27 > ...
5 months ago (2017-03-17 20:09:35 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2759723002/40001
5 months ago (2017-03-17 20:32:59 UTC) #16
jam1
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_interception/navigation_params.h ...
5 months ago (2017-03-17 21:30:12 UTC) #17
commit-bot: I haz the power
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_rel_ng/builds/411417)
5 months ago (2017-03-17 22:06:22 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2759723002/40001
5 months ago (2017-03-17 22:46:58 UTC) #21
commit-bot: I haz the power
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_ng/builds/403179)
5 months ago (2017-03-17 23:03:28 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2759723002/40001
5 months ago (2017-03-17 23:12:14 UTC) #25
commit-bot: I haz the power
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/ab03e18490552251ca0e2ee6249509071f8c5b15
5 months ago (2017-03-18 02:18:25 UTC) #28
sgurun-gerrit only
5 months ago (2017-03-18 02:21:22 UTC) #29
Message was sent while issue was closed.
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 :)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b