Bug 31627 - Web Inspector: Store cookie domains in the WebInspector object.
Summary: Web Inspector: Store cookie domains in the WebInspector object.
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2009-11-18 05:47 PST by Alexander Pavlov (apavlov)
Modified: 2009-11-19 05:03 PST (History)
9 users (show)

See Also:

patch (7.65 KB, patch)
2009-11-18 08:35 PST, Alexander Pavlov (apavlov)
timothy: review-
Details | Formatted Diff | Diff
patch (fixed) (7.67 KB, patch)
2009-11-18 09:57 PST, Alexander Pavlov (apavlov)
timothy: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
patch (fix mac build) (7.67 KB, patch)
2009-11-19 03:51 PST, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Pavlov (apavlov) 2009-11-18 05:47:55 PST
Currently cookie domains are pushed from InspectorController and stored in the Storage panel. Since other clients may need the cookie domain list, it should be stored in the WebInspector object.
Comment 1 Alexander Pavlov (apavlov) 2009-11-18 08:35:17 PST
Created attachment 43435 [details]
Comment 2 Timothy Hatcher 2009-11-18 09:26:40 PST
Comment on attachment 43435 [details]

> +    var match = payload.documentURL.match(/^(?:http[s]?|file):\/\/([\/]*[^\/]+)/);

I am curious if this catches the right "domain" for file URLs.

> +    if (typeof this.cookieDomains[domain] !== "undefined")

A better way to do this is:

    if (domain in this.cookieDomains)

This change would mean Cookies would not be available if the Resources panel ins't enabled. That seems confusing and limiting. But otherwise I like the clean approch this has over the old code…
Comment 3 Timothy Hatcher 2009-11-18 09:28:34 PST
Comment on attachment 43435 [details]

I'd like to understand how this works when the Resources panel is disabled before it gets r+ed.
Comment 4 Alexander Pavlov (apavlov) 2009-11-18 09:57:34 PST
Created attachment 43438 [details]
patch (fixed)

Fixed the "Local Files" issue and the domain existence check.
Comment 5 WebKit Commit Bot 2009-11-18 14:51:35 PST
Comment on attachment 43438 [details]
patch (fixed)

Rejecting patch 43438 from commit-queue.

Failed to run "WebKitTools/Scripts/build-webkit" exit_code: 1
Last 500 characters of output:
cts/CommitQueue/WebCore/inspector/TimelineRecordFactory.cpp -o /Users/eseidel/Projects/build/WebCore.build/Release/WebCore.build/Objects-normal/i386/TimelineRecordFactory.o

The following build commands failed:
	Distributed-CompileC /Users/eseidel/Projects/build/WebCore.build/Release/WebCore.build/Objects-normal/i386/InspectorResource.o /Users/eseidel/Projects/CommitQueue/WebCore/inspector/InspectorResource.cpp normal i386 c++ com.apple.compilers.gcc.4_2
(1 failure)
Comment 6 Eric Seidel (no email) 2009-11-18 15:01:47 PST
Sorry to be so slow about getting to the patch. The commit-queue was down for the last 5 days due to bug 31615.  It's back now.  If you post an updated patch which builds properly on mac this will be landed promptly.
Comment 7 Alexander Pavlov (apavlov) 2009-11-19 03:51:49 PST
Created attachment 43494 [details]
patch (fix mac build)
Comment 8 WebKit Commit Bot 2009-11-19 05:03:19 PST
Comment on attachment 43494 [details]
patch (fix mac build)

Clearing flags on attachment: 43494

Committed r51182: <http://trac.webkit.org/changeset/51182>
Comment 9 WebKit Commit Bot 2009-11-19 05:03:29 PST
All reviewed patches have been landed.  Closing bug.