Bug 26180

Summary:

If "otherOrigin == this" in SecurityOrigin::equal, it's obviously equal.

Product:

WebKit

Reporter:

Jeremy Orlow <jorlow>

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
A fix.	darin: review+

Jeremy Orlow

Reported 2009-06-03 19:19:51 PDT

If "otherOrigin == this" in SecurityOrigin::equal, it's obviously equal. From http://lists.macosforge.org/pipermail/webkit-dev/2009-June/008038.html > SecurityOrigin was created to cleanup the way we do same-origin checks in > the JavaScript bindings, and as such, carry some of that baggage (eg. domain > relaxing). The main design constraint then was that it was a cheap compare > for two origins representing the same document (pointer compare) as that is > common case. The code evolved from there. Makes sense. Note that SecurityOrigin::equal does not currently do a fast path check of "this == other" (to just do a pointer compare as you mentioned) so this is probably low hanging fruit to make things a bit faster. (Though, as you said, it's not showing up on current profiles...so it's probably not a huge deal.)

Attachments
A fix. (1.14 KB, patch) 2009-06-03 19:27 PDT, Jeremy Orlow	darin: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Jeremy Orlow

Comment 1 2009-06-03 19:27:42 PDT

Created attachment 30937 [details] A fix.

Sam Weinig

Comment 2 2009-06-03 19:42:29 PDT

Wouldn't it makes more sense to do this in isSameSchemeHostPort?

Jeremy Orlow

Comment 3 2009-06-03 20:23:38 PDT

I don't think so. I only see isSameSchemeHostPort used a couple places, but they don't seem like they'd benefit from such an optimization [1]. ::equal, on the other hand, is called pretty often (it's used a lot in hash tables). It seems a bit painful to require another function call plus a branch or two for a function that's not called very often. We could put the == check in both places. Doing one additional branch isn't going to hurt the slow path. It doesn't seem necessary though. [1] http://www.google.com/codesearch?hl=en&q=isSameSchemeHostPort+package%3A%22git%3A%2F%2Fandroid.git.kernel.org%2Fplatform%2Fexternal%2Fwebkit.git%22

Darin Adler

Comment 4 2009-06-04 09:18:59 PDT

Comment on attachment 30937 [details] A fix. Seems fine. r=me

Brent Fulgham

Comment 5 2009-06-04 11:10:54 PDT

Assign for landing.

Brent Fulgham

Comment 6 2009-06-04 11:34:50 PDT

Landed in @r44420.

Note You need to log in before you can comment on or make changes to this bug.