Fix a crashing call to GURL::spec() in downloads_api.cc:DownloadItemToJSON()

Fix a crashing call to GURL::spec() in downloads_api.cc:DownloadItemToJSON()

The actual bug is DownloadItemImpl::GetOriginalUrl(): the front() of an empty vector is not a good GURL reference!

The sprinkling of GURL::is_valid() is just to make me feel better.

There is still the open question of what happened to the downloads_url_chain records, but that is less urgent than stopping the crashes.

BUG=190096
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188602

[benjhayden, 2013-03-14 13:58:33]

I repro'd the crash and the fix, but can I just have another set of eyes on this
before I commit it?

It looks like there are History databases out there where the items have no
URLs. Both from the stack trace in the bug, and because I repro'd it by going
into sqlite3 and deleting the downloads_url_chain entries. Not sure if we want
to try to do anything about that.

[asanka, 2013-03-14 16:10:12]

On 2013/03/14 13:58:33, benjhayden_chromium wrote:
> I repro'd the crash and the fix, but can I just have another set of eyes on
this
> before I commit it?
> 
> It looks like there are History databases out there where the items have no
> URLs. Both from the stack trace in the bug, and because I repro'd it by going
> into sqlite3 and deleting the downloads_url_chain entries. Not sure if we want
> to try to do anything about that.

Interesting. Have you audited the callers of DownloadItem::GetUrlChain() to make
sure they deal with empty URL chains?

[asanka, 2013-03-14 16:19:00]

LGTM

[Randy Smith (Not in Mondays), 2013-03-14 16:52:23]

On 2013/03/14 16:19:00, asanka wrote:
> LGTM

Not reviewing this since Asanka did (and I have no objections to the basic
concept), but after this lands can you put some thought into how the DBs might
have gotten a null URL chain in them?  It might be just DB corruption, but the
timing is suspicious--why did this suddenly spike?  I suspect we don't have root
cause yet, and I'm a big believer in "if you don't understand it, it's
dangerous", especially with regard to crashes.

[benjhayden, 2013-03-14 17:38:36]

Scott: review for safe_browsing ownership?

[Scott Hess - ex-Googler, 2013-03-14 17:46:51]

I'm with Randy - this papers over the symptom, but I think that there's probably
something wrong which isn't fixing.

https://codereview.chromium.org/12607011/diff/27002/chrome/browser/safe_brows...
File chrome/browser/safe_browsing/ping_manager.cc (right):

https://codereview.chromium.org/12607011/diff/27002/chrome/browser/safe_brows...
chrome/browser/safe_browsing/ping_manager.cc:142: referrer_url.spec() : "";
What does it mean to have a threat-hit but empty URLs for this?  It looks like
the "none" case is expected to never happen, so I think it means that this fix
will pollute someone's reporting.

[commit-bot: I haz the power, 2013-03-15 19:43:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benjhayden@chromium.org/12607011/33004

[commit-bot: I haz the power, 2013-03-16 03:34:57]

Retried try job too often on win_rel for step(s) sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[benjhayden, 2013-03-16 17:34:11]

Committed patchset #3 manually as r188602 (presubmit successful).