Add DCHECK that Reload should never be called with ReloadType::NONE

Add DCHECK that Reload should never be called with ReloadType::NONE

In ReloadType, NONE means that the navigation is not a reload, so it
makes no sense to call Reload function with ReloadType::NONE as
parameter.

This CL clarifies it by mentioning it in public API comment and add a
DCHECK in the implementation.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2740843004
Cr-Commit-Position: refs/heads/master@{#456129}
Committed: https://chromium.googlesource.com/chromium/src/+/9168fba24e7585305b61b67f426e025c358b3c61

[liaoyuke, 2017-03-10 06:01:29]

Description was changed from

==========
Add DCHECK that Reload should never be called with ReloadType::NONE

In ReloadType, NONE means that the navigation is not a reload, so it
makes no sense to call Reload function with ReloadType::NONE as
parameter.

This CL clarifies it by mentioning it in public API comment and add a
DCHECK in the implementation.

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

to

==========
Add DCHECK that Reload should never be called with ReloadType::NONE

In ReloadType, NONE means that the navigation is not a reload, so it
makes no sense to call Reload function with ReloadType::NONE as
parameter.

This CL clarifies it by mentioning it in public API comment and add a
DCHECK in the implementation.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[liaoyuke, 2017-03-10 06:01:49]

The CQ bit was checked by liaoyuke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-10 06:02:20]

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, 2017-03-10 06:51:23]

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

[commit-bot: I haz the power, 2017-03-10 06:51:24]

Dry run: This issue passed the CQ dry run.

[liaoyuke, 2017-03-10 16:57:33]

liaoyuke@chromium.org changed reviewers:
+ creis@chromium.org

[liaoyuke, 2017-03-10 16:57:33]

Hi Charlie,

PTAL.

Thank you very much!

[Charlie Reis, 2017-03-10 17:41:05]

[CC toyoshim]

Thanks-- that should help clarify the intent.  Also glad to see the bots were
happy with it, and that we didn't have any unexpected corner cases.

LGTM with nit.

https://codereview.chromium.org/2740843004/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl.cc (right):

https://codereview.chromium.org/2740843004/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl.cc:300:
DCHECK_NE(reload_type, ReloadType::NONE);
Minor nit: Swap order of arguments.  (DCHECK macros are normally "expected,
actual", which can affect how a failure message is phrased.)

[liaoyuke, 2017-03-10 18:00:14]

Addressed comments.

Thank you for the quick response!

https://codereview.chromium.org/2740843004/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl.cc (right):

https://codereview.chromium.org/2740843004/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl.cc:300:
DCHECK_NE(reload_type, ReloadType::NONE);
On 2017/03/10 17:41:05, Charlie Reis (slow) wrote:
> Minor nit: Swap order of arguments.  (DCHECK macros are normally "expected,
> actual", which can affect how a failure message is phrased.)

Done.

[liaoyuke, 2017-03-10 18:00:55]

The CQ bit was checked by liaoyuke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-10 18:01:15]

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, 2017-03-10 18:55:21]

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

[commit-bot: I haz the power, 2017-03-10 18:55:22]

Dry run: This issue passed the CQ dry run.

[liaoyuke, 2017-03-10 19:15:50]

The CQ bit was checked by liaoyuke@chromium.org

[liaoyuke, 2017-03-10 19:15:51]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org
Link to the patchset: https://codereview.chromium.org/2740843004/#ps20001
(title: "Swapped arguments order in DCHECK_NE")

[commit-bot: I haz the power, 2017-03-10 19:16:25]

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-10 19:20:33]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489173350443920,
"parent_rev": "5f8b72e3c433990df81374e7a68144069da8d7d9", "commit_rev":
"9168fba24e7585305b61b67f426e025c358b3c61"}

[commit-bot: I haz the power, 2017-03-10 19:21:13]

Description was changed from

==========
Add DCHECK that Reload should never be called with ReloadType::NONE

In ReloadType, NONE means that the navigation is not a reload, so it
makes no sense to call Reload function with ReloadType::NONE as
parameter.

This CL clarifies it by mentioning it in public API comment and add a
DCHECK in the implementation.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add DCHECK that Reload should never be called with ReloadType::NONE

In ReloadType, NONE means that the navigation is not a reload, so it
makes no sense to call Reload function with ReloadType::NONE as
parameter.

This CL clarifies it by mentioning it in public API comment and add a
DCHECK in the implementation.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2740843004
Cr-Commit-Position: refs/heads/master@{#456129}
Committed:
https://chromium.googlesource.com/chromium/src/+/9168fba24e7585305b61b67f426e...
==========

[commit-bot: I haz the power, 2017-03-10 19:21:14]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/9168fba24e7585305b61b67f426e...