Bug 21826 - Add accessor for SecurityOrigin::m_domainWasSetInDOM
Summary: Add accessor for SecurityOrigin::m_domainWasSetInDOM
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-23 01:53 PDT by Adam Barth
Modified: 2008-10-23 10:20 PDT (History)
2 users (show)

See Also:


Attachments
le patch (2.00 KB, patch)
2008-10-23 03:02 PDT, Adam Barth
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2008-10-23 01:53:11 PDT
It will help Chromium unfork SecurityOrigin.cpp if we add an accessor for SecurityOrigin::m_domainWasSetInDOM.  If this flag is false, Chromium can do a same-origin check in one pointer comparison by atomizing the toString() of the SecurityOrigin (with some other caveats).

Currently they have YASOTSF (Yet Another SecurityOrigin toString Function) called SecurityOrigin::securityToken that knows about m_domainWasSetInDOM.  Adding an accessor for m_domainWasSetInDOM seems better than adding this function to our tree.

Now, if only I can get rid of the YASOTSF called SecurityOrigin::databaseIdentifier, my plans for grand unification of the SecurityOrigin toString methods will be complete!

Patch forthcoming.
Comment 1 Adam Barth 2008-10-23 03:02:25 PDT
Created attachment 24594 [details]
le patch
Comment 2 Sam Weinig 2008-10-23 06:46:50 PDT
Comment on attachment 24594 [details]
le patch

+ * Copyright (C) 2007-2008 Apple Inc. All rights reserved.
We like to use comma delineated years.

         void setDomainFromDOM(const String& newDomain);
 
+        bool domainWasSetInDOM() const { return m_domainWasSetInDOM; }
These should be paragraphed together.

I am a little unclear about why this is necessary, the SecurityOrigin it self should be sufficient for a fast compare.  Perhaps we can remove this later.
Comment 3 Adam Barth 2008-10-23 10:20:51 PDT
Fixed in http://trac.webkit.org/changeset/37814

> I am a little unclear about why this is necessary, the SecurityOrigin it self
> should be sufficient for a fast compare.  Perhaps we can remove this later.

Yeah, maybe there is a better long term solution here.  I think the fast path for comparing SecurityOrigins in V8 gets inlined into the JITed code and they want to avoid the function call overhead.  (I might be wrong about this, V8 is pretty mysterious to me.)  It's similar to how JavaScriptCore doesn't bother calling SecurityOrigin::canAccess if a frame is accessing itself (the most common case) but taken to a little further extreme.