[Webkit-unassigned] [Bug 28185] Inspector: Show Hidden Cookie Data

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 11 22:52:04 PDT 2009


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
  Attachment #34634|review?                     |review+
               Flag|                            |

--- Comment #7 from Darin Adler <darin at apple.com>  2009-08-11 22:52:03 PDT ---
(From update of attachment 34634)
> +    Document* document = ic->inspectedPage()->mainFrame()->domWindow()->document();

>From mainFrame() you can get to the document directly. No need to go through

> +    const Identifier nameIdentifier = Identifier(exec, "name");
> +    const Identifier valueIdentifier = Identifier(exec, "value");
> +    const Identifier domainIdentifier = Identifier(exec, "domain");
> +    const Identifier pathIdentifier = Identifier(exec, "path");
> +    const Identifier expiresIdentifier = Identifier(exec, "expires");
> +    const Identifier sizeIdentifier = Identifier(exec, "size");
> +    const Identifier httpOnlyIdentifier = Identifier(exec, "httpOnly");
> +    const Identifier secureIdentifier = Identifier(exec, "secure");
> +    const Identifier sessionIdentifier = Identifier(exec, "session");

You can use construction syntax instead of assignment for these. Also, no real
reason to make them const. After all, almost any local variable in most
programs could be marked const, and for brevity we normally don't do it unless
there's a special reason to.

    Identifier nameIdentifier(exec, "name");

and so on.

> +        const String name;
> +        const String value;
> +        const String domain;
> +        const String path;
> +        const String expires;
> +        const bool httpOnly;
> +        const bool secure;
> +        const bool session;
> +        const unsigned size;

Making all these data members const isn't really all that good an idea. If
someone wants to use a Cookie in some non-const way, perhaps modifying one of
these, then declaring it this way would get in the way, for no good reason. On
the other hand, clients who don't want to modify a Cookie object get the same
effect you got here by just saying "const Cookie". I'd leave off all the const
on the individual members.

> +void getRawCookies(const Document*, const KURL& url, Vector<Cookie>& rawCookies)
> +{
> +    rawCookies.clear();
> +
> +    NSURL *cookieURL = url;
> +    NSArray *cookies = [[NSHTTPCookieStorage sharedHTTPCookieStorage] cookiesForURL:cookieURL];
> +
> +    NSUInteger count = [cookies count];

For best memory use and performance, you will want to do this here:


This allocates the memory for the vector rather than sizing it up each time you
hit the limit while appending. It's faster and saves a bit on extra storage.
Once you've done that change, it is then safe to use the slightly faster
uncheckedAppend function below inside the loop instead of append. But even if
you chose not to do uncheckedAppend, reserveCapacity is a good idea.

> +    for (NSUInteger i = 0; i < count; ++i) {
> +        NSHTTPCookie *cookie = (NSHTTPCookie *)[cookies objectAtIndex:i];
> +        NSString *name = [cookie name];
> +        NSString *value = [cookie value];
> +        NSString *domain = [cookie domain];
> +        NSString *path = [cookie path];
> +        NSString *expires = [[cookie expiresDate] description];
> +        bool httpOnly = [cookie isHTTPOnly];
> +        bool secure = [cookie isSecure];
> +        bool session = [cookie isSessionOnly];
> +        Cookie cookieData = Cookie(name, value, domain, path, expires, httpOnly, secure, session);

Again could use construction syntax here:

    Cookie cookieData(name, value, domain, path, expires, httpOnly, secure,

> +        rawCookies.append(cookieData);

Or you could construct right in the append function call:

    rawCookies.uncheckedAppend(Cookie(name, value, domain, path, expires,
httpOnly, secure, session));

Otherwise this looks really great.

None of my comments are critical, so I'll say r=me on this version, but feel
free to do a revised version if you like some of the things I propose above.

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list