Bug 69380 - toURLRef() in WKSharedAPICast.h should return null for a null string
Summary: toURLRef() in WKSharedAPICast.h should return null for a null string
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ada Chan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-04 15:05 PDT by Ada Chan
Modified: 2011-10-14 17:14 PDT (History)
2 users (show)

See Also:


Attachments
Patch: Add the missing return for the null string case. (1.22 KB, patch)
2011-10-04 15:29 PDT, Ada Chan
jhoneycutt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ada Chan 2011-10-04 15:05:49 PDT
toURLRef() in WKSharedAPICast.h should return null for a null string.

Looks like it's just missing a "return" here:

    if (!string)
        ProxyingRefPtr<WebURL>(0);
Comment 1 Ada Chan 2011-10-04 15:29:52 PDT
Created attachment 109705 [details]
Patch: Add the missing return for the null string case.
Comment 2 Ada Chan 2011-10-04 15:46:41 PDT
Committed: http://trac.webkit.org/changeset/96656
Comment 3 Adam Roben (:aroben) 2011-10-05 04:02:02 PDT
What's the user-visible or developer-visible effect of this bug? In general I think it's a lot more useful to have bugs describe symptoms rather than describing changes we'd like to make to the code.

Is it possible to write a regression test for this?
Comment 4 Ada Chan 2011-10-05 10:39:52 PDT
(In reply to comment #3)
> What's the user-visible or developer-visible effect of this bug? In general I think it's a lot more useful to have bugs describe symptoms rather than describing changes we'd like to make to the code.

If WKURLCopyCFURL() is called on the resulting WKURLRef later, it'll assert in
ASSERT(!toImpl(URLRef)->string().isNull());
because the string is null.

> Is it possible to write a regression test for this?

I'll look into writing a TestWebKitAPI test for this.

Thanks for the suggestion!
Comment 5 Ada Chan 2011-10-14 17:14:37 PDT
(In reply to comment #3)
> What's the user-visible or developer-visible effect of this bug? In general I think it's a lot more useful to have bugs describe symptoms rather than describing changes we'd like to make to the code.
> 
> Is it possible to write a regression test for this?

Coming up with a test for this is not as straightforward as I imagined, since we can't call toURLRef() directly from a TestWebKitAPI test.  I've filed https://bugs.webkit.org/show_bug.cgi?id=70159 to track coming up with a test for this.