DCHECK that jstring to C++ string conversions don't pass in null.

DCHECK that jstring to C++ string conversions don't pass in null.

Also, update existing callers to avoid the call if they would otherwise trip the DCHECK now.

BUG=597564
Committed: https://crrev.com/f6d2732a2b81694b3d7ee47affaa6a22ee405cf9
Cr-Commit-Position: refs/heads/master@{#384010}

[Bernhard Bauer, 2016-03-24 10:58:18]

Description was changed from

==========
DCHECK that jstring to C++ string conversions don't pass in null.

BUG=TBD
==========

to

==========
DCHECK that jstring to C++ string conversions don't pass in null.

BUG=597564
==========

[Bernhard Bauer, 2016-03-24 12:13:04]

bauerb@chromium.org changed reviewers:
+ torne@chromium.org

[Bernhard Bauer, 2016-03-24 12:13:05]

Please review. Thanks!

[Torne, 2016-03-24 14:25:47]

Making this a DCHECK and then not handling it at all means we'll be crashing on
release builds instead of logspamming and using emptystring; is that okay?

Obviously the current situation isn't great but is this being hit somewhere?

[Bernhard Bauer, 2016-03-24 14:46:10]

On 2016/03/24 14:25:47, Torne wrote:
> Making this a DCHECK and then not handling it at all means we'll be crashing
on
> release builds instead of logspamming and using emptystring; is that okay?

That is in fact my plan 😃 In the ideal case, we'll hit those already when
running locally in a Debug build, but for other cases, crashing in the wild is
going to be the only way for us to find out about this and ultimately fix it.

> Obviously the current situation isn't great but is this being hit somewhere?

I saw a CL yesterday that was on purpose passing null in here, which would have
been caught by this CL.

If this is _not_ being hit anywhere else, that would actually be great, because
it would mean we could just turn this DCHECK on without breaking anything.

[Torne, 2016-03-24 14:51:40]

lgtm

[Bernhard Bauer, 2016-03-24 14:53:31]

The CQ bit was checked by bauerb@chromium.org

[commit-bot: I haz the power, 2016-03-24 14:53:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1828193002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1828193002/1

[commit-bot: I haz the power, 2016-03-24 18:18:25]

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

[commit-bot: I haz the power, 2016-03-24 18:18:26]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Bernhard Bauer, 2016-03-30 13:42:38]

bauerb@chromium.org changed reviewers:
+ johnme@chromium.org, miguelg@chromium.org

[Bernhard Bauer, 2016-03-30 13:43:38]

bauerb@chromium.org changed reviewers:
+ mmenke@chromium.org, peter@chromium.org
- johnme@chromium.org

[Bernhard Bauer, 2016-03-30 13:45:19]

Miguel: Please review (/chrome/browser)/ui/android changes.
Matt: Please review prerender changes.
Peter: Please review GCM changes.

Thanks!

[Peter Beverloo, 2016-03-30 13:47:26]

On 2016/03/30 13:45:19, Bernhard Bauer wrote:
> Miguel: Please review (/chrome/browser)/ui/android changes.
> Matt: Please review prerender changes.
> Peter: Please review GCM changes.
> 
> Thanks!

gcm lgtm

[Miguel Garcia, 2016-03-30 14:30:12]

chrome/browser/ui/android/ lgtm

https://codereview.chromium.org/1828193002/diff/80001/chrome/browser/ui/andro...
File chrome/browser/ui/android/context_menu_helper.cc (right):

https://codereview.chromium.org/1828193002/diff/80001/chrome/browser/ui/andro...
chrome/browser/ui/android/context_menu_helper.cc:33: const char
kDataReductionProxyPassthroughHeader[] =
can you at least add a comment saying that the java version is in
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ContextMenuHelper.java
and should be kept in sync?

[Bernhard Bauer, 2016-03-30 14:43:45]

https://codereview.chromium.org/1828193002/diff/80001/chrome/browser/ui/andro...
File chrome/browser/ui/android/context_menu_helper.cc (right):

https://codereview.chromium.org/1828193002/diff/80001/chrome/browser/ui/andro...
chrome/browser/ui/android/context_menu_helper.cc:33: const char
kDataReductionProxyPassthroughHeader[] =
On 2016/03/30 14:30:12, Miguel Garcia wrote:
> can you at least add a comment saying that the java version is in
>
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ContextMenuHelper.java
> and should be kept in sync?

It's gone from there. That value was only used to pass it to C++, so I moved the
whole thing here.

[mmenke, 2016-03-30 14:52:58]

LGTM, modulo comment.

https://codereview.chromium.org/1828193002/diff/80001/chrome/browser/prerende...
File chrome/browser/prerender/external_prerender_handler_android.cc (right):

https://codereview.chromium.org/1828193002/diff/80001/chrome/browser/prerende...
chrome/browser/prerender/external_prerender_handler_android.cc:71: if
(!jreferrer.is_null()) {
None of these changes seem to fall under the rubric "DCHECK that jstring to C++
string conversions don't pass in null".  Should update the CL description.

[Bernhard Bauer, 2016-03-30 14:55:38]

Description was changed from

==========
DCHECK that jstring to C++ string conversions don't pass in null.

BUG=597564
==========

to

==========
DCHECK that jstring to C++ string conversions don't pass in null.

Also, update existing callers that would otherwise trip the DCHECK now.

BUG=597564
==========

[Bernhard Bauer, 2016-03-30 15:52:09]

https://codereview.chromium.org/1828193002/diff/80001/chrome/browser/prerende...
File chrome/browser/prerender/external_prerender_handler_android.cc (right):

https://codereview.chromium.org/1828193002/diff/80001/chrome/browser/prerende...
chrome/browser/prerender/external_prerender_handler_android.cc:71: if
(!jreferrer.is_null()) {
On 2016/03/30 14:52:57, mmenke wrote:
> None of these changes seem to fall under the rubric "DCHECK that jstring to
C++
> string conversions don't pass in null".  Should update the CL description.

Done.

[Bernhard Bauer, 2016-03-30 15:53:02]

Description was changed from

==========
DCHECK that jstring to C++ string conversions don't pass in null.

Also, update existing callers that would otherwise trip the DCHECK now.

BUG=597564
==========

to

==========
DCHECK that jstring to C++ string conversions don't pass in null.

Also, update existing callers to avoid the call if they would otherwise trip the
DCHECK now.

BUG=597564
==========

[Bernhard Bauer, 2016-03-30 15:53:11]

The CQ bit was checked by bauerb@chromium.org

[Bernhard Bauer, 2016-03-30 15:53:11]

The patchset sent to the CQ was uploaded after l-g-t-m from torne@chromium.org,
miguelg@chromium.org, peter@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/1828193002/#ps100001
(title: "braces")

[commit-bot: I haz the power, 2016-03-30 15:53:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1828193002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1828193002/100001

[commit-bot: I haz the power, 2016-03-30 17:03:01]

Description was changed from

==========
DCHECK that jstring to C++ string conversions don't pass in null.

Also, update existing callers to avoid the call if they would otherwise trip the
DCHECK now.

BUG=597564
==========

to

==========
DCHECK that jstring to C++ string conversions don't pass in null.

Also, update existing callers to avoid the call if they would otherwise trip the
DCHECK now.

BUG=597564
==========

[commit-bot: I haz the power, 2016-03-30 17:03:03]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-03-30 17:04:13]

Description was changed from

==========
DCHECK that jstring to C++ string conversions don't pass in null.

Also, update existing callers to avoid the call if they would otherwise trip the
DCHECK now.

BUG=597564
==========

to

==========
DCHECK that jstring to C++ string conversions don't pass in null.

Also, update existing callers to avoid the call if they would otherwise trip the
DCHECK now.

BUG=597564

Committed: https://crrev.com/f6d2732a2b81694b3d7ee47affaa6a22ee405cf9
Cr-Commit-Position: refs/heads/master@{#384010}
==========

[commit-bot: I haz the power, 2016-03-30 17:04:14]

Patchset 6 (id:??) landed as
https://crrev.com/f6d2732a2b81694b3d7ee47affaa6a22ee405cf9
Cr-Commit-Position: refs/heads/master@{#384010}