Check for null StringImpl in blockIsRowOfLinks.

Check for null StringImpl in blockIsRowOfLinks.

BUG=426237
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184292

[skobes, 2014-10-23 01:35:30]

skobes@chromium.org changed reviewers:
+ pdr@chromium.org

[skobes, 2014-10-23 01:35:30]

I'm pretty sure this is the right fix based on the stacktrace.  But I can't get
a repro, even with the testcase in the report.  Let me know if you have any
ideas.

[pdr., 2014-10-23 02:33:31]

On 2014/10/23 at 01:35:30, skobes wrote:
> I'm pretty sure this is the right fix based on the stacktrace.  But I can't
get a repro, even with the testcase in the report.  Let me know if you have any
ideas.

I'm not sure this is correct. WTFString::stripWhiteSpace already has a check for
a null string impl. We have similar crashes (e.g., go/crash/6f765cc9474441f9)
elsewhere so I wonder if we're running into some deeper issue.

[pdr., 2014-10-23 02:37:08]

On 2014/10/23 at 02:33:31, pdr wrote:
> On 2014/10/23 at 01:35:30, skobes wrote:
> > I'm pretty sure this is the right fix based on the stacktrace.  But I can't
get a repro, even with the testcase in the report.  Let me know if you have any
ideas.
> 
> I'm not sure this is correct. WTFString::stripWhiteSpace already has a check
for a null string impl. We have similar crashes (e.g.,
go/crash/6f765cc9474441f9) elsewhere so I wonder if we're running into some
deeper issue.

Should we just be calling stripWhiteSpace on the string directly instead of
going through the impl?

[skobes, 2014-10-23 16:31:31]

On 2014/10/23 02:37:08, pdr wrote:
> On 2014/10/23 at 02:33:31, pdr wrote:
> > On 2014/10/23 at 01:35:30, skobes wrote:
> > > I'm pretty sure this is the right fix based on the stacktrace.  But I
can't
> get a repro, even with the testcase in the report.  Let me know if you have
any
> ideas.
> > 
> > I'm not sure this is correct. WTFString::stripWhiteSpace already has a check
> for a null string impl. We have similar crashes (e.g.,
> go/crash/6f765cc9474441f9) elsewhere so I wonder if we're running into some
> deeper issue.
> 
> Should we just be calling stripWhiteSpace on the string directly instead of
> going through the impl?

Good catch, I didn't notice String has its own stripWhiteSpace.  Updated.

[pdr., 2014-10-23 17:08:15]

The CQ bit was checked by pdr@chromium.org

[pdr., 2014-10-23 17:08:16]

lgtm

[commit-bot: I haz the power, 2014-10-23 17:08:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/668823005/20001

[commit-bot: I haz the power, 2014-10-23 18:11:11]

Committed patchset #2 (id:20001) as 184292