On Wed, Oct 14, 2009 at 4:03 PM, Jeremy Orlow <jor...@chromium.org> wrote:
> On Wed, Oct 14, 2009 at 3:58 PM, Dumitru Daniliuc <d...@chromium.org>wrote: > >> >> >> On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher <da...@chromium.org> wrote: >> >>> On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow <jor...@chromium.org>wrote: >>> >>>> On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher <da...@chromium.org>wrote: >>>> >>>>> On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow <jor...@chromium.org>wrote: >>>>> >>>>>> On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman <micha...@google.com >>>>>> > wrote: >>>>>> >>>>>>> +1 SecurityOrigin class >>>>>>> Sounds like a reasonable plan. >>>>>>> I suspect there may already be cases where we're actually comparing a >>>>>>> chrome generated security origin, as produced by GURL.GetOrigin(), with >>>>>>> a >>>>>>> webkit generated security origin, as produced by >>>>>>> WebSecurityOrigin.toString(). So we may want to accelerate the part of >>>>>>> the >>>>>>> plan to do more than opaquely pass around and test webkit generated >>>>>>> representations. >>>>>>> >>>>>>> Also, I think dumi has a use case to crack it open in order to form >>>>>>> file path elements of the form 'scheme_host_port' >>>>>>> >>>>>> >>>>>> Actually, Dumi's case is slightly different. He wants to get >>>>>> SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin >>>>>> should >>>>>> have a databaseIdentifier() method that outputs a FilePath object? >>>>>> >>>>> >>>>> Dumi has such a method in a CL that he is working on at the moment. >>>>> Also, note: we don't have a way to use FilePath from the WebKit API, and >>>>> I'm not sure that we should. We use WebString for file paths in the >>>>> WebKit >>>>> API. >>>>> >>>> >>>> So then he's adding such a method to WebSecurityOrigin that returns a >>>> string? If so, sounds good. What's the CL, btw? >>>> >>> >>> Yes: >>> http://codereview.chromium.org/256073/diff/11001/11029 >>> >>> >> we decided to use GURLs instead of string16s to represent SecurityOrigins >> on the chromium side, so we don't need a >> (Web)SecurityOrigin::toFilePath()-like method anymore; we can just do >> GURL(SecurityOrigin::toString()), and then create a file path from >> GURL::scheme(), GURL::host() and GURL::port(). >> > > The point of this thread is that we should not be converting > SecurityOrigins of GURLs. I believe michaeln was the primary proponent of > this and I believe we convinced him that we shouldn't be doing it. And I > believe most if not all the reasons why were in my original email. > (Michael, correct me if I'm wrong.) > I think i have two primary concerns. 1) What is the format of the data written to disk that we need to support going forward since it is on disk. We need a decision that we can stick with. 2) What measures are in place to ensure that we're actually persisting data in that prescribed format today. Having 'strings' float around makes me uneasy on that second point. If chrome can't validate these string values are in the prescribed format (its not smart enough to reason about them), how can we assert we've got it right (inspection doesn't work well... working backwards from a callsite in chrome browser code storing an 'origin' string to the source of that string being produced is just not practical). Given the current state of affairs, i still think GURL is a better option. Given a GURL, we can reason about it (produce path elements, produce a canonical string representation, test if another GURL falls in that origin or not (we do that in appcache code sans webkit)). The "null" security origin is an odd corner case that gives the GURL representation grief. > >> also, i'd argue that no class representing an origin should have a >> toFilePath()-like method: origins and file paths have nothing in common; >> using the origin URL as part of the DB file name is a database-specific >> decision and the code for that conversion should be kept in some >> database-specific class, or a separate origin_to_file_path_util.h file at >> best. (It was very tempting to use SecurityOrigin::databaseIdentifier() only >> because that method was already there.) >> > > Well, SecurityOrigin has a method that creates a database file name. I > don't see why we can't have a "::databasePath" method of our own. And if we > do, then it does make sense for it to return a FilePath. > > That said, I think what Darin suggested in the code review is actually the > cleanest way to do it. And I think returning a String is not a big deal. > --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
