Bug 37449 – SecurityOrigin needs a way to remove individual OriginAccessEntries

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Timothy Hatcher]

See summary.

[Timothy Hatcher]

Created attachment 53176 [details]
Proposed patch

[Adam Roben (:aroben)]

Comment on attachment 53176 [details]
Proposed patch

> +Testing: http://127.0.0.1:8000 http:localhost 

I find this output a little confusing to parse. Maybe adding "source origin=" and "destination origin=" would make it clearer, e.g., "Testing: source origin=http://127.0.0.1:8000 destination origin=http:localhost".

> +Testing: http://127.0.0.1:8000 http:localhost allowing subdoamins

Typo: subdoamins

> +function nextTest()
> +{
> +    if (currentTest < tests.length)
> +        test.apply(null, tests[currentTest++]);
> +    else
> +        layoutTestController.notifyDone();
> +}

Seems like a for loop or Array.forEach would work just fine.

> +        (WebCore::SecurityOrigin::whiteListAccessFromOrigin): Use the add method to preent a
> +        second hash lookup.

Typo: preent

> +        (WebCore::SecurityOrigin::removeWhiteListAccessFromOrigin): Added. Find a matching
> +        OriginAccessEntry and remove it.

This name is unfortunately not parallel with "whiteListAccessFromOrigin". That function uses the verb "white list", while your new function uses the noun "white list access". We either need a verb that means "un-white list", or we should rename whiteListAccessFromOrigin. Maybe addOriginAccessWhitelistEntry/removeOriginAccessWhitelistEntry?

> +inline bool operator==(const OriginAccessEntry& a, const OriginAccessEntry& b)
> +{
> +    return a.protocol() == b.protocol() && a.host() == b.host() && a.subdomainSettings() == b.subdomainSettings();
> +}

Protocols and hosts are supposed to be treated case-insensitively.

r- because of the case-sensitivity issue (maybe you should add a test for this), but otherwise this looks good to go (though figuring out better names would be nice).

[Timothy Hatcher]

Created attachment 53230 [details]
Rename part

[Timothy Hatcher]

Created attachment 53259 [details]
Final part

[Dave Hyatt]

Comment on attachment 53259 [details]
Final part

r=me

[Dave Hyatt]

Comment on attachment 53230 [details]
Rename part

r=me

[Timothy Hatcher]

Fixed in r57535 and 57537.