Cross-origin resource sharing is a separate specification that will be used for more than just XHR.
Created attachment 28413 [details] Part 1
Comment on attachment 28413 [details] Part 1 > +bool accessControlCheck(const ResourceResponse& response, bool includeCredentials, SecurityOrigin* securityOrigin) I'm not sure this name is clear enough; we normally try to name functions so they describe their return values. Maybe something like passesAccessControlCheck would be better? Or some other name that describes the boolean return value. I also think the "named boolean" idiom might be best for the includeCredentials boolean, assuming that most callers pass a constant. > + unsigned expiryDelta = 0; > + if (!parseAccessControlMaxAge(response.httpHeaderField("Access-Control-Max-Age"), expiryDelta)) > + expiryDelta = 5; Annoying to find this magic number hiding here. Ideally the 5 would be named constant. I know you didn't add this. It also seems strange to initialize expiryDelta to 0 and then set it to 5. Do we need to initialize it at all? > +bool CrossOriginPreflightResultCache::canSkipPreflight(const String& origin, const KURL& url, bool includeCredentials, > + const String& method, const HTTPHeaderMap& requestHeaders) The above is a practical demonstration of why I prefer to indent by a single tab stop instead of lining things up. > + template<class HashType> > + static void addToAccessControlAllowList(const String& string, unsigned start, unsigned end, HashSet<String, HashType>&); > + template<class HashType> > + static bool parseAccessControlAllowList(const String& string, HashSet<String, HashType>&); > + static bool parseAccessControlMaxAge(const String& string, unsigned& expiryDelta); I think the template declarations either need to be all one one line, or perhaps with some blank space. I'm not sure why these functions are class members -- they could be free functions instead. The argument name "string" could be omitted. > + // FIXME: A better solution to holding onto the absolute expiration time might be > + // to start a timer for the expiration delta, that removes this from the cache when > + // it fires. This has a comma that's incorrect and should be removed. r=me, with or without changes
Comment on attachment 28413 [details] Part 1 Committed revision 41548, clearing review flag. I made the changes, except for adding an enum for includeCredentials argument (because the funciton is always called with a variable for it), and for adding a constant for expiryDelta (my understanding that this code will need to be refactored soon).
This work is continued in bug 28134.