Remove unnecessary IsInfoEmpty() call in app_banner_infobar_delegate_android.cc

Remove unnecessary IsInfoEmpty() call in app_banner_infobar_delegate_android.cc

BUG=728300
Review-Url: https://codereview.chromium.org/2913383002
Cr-Commit-Position: refs/heads/master@{#476531}
Committed: https://chromium.googlesource.com/chromium/src/+/70f73ad825e0b878a81815df06d82e8da1e8d81e

[pkotwicz, 2017-06-01 04:39:21]

pkotwicz@chromium.org changed reviewers:
+ dominickn@chromium.org

[pkotwicz, 2017-06-01 04:39:21]

Dominick, can you please take a look?

[dominickn, 2017-06-01 04:56:30]

https://codereview.chromium.org/2913383002/diff/1/chrome/browser/android/bann...
File chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
(right):

https://codereview.chromium.org/2913383002/diff/1/chrome/browser/android/bann...
chrome/browser/android/banners/app_banner_infobar_delegate_android.cc:52: if
(url.is_empty())
Nit: add DCHECK(shortcut_info) if we're going to assume that the unique_ptr is
never null when passed in.

[pkotwicz, 2017-06-01 05:11:29]

https://codereview.chromium.org/2913383002/diff/1/chrome/browser/android/bann...
File chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
(right):

https://codereview.chromium.org/2913383002/diff/1/chrome/browser/android/bann...
chrome/browser/android/banners/app_banner_infobar_delegate_android.cc:52: if
(url.is_empty())
This is unnecessary. Line 51 will crash if unique_ptr is null.

[dominickn, 2017-06-01 05:29:05]

https://codereview.chromium.org/2913383002/diff/1/chrome/browser/android/bann...
File chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
(right):

https://codereview.chromium.org/2913383002/diff/1/chrome/browser/android/bann...
chrome/browser/android/banners/app_banner_infobar_delegate_android.cc:52: if
(url.is_empty())
On 2017/06/01 05:11:28, pkotwicz wrote:
> This is unnecessary. Line 51 will crash if unique_ptr is null.

Yes, I know, but I prefer a DCHECK crash (which is an explicit contract that it
should not be null) rather than the implicit contract of a crash.

[pkotwicz, 2017-06-01 19:42:54]

Ok, added a DCHECK()

Personally, I think that DCHECKS are mostly useful in flagging bugs when running
tests on try bots. I don't know how many people pay attention to DCHECKs. I have
not paid any attention to them for many years. I have recently started fixing
debug mode crashes when I find them.

[dominickn, 2017-06-02 00:22:45]

lgtm

As a general rule, devs should be running with DCHECKs on or in debug mode
unless they're doing performance testing. We use them extensively in
installability code to document and verify assumptions.

[pkotwicz, 2017-06-02 01:05:22]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2017-06-02 01:05:50]

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-06-02 02:44:40]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496365522284180,
"parent_rev": "ef488abe166563c1bd32648513543efc1efc7a7c", "commit_rev":
"70f73ad825e0b878a81815df06d82e8da1e8d81e"}

[commit-bot: I haz the power, 2017-06-02 02:44:48]

Description was changed from

==========
Remove unnecessary IsInfoEmpty() call in app_banner_infobar_delegate_android.cc

BUG=728300
==========

to

==========
Remove unnecessary IsInfoEmpty() call in app_banner_infobar_delegate_android.cc

BUG=728300

Review-Url: https://codereview.chromium.org/2913383002
Cr-Commit-Position: refs/heads/master@{#476531}
Committed:
https://chromium.googlesource.com/chromium/src/+/70f73ad825e0b878a81815df06d8...
==========

[commit-bot: I haz the power, 2017-06-02 02:44:49]

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