| Summary: | Web Inspector: WebSocket in Resources tab | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Yuta Kitamura <yutak> |
| Component: | Web Inspector (Deprecated) | Assignee: | Yuta Kitamura <yutak> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | abarth, ap, bweinstein, caseq, commit-queue, eric, hamaji, joepeck, keishi, pfeldman, pmuellr, rik, timothy, tkent, ukai, victorw, webkit.review.bot, yurys |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | All | ||
| OS: | All | ||
| Bug Depends on: | 38728, 39864, 41661 | ||
| Bug Blocks: | 34565 | ||
| Attachments: | |||
|
Description
Yuta Kitamura
2010-06-17 03:00:53 PDT
Created attachment 58976 [details]
Screenshot of prototype v1
Here is a screenshot of my first try.
There are a few rough edges:
- We need to decide the color of the bar chart (line 466 of inspector.js, see below), and create an image (front-end/Images/timelineBar???.png).
scripts: new WebInspector.ResourceCategory("scripts", WebInspector.UIString("Scripts"), "rgb(255,121,0)"),
xhr: new WebInspector.ResourceCategory("xhr", WebInspector.UIString("XHR"), "rgb(231,231,10)"),
fonts: new WebInspector.ResourceCategory("fonts", WebInspector.UIString("Fonts"), "rgb(255,82,62)"),
+ websocket: new WebInspector.ResourceCategory("websockets", WebInspector.UIString("WebSocket"), "rgb(???,???,???)"),
other: new WebInspector.ResourceCategory("other", WebInspector.UIString("Other"), "rgb(186,186,186)")
};
I'm wondering if there's anybody responsible for these colors.
- We need to consider how to represent extra challenge/response field of WebSocket handshake. In my (tentative) implementation they are shown as "(Key3)" and "(Challenge Response)" in Request/Response headers list.
Suggestions and comments are highly appreciated. Thanks.
(In reply to comment #1) > Created an attachment (id=58976) [details] > Screenshot of prototype v1 > - We need to consider how to represent extra challenge/response field of WebSocket handshake. In my (tentative) implementation they are shown as "(Key3)" and "(Challenge Response)" in Request/Response headers list. I think (Key3) and (Challenge Response) would be fine. (Reply) for (Challenge Response) might be ok, because websocket spec says it's /reply/ (4.1 44 in http://www.whatwg.org/specs/web-socket-protocol/ ) Is there anybody who can create a new bar image (front-end/Images/timelinePill<new color name>.png) for WebSocket? (In reply to comment #3) > Is there anybody who can create a new bar image (front-end/Images/timelinePill<new color name>.png) for WebSocket? timelineHollowPill<new color name>.png, too. (In reply to comment #4) > (In reply to comment #3) > > Is there anybody who can create a new bar image (front-end/Images/timelinePill<new color name>.png) for WebSocket? > > timelineHollowPill<new color name>.png, too. You should reuse the one for 'loading' instead (blue). (In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > Is there anybody who can create a new bar image (front-end/Images/timelinePill<new color name>.png) for WebSocket? > > > > timelineHollowPill<new color name>.png, too. > > You should reuse the one for 'loading' instead (blue). Oops, I should have mentioned that these images will be used for timeline bars in Resources tab, not in Timeline tab. Let me give some more context. There are seven predefined colors for each resource type: - Documents: Blue - Stylesheets: Stylesheets - Images: Purple - Scripts: Orange - XHR: Yellow - Fonts: Red - Other: Gray I want to add "WebSocket resource type", but apparently timeline bars in Resources tab are drawn with images under front-end/Images directory, namely timelinePill*.png and timelineHollowPill*.png. These colors should match ones defined in line 460 of front-end/inspector.js. As I don't have enough image-hacking skills to create a nice shiny image, I'd like to find someone to create one. If there's nobody found, I'd like to try to do it myself, but mine might not be so cool :p Created attachment 59642 [details]
Patch
So far I don't decide a bar color of WebSocket resources, because I didn't create a bar image so far. WebSocket resources are shown gray. I don't have an idea of how to create a new bar image aligned with the existing ones... Comment on attachment 59642 [details]
Patch
Looks sane. Please find a number of nits below. I don't like the way m_frame and m_loader are now nullable, but maybe it is Ok for now.
WebCore/inspector/InspectorController.cpp:1441
+ void InspectorController::didCreateWebSocket(unsigned long identifier, const KURL& url)
Do we really need to distinguish between didCreateWebSocket and willSendWebSocketHandshake? Can socket lag in between of those on its own or these are just corresponding js calls?
WebCore/inspector/InspectorController.cpp:1470
+ // FIXME: Record timing?
I think you should mark response received time here.
WebCore/inspector/InspectorResource.cpp:199
+ jsonObject.set("documentURL", m_requestURL.string());
Not sure this is entirely correct. Frameless resource should not reference any document.
WebCore/inspector/InspectorResource.cpp:327
+ if (!m_loader)
I think this should be based on Key challenge, not the loader existence.
WebCore/websockets/WebSocketChannel.cpp:370
+ m_identifierAssigned = true;
Isn't it equivalent to m_identifier != 0?
WebCore/inspector/front-end/ResourceView.js:283
+ headers.push({header: "(Key3)", value: this.resource.webSocketRequestKey3});
This formally is not a header. We do have special treatment of form parameters and such. This key could have dedicated place in UI too, no need for hack.
WebCore/inspector/front-end/ResourceView.js:292
+ headers.push({header: "(Challenge Response)", value: this.resource.webSocketChallengeResponse});
Ditto.
Thank you for your review! (In reply to comment #9) > (From update of attachment 59642 [details]) > Looks sane. Please find a number of nits below. I don't like the way m_frame and m_loader are now nullable, but maybe it is Ok for now. Nullable loader and frame were actually my concern, too, but I could not think of a good resolution. Maybe we need to create an abstract layer of inspector resources? I don't have a concrete idea for now. > > WebCore/inspector/InspectorController.cpp:1441 > + void InspectorController::didCreateWebSocket(unsigned long identifier, const KURL& url) > Do we really need to distinguish between didCreateWebSocket and willSendWebSocketHandshake? Can socket lag in between of those on its own or these are just corresponding js calls? Between didCreateWebSocket and willSendWebSocketHandshake, the browser tries to establish a socket connection. Thus willSendWebSocketHandshake may not happen if we cannot connect to the server (e.g. connection refused). I want to see a resource line even if the connecting attempt is failed, so I guess the two functions should be separated. > > WebCore/inspector/InspectorController.cpp:1470 > + // FIXME: Record timing? > I think you should mark response received time here. Hmm, I tried it in fact, but the timing chart was not shown in a way I intended. The bar timing is calculated as follows: - latency (light-colored bar) = responseReceived time - start time - download (solid-colored bar) = end time - responseReceived time I didn't think this formula fit this case. I thought I had to tweak the bar representation. That is why I did not mark response received time here. Perhaps I have to add a new timing type for WebSocket? > > WebCore/inspector/InspectorResource.cpp:199 > + jsonObject.set("documentURL", m_requestURL.string()); > Not sure this is entirely correct. Frameless resource should not reference any document. I was not sure about the difference between "url" and "documentURL". Could you elaborate more? > > WebCore/inspector/InspectorResource.cpp:327 > + if (!m_loader) > I think this should be based on Key challenge, not the loader existence. Challenge key is not set until the handshake is sent. Probably it's good to add a flag that identifies the WebSocket resource type. > > WebCore/websockets/WebSocketChannel.cpp:370 > + m_identifierAssigned = true; > Isn't it equivalent to m_identifier != 0? I didn't know 0 is not a valid identifier. Will fix. > > > WebCore/inspector/front-end/ResourceView.js:283 > + headers.push({header: "(Key3)", value: this.resource.webSocketRequestKey3}); > This formally is not a header. We do have special treatment of form parameters and such. This key could have dedicated place in UI too, no need for hack. > > WebCore/inspector/front-end/ResourceView.js:292 > + headers.push({header: "(Challenge Response)", value: this.resource.webSocketChallengeResponse}); > Ditto. Sure, I want to fix. Created attachment 60503 [details]
Patch v2 (Updated as per Pavel's comments)
Comment on attachment 60503 [details]
Patch v2 (Updated as per Pavel's comments)
Looks good. One thing before we land it: I still don't like the way it forks InspectorResource. What you were suggesting (extracting the common part into a superclass) sounds like a good solution to me. We could do it later, but it just might be easier to do it now, before the HTTP and WebSocket concepts are mixed in the InspectorResource class. I also foresee InspectorWebSocketResource getting more functionality InspectorResource does not have (like having chunks of data available). The complex thing is that this hierarchy needs to exist both in naive and js layers. Would you sign up for such a refactoring?
WebCore/inspector/InspectorResource.h:110
+ KURL documentURL() const { return m_documentURL; }
Does not seem to be used.
Thank you for your review. (In reply to comment #12) > (From update of attachment 60503 [details]) > Looks good. One thing before we land it: I still don't like the way it forks InspectorResource. What you were suggesting (extracting the common part into a superclass) sounds like a good solution to me. We could do it later, but it just might be easier to do it now, before the HTTP and WebSocket concepts are mixed in the InspectorResource class. I also foresee InspectorWebSocketResource getting more functionality InspectorResource does not have (like having chunks of data available). The complex thing is that this hierarchy needs to exist both in naive and js layers. Would you sign up for such a refactoring? Sure, I'm happy to decouple InspectorResource and do the refactoring. I want to start working on it after I complete WebSocket support in Inspector's Timeline panel (bug 38831), which I'm preparing a patch for. > > WebCore/inspector/InspectorResource.h:110 > + KURL documentURL() const { return m_documentURL; } > Does not seem to be used. Will be removed. Created attachment 60592 [details]
Patch v3 (Remove documentURL function)
Comment on attachment 60592 [details]
Patch v3 (Remove documentURL function)
I will land.
Committed r62529: <http://trac.webkit.org/changeset/62529> http://trac.webkit.org/changeset/62529 might have broken Qt Linux Release The commit was reverted since it broke http/tests/inspector/resource-har-conversion.html. I'm looking into it. Created attachment 60620 [details]
Patch v4 (Fix documentURL regression)
It would be nice if you describe how you fixed the regression on this bug. (In reply to comment #20) > It would be nice if you describe how you fixed the regression on this bug. The regression was happened because in InspectorResource::create() loader->frame()->document()->url() may be inaccurate (may point to the previous page). To fix this, I added documentURL() function that lazily obtains the document URL. The following is my fix: diff --git a/WebCore/inspector/InspectorResource.cpp b/WebCore/inspector/InspectorResource.cpp index 5157a53..6dd00e0 100644 --- a/WebCore/inspector/InspectorResource.cpp +++ b/WebCore/inspector/InspectorResource.cpp @@ -115,7 +115,9 @@ PassRefPtr<InspectorResource> InspectorResource::appendRedirect(unsigned long id PassRefPtr<InspectorResource> InspectorResource::create(unsigned long identifier, DocumentLoader* loader, const KURL& requestURL) { ASSERT(loader); - return adoptRef(new InspectorResource(identifier, loader, requestURL, loader->frame()->document()->url())); + // We cannot set documentURL here, because loader->frame()->document()->url() may not be accurate at this moment. + // It will be fetched in documentURL() method when it becomes necessary. + return adoptRef(new InspectorResource(identifier, loader, requestURL, KURL())); } PassRefPtr<InspectorResource> InspectorResource::createCached(unsigned long identifier, DocumentLoader* loader, const CachedResource* cachedResource) @@ -212,7 +214,7 @@ void InspectorResource::updateScriptObject(InspectorFrontend* frontend) ScriptObject jsonObject = frontend->newScriptObject(); if (m_changes.hasChange(RequestChange)) { jsonObject.set("url", m_requestURL.string()); - jsonObject.set("documentURL", m_documentURL.string()); + jsonObject.set("documentURL", documentURL().string()); jsonObject.set("host", m_requestURL.host()); jsonObject.set("path", m_requestURL.path()); jsonObject.set("lastPathComponent", m_requestURL.lastPathComponent()); @@ -408,6 +410,16 @@ PassRefPtr<SharedBuffer> InspectorResource::resourceData(String* textEncodingNam return cachedResource->data(); } +KURL InspectorResource::documentURL() +{ + if (!m_documentURL.isNull()) + return m_documentURL; + + ASSERT(m_frame); + m_documentURL = m_frame->document()->url(); + return m_documentURL; +} + void InspectorResource::startTiming() { m_startTime = currentTime(); diff --git a/WebCore/inspector/InspectorResource.h b/WebCore/inspector/InspectorResource.h index 26110f2..34aa2d9 100644 --- a/WebCore/inspector/InspectorResource.h +++ b/WebCore/inspector/InspectorResource.h @@ -107,6 +107,7 @@ namespace WebCore { void markMainResource() { m_isMainResource = true; } unsigned long identifier() const { return m_identifier; } KURL requestURL() const { return m_requestURL; } + KURL documentURL(); Frame* frame() const { return m_frame.get(); } const String& mimeType() const { return m_mimeType; } const HTTPHeaderMap& requestHeaderFields() const { return m_requestHeaderFields; } Comment on attachment 60620 [details]
Patch v4 (Fix documentURL regression)
WebCore/inspector/InspectorResource.cpp:415
+ if (!m_documentURL.isNull())
This logic looks unclear. I don't think you should initialize m_documentURL in constructor given that majority of cases it will initialize with empty URL. I'd just assign to it when you know information is available: in InspectorResource::updateRequest for generic resources and in InspectorResource::createWebSocket for web sockets. r- for that.
Otherwise, looks good.
Created attachment 60702 [details] Patch v5 (Updated as per comment #22) Comment on attachment 60702 [details] Patch v5 (Updated as per comment #22) Rejecting patch 60702 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--ignore-tests', 'compositing', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 19294 test cases. http/tests/inspector/resource-har-conversion.html -> failed Exiting early after 1 failures. 18452 tests run. 526.58s total testing time 18451 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 12 test cases (<1%) had stderr output Full output: http://webkit-commit-queue.appspot.com/results/3358472 :( Sounds like updateRequest() is also too early? As far as I tested, updateRequest() did not seem to be the right place to call m_frame->document()->url(). updateScriptObject() seemed safe. I don't know why :( Comment on attachment 60702 [details] Patch v5 (Updated as per comment #22) Not sure why it failed. Does it pass locally? Anyways, now that it is again not landed I have a chance to comment on it a bit more. WebCore/inspector/InspectorResource.h:164 + InspectorResource(unsigned long identifier, DocumentLoader*, const KURL& requestURL, const KURL& documentURL); I thought we agreed that this parameter should not go to the constructor (see previous review comments). WebCore/inspector/InspectorResource.h:110 + KURL documentURL(); Seems to be of no use. WebCore/inspector/InspectorResource.cpp:118 + // We cannot set documentURL here, because loader->frame()->document()->url() may not be accurate at this moment. If you remove extra constructor argument, there will be no need in this comment. WebCore/inspector/InspectorResource.cpp:145 + RefPtr<InspectorResource> resource(new InspectorResource(identifier, 0, requestURL, documentURL)); Add m_documentURL = documentURL below instead. Apologies for delay in response. (In reply to comment #27) > (From update of attachment 60702 [details]) > Not sure why it failed. Does it pass locally? Actually it does not. I've figured out the cause: - InspectorResource::updateRequest() is called during willSendRequest callback, but m_frame->document() is not ready at this point. It points to the URL of the previous page. - Frame::setURL() is called (indirectly) by FrameLoader::receivedFirstData(), and we need to obtain the documentURL after this point. I think the only way to resolve this situation is to obtain documentURL in InspectorResource::updateScriptObject(). If you have an alternative idea, I appreciate it. I want to fix other issues you pointed out in comment #27. Created attachment 62556 [details]
Patch v6 (Fix regression, this time for sure)
Comment on attachment 62556 [details] Patch v6 (Fix regression, this time for sure) Clearing flags on attachment: 62556 Committed r64104: <http://trac.webkit.org/changeset/64104> All reviewed patches have been landed. Closing bug. Reverted r64104 for reason: The patch causes chromium webkit socket laytest crashes on windows randomly Committed r64142: <http://trac.webkit.org/changeset/64142> (In reply to comment #32) > Reverted r64104 for reason: > > The patch causes chromium webkit socket laytest crashes on windows randomly > > Committed r64142: <http://trac.webkit.org/changeset/64142> I've been investigating this, but I really could not reproduce these crashes, and there were no useful stack trace available. I suspect they were caused by Chromium's bot issue (Incredibuild sometimes produces a corrupt binary). http://groups.google.com/a/chromium.org/group/chromium-dev/browse_thread/thread/8e831aec217eab90/dd397c598f8e8139 Thus, I want to check in my code again, and see if my code really cause crashes. I will shortly upload another patch for review that contains a few changes to follow recent changes in web inspector. There's no change in functionality. To WebKit gardeners: If you see crashes under websocket/ tests after WebKit roll , please try to clobber builders before reverting my commit. If the crashes are real, feel free to revert my change. Created attachment 65532 [details]
Patch v7 (Updated for inspector changes)
(In reply to comment #34) > Created an attachment (id=65532) [details] > Patch v7 (Updated for inspector changes) I'll land this change next week, after Chromium M7 branch is cut. Comment on attachment 65532 [details] Patch v7 (Updated for inspector changes) > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > index 90f722c24dba7744f34e4645233c077060bcf63f..29bdc4b09a26e081f781908380b1bf4b4787c781 100644 > --- a/WebCore/ChangeLog > +++ b/WebCore/ChangeLog > @@ -1,3 +1,72 @@ > +2010-08-26 Yuta Kitamura <yutak@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Add WebSocket resource type to Web Inspector. > + > + When a new WebSocket connection is established, a line for that connection > + will appear in Web Inspector's Resources tab. If the resource name is > + clicked, the details of handshake request and response will be shown. > + > + Web Inspector: WebSocket in Resources tab > + https://bugs.webkit.org/show_bug.cgi?id=40768 > + > + * inspector/InspectorController.cpp: > + (WebCore::InspectorController::addResource): WebSocket resource does not > + have an associated loader, thus frame might be null. Need to check it. > + (WebCore::InspectorController::removeResource): Ditto. > + (WebCore::InspectorController::didCreateWebSocket): > + (WebCore::InspectorController::willSendWebSocketHandshakeRequest): > + (WebCore::InspectorController::didReceiveWebSocketHandshakeResponse): > + (WebCore::InspectorController::didCloseWebSocket): > + * inspector/InspectorController.h: > + * inspector/InspectorResource.cpp: Add null checks of m_loader and m_frame, > + because WebSocket does not have a loader and we need to allow null for > + these variables. > + (WebCore::createReadableStringFromBinary): > + (WebCore::InspectorResource::InspectorResource): > + (WebCore::InspectorResource::create): Factory function of > + regular (non-WebSocket) resources. > + (WebCore::InspectorResource::createWebSocket): Factory function of > + WebSocket resources. > + (WebCore::InspectorResource::updateWebSocketRequest): > + (WebCore::InspectorResource::updateWebSocketResponse): > + (WebCore::InspectorResource::updateScriptObject): m_frame->document() becomes > + available when Frame::setDocument() is called. We cannot obtain documentURL > + during the constructor or updateRequest() function, because m_frame->document() > + is not available yet at that point and documentURL will contain a wrong URL. > + As far as I know, updateScriptObject() is the only place where we can safely > + obtain documentURL. > + (WebCore::InspectorResource::cachedResource): > + (WebCore::InspectorResource::type): > + (WebCore::InspectorResource::resourceData): > + * inspector/InspectorResource.h: > + (WebCore::InspectorResource::): > + (WebCore::InspectorResource::markWebSocket): > + * inspector/front-end/Resource.js: > + (WebInspector.Resource.Type.toString): > + (WebInspector.Resource.prototype.set type): > + (WebInspector.Resource.prototype._mimeTypeIsConsistentWithType): > + * inspector/front-end/ResourceView.js: > + (WebInspector.ResourceView.prototype._refreshRequestHeaders): > + (WebInspector.ResourceView.prototype._refreshResponseHeaders): > + (WebInspector.ResourceView.prototype._refreshHeaders): > + * inspector/front-end/inspector.css: > + (.resources-category-websockets, .resources-category-other): > + (.resources-category-websockets .resources-graph-bar): > + (.resources-category-websockets.resource-cached .resources-graph-bar): > + * inspector/front-end/inspector.js: > + (WebInspector.loaded): > + (WebInspector.updateResource): > + * websockets/WebSocketChannel.cpp: > + (WebCore::WebSocketChannel::WebSocketChannel): > + (WebCore::WebSocketChannel::disconnect): > + (WebCore::WebSocketChannel::didOpen): > + (WebCore::WebSocketChannel::didClose): > + (WebCore::WebSocketChannel::processBuffer): > + (WebCore::WebSocketChannel::identifier): > + * websockets/WebSocketChannel.h: > + > 2010-08-25 Sheriff Bot <webkit.review.bot@gmail.com> > > Unreviewed, rolling out r66074. > diff --git a/WebCore/inspector/InspectorController.cpp b/WebCore/inspector/InspectorController.cpp > index f47b32174967ce6faff4f008a2df613f9b925272..1a47571e329e256edb7f3cf3fe83accf13f73791 100644 > --- a/WebCore/inspector/InspectorController.cpp > +++ b/WebCore/inspector/InspectorController.cpp > @@ -798,6 +798,8 @@ void InspectorController::addResource(InspectorResource* resource) > m_knownResources.add(resource->requestURL()); > > Frame* frame = resource->frame(); > + if (!frame) > + return; > ResourcesMap* resourceMap = m_frameResources.get(frame); > if (resourceMap) > resourceMap->set(resource->identifier(), resource); > @@ -816,6 +818,8 @@ void InspectorController::removeResource(InspectorResource* resource) > m_knownResources.remove(requestURL); > > Frame* frame = resource->frame(); > + if (!frame) > + return; > ResourcesMap* resourceMap = m_frameResources.get(frame); > if (!resourceMap) { > ASSERT_NOT_REACHED(); > @@ -1442,6 +1446,56 @@ InspectorDOMStorageResource* InspectorController::getDOMStorageResourceForId(lon > } > #endif > > +#if ENABLE(WEB_SOCKETS) > +void InspectorController::didCreateWebSocket(unsigned long identifier, const KURL& requestURL, const KURL& documentURL) > +{ > + if (!enabled()) > + return; > + ASSERT(m_inspectedPage); > + > + RefPtr<InspectorResource> resource = InspectorResource::createWebSocket(identifier, requestURL, documentURL); > + addResource(resource.get()); > + > + if (m_frontend) > + resource->updateScriptObject(m_frontend.get()); > +} > + > +void InspectorController::willSendWebSocketHandshakeRequest(unsigned long identifier, const WebSocketHandshakeRequest& request) > +{ > + RefPtr<InspectorResource> resource = getTrackedResource(identifier); > + if (!resource) > + return; > + resource->startTiming(); > + resource->updateWebSocketRequest(request); > + if (m_frontend) > + resource->updateScriptObject(m_frontend.get()); > +} > + > +void InspectorController::didReceiveWebSocketHandshakeResponse(unsigned long identifier, const WebSocketHandshakeResponse& response) > +{ > + RefPtr<InspectorResource> resource = getTrackedResource(identifier); > + if (!resource) > + return; > + // Calling resource->markResponseReceivedTime() here makes resources bar chart confusing, because > + // we cannot apply the "latency + download" model of regular resources to WebSocket connections. > + // FIXME: Design a new UI for bar charts of WebSocket resources, and record timing here. > + resource->updateWebSocketResponse(response); > + if (m_frontend) > + resource->updateScriptObject(m_frontend.get()); > +} > + > +void InspectorController::didCloseWebSocket(unsigned long identifier) > +{ > + RefPtr<InspectorResource> resource = getTrackedResource(identifier); > + if (!resource) > + return; > + > + resource->endTiming(); > + if (m_frontend) > + resource->updateScriptObject(m_frontend.get()); > +} > +#endif // ENABLE(WEB_SOCKETS) > + > #if ENABLE(JAVASCRIPT_DEBUGGER) > void InspectorController::addProfile(PassRefPtr<ScriptProfile> prpProfile, unsigned lineNumber, const String& sourceURL) > { > diff --git a/WebCore/inspector/InspectorController.h b/WebCore/inspector/InspectorController.h > index 7ed25490de2e7af1c7646e739cf5c49fcf8abd1b..43974ab427c98ef66213edeccb4ecf35faeeff04 100644 > --- a/WebCore/inspector/InspectorController.h > +++ b/WebCore/inspector/InspectorController.h > @@ -87,6 +87,11 @@ class StorageArea; > class InspectorApplicationCacheAgent; > #endif > > +#if ENABLE(WEB_SOCKETS) > +class WebSocketHandshakeRequest; > +class WebSocketHandshakeResponse; > +#endif > + > class InspectorController : public Noncopyable { > public: > typedef HashMap<unsigned long, RefPtr<InspectorResource> > ResourcesMap; > @@ -204,6 +209,12 @@ public: > void setDOMStorageItem(long storageId, const String& key, const String& value, bool* success); > void removeDOMStorageItem(long storageId, const String& key, bool* success); > #endif > +#if ENABLE(WEB_SOCKETS) > + void didCreateWebSocket(unsigned long identifier, const KURL& requestURL, const KURL& documentURL); > + void willSendWebSocketHandshakeRequest(unsigned long identifier, const WebSocketHandshakeRequest&); > + void didReceiveWebSocketHandshakeResponse(unsigned long identifier, const WebSocketHandshakeResponse&); > + void didCloseWebSocket(unsigned long identifier); > +#endif > > const ResourcesMap& resources() const { return m_resources; } > InspectorResource* resourceForURL(const String& url); > diff --git a/WebCore/inspector/InspectorResource.cpp b/WebCore/inspector/InspectorResource.cpp > index ed07339e6a1301eea0fdbe1901086d9ad92a678a..342ad7904ced429754342aa9f78243aa5a1c1527 100644 > --- a/WebCore/inspector/InspectorResource.cpp > +++ b/WebCore/inspector/InspectorResource.cpp > @@ -43,14 +43,37 @@ > #include "ResourceLoadTiming.h" > #include "ResourceRequest.h" > #include "ResourceResponse.h" > +#include "StringBuffer.h" > #include "TextEncoding.h" > +#include "WebSocketHandshakeRequest.h" > +#include "WebSocketHandshakeResponse.h" > + > +#include <wtf/Assertions.h> > > namespace WebCore { > > +// Create human-readable binary representation, like "01:23:45:67:89:AB:CD:EF". > +static String createReadableStringFromBinary(const unsigned char* value, size_t length) > +{ > + ASSERT(length > 0); > + static const char hexDigits[17] = "0123456789ABCDEF"; > + size_t bufferSize = length * 3 - 1; > + StringBuffer buffer(bufferSize); > + size_t index = 0; > + for (size_t i = 0; i < length; ++i) { > + if (i > 0) > + buffer[index++] = ':'; > + buffer[index++] = hexDigits[value[i] >> 4]; > + buffer[index++] = hexDigits[value[i] & 0xF]; > + } > + ASSERT(index == bufferSize); > + return String::adopt(buffer); > +} > + > InspectorResource::InspectorResource(unsigned long identifier, DocumentLoader* loader, const KURL& requestURL) > : m_identifier(identifier) > , m_loader(loader) > - , m_frame(loader->frame()) > + , m_frame(loader ? loader->frame() : 0) > , m_requestURL(requestURL) > , m_expectedContentLength(0) > , m_cached(false) > @@ -66,6 +89,9 @@ InspectorResource::InspectorResource(unsigned long identifier, DocumentLoader* l > , m_connectionID(0) > , m_connectionReused(false) > , m_isMainResource(false) > +#if ENABLE(WEB_SOCKETS) > + , m_isWebSocket(false) > +#endif > { > } > > @@ -88,6 +114,12 @@ PassRefPtr<InspectorResource> InspectorResource::appendRedirect(unsigned long id > return redirect; > } > > +PassRefPtr<InspectorResource> InspectorResource::create(unsigned long identifier, DocumentLoader* loader, const KURL& requestURL) > +{ > + ASSERT(loader); > + return adoptRef(new InspectorResource(identifier, loader, requestURL)); > +} > + > PassRefPtr<InspectorResource> InspectorResource::createCached(unsigned long identifier, DocumentLoader* loader, const CachedResource* cachedResource) > { > PassRefPtr<InspectorResource> resource = create(identifier, loader, KURL(ParsedURLString, cachedResource->url())); > @@ -107,6 +139,16 @@ PassRefPtr<InspectorResource> InspectorResource::createCached(unsigned long iden > return resource; > } > > +#if ENABLE(WEB_SOCKETS) > +PassRefPtr<InspectorResource> InspectorResource::createWebSocket(unsigned long identifier, const KURL& requestURL, const KURL& documentURL) > +{ > + RefPtr<InspectorResource> resource = adoptRef(new InspectorResource(identifier, 0, requestURL)); > + resource->markWebSocket(); > + resource->m_documentURL = documentURL; > + return resource.release(); > +} > +#endif > + > void InspectorResource::updateRequest(const ResourceRequest& request) > { > m_requestHeaderFields = request.httpHeaderFields(); > @@ -146,6 +188,27 @@ void InspectorResource::updateResponse(const ResourceResponse& response) > m_changes.set(TypeChange); > } > > +#if ENABLE(WEB_SOCKETS) > +void InspectorResource::updateWebSocketRequest(const WebSocketHandshakeRequest& request) > +{ > + m_requestHeaderFields = request.headerFields(); > + m_requestMethod = "GET"; // Currently we always use "GET" to request handshake. > + m_webSocketRequestKey3.set(new WebSocketHandshakeRequest::Key3(request.key3())); > + m_changes.set(RequestChange); > + m_changes.set(TypeChange); > +} > + > +void InspectorResource::updateWebSocketResponse(const WebSocketHandshakeResponse& response) > +{ > + m_responseStatusCode = response.statusCode(); > + m_responseStatusText = response.statusText(); > + m_responseHeaderFields = response.headerFields(); > + m_webSocketChallengeResponse.set(new WebSocketHandshakeResponse::ChallengeResponse(response.challengeResponse())); > + m_changes.set(ResponseChange); > + m_changes.set(TypeChange); > +} > +#endif // ENABLE(WEB_SOCKETS) > + > static PassRefPtr<InspectorObject> buildHeadersObject(const HTTPHeaderMap& headers) > { > RefPtr<InspectorObject> object = InspectorObject::create(); > @@ -183,8 +246,10 @@ void InspectorResource::updateScriptObject(InspectorFrontend* frontend) > RefPtr<InspectorObject> jsonObject = InspectorObject::create(); > jsonObject->setNumber("id", m_identifier); > if (m_changes.hasChange(RequestChange)) { > + if (m_frame) > + m_documentURL = m_frame->document()->url(); > jsonObject->setString("url", m_requestURL.string()); > - jsonObject->setString("documentURL", m_frame->document()->url().string()); > + jsonObject->setString("documentURL", m_documentURL.string()); > jsonObject->setString("host", m_requestURL.host()); > jsonObject->setString("path", m_requestURL.path()); > jsonObject->setString("lastPathComponent", m_requestURL.lastPathComponent()); > @@ -194,6 +259,10 @@ void InspectorResource::updateScriptObject(InspectorFrontend* frontend) > jsonObject->setString("requestMethod", m_requestMethod); > jsonObject->setString("requestFormData", m_requestFormData); > jsonObject->setBool("didRequestChange", true); > +#if ENABLE(WEB_SOCKETS) > + if (m_webSocketRequestKey3) > + jsonObject->setString("webSocketRequestKey3", createReadableStringFromBinary(m_webSocketRequestKey3->value, sizeof(m_webSocketRequestKey3->value))); > +#endif > } > > if (m_changes.hasChange(ResponseChange)) { > @@ -209,6 +278,10 @@ void InspectorResource::updateScriptObject(InspectorFrontend* frontend) > jsonObject->setBool("cached", m_cached); > if (m_loadTiming && !m_cached) > jsonObject->setObject("timing", buildObjectForTiming(m_loadTiming.get())); > +#if ENABLE(WEB_SOCKETS) > + if (m_webSocketChallengeResponse) > + jsonObject->setString("webSocketChallengeResponse", createReadableStringFromBinary(m_webSocketChallengeResponse->value, sizeof(m_webSocketChallengeResponse->value))); > +#endif > jsonObject->setBool("didResponseChange", true); > } > > @@ -267,6 +340,8 @@ CachedResource* InspectorResource::cachedResource() const > // Try hard to find a corresponding CachedResource. During preloading, DocLoader may not have the resource in document resources set yet, > // but Inspector will already try to fetch data that is only available via CachedResource (and it won't update once the resource is added, > // because m_changes will not have the appropriate bits set). > + if (!m_frame) > + return 0; > const String& url = m_requestURL.string(); > CachedResource* cachedResource = m_frame->document()->docLoader()->cachedResource(url); > if (!cachedResource) > @@ -303,6 +378,12 @@ InspectorResource::Type InspectorResource::type() const > if (!m_overrideContent.isNull()) > return m_overrideContentType; > > +#if ENABLE(WEB_SOCKETS) > + if (m_isWebSocket) > + return WebSocket; > +#endif > + > + ASSERT(m_loader); > if (equalIgnoringFragmentIdentifier(m_requestURL, m_loader->requestURL())) { > InspectorResource::Type resourceType = cachedResourceType(); > if (resourceType == Other) > @@ -342,7 +423,7 @@ String InspectorResource::sourceString() const > > PassRefPtr<SharedBuffer> InspectorResource::resourceData(String* textEncodingName) const > { > - if (equalIgnoringFragmentIdentifier(m_requestURL, m_loader->requestURL())) { > + if (m_loader && equalIgnoringFragmentIdentifier(m_requestURL, m_loader->requestURL())) { > *textEncodingName = m_frame->document()->inputEncoding(); > return m_loader->mainResourceData(); > } > diff --git a/WebCore/inspector/InspectorResource.h b/WebCore/inspector/InspectorResource.h > index 4004142c2374cce64ee58714bae04a4ce43faa46..3fd5a69b9d8cb9b0e8399d4ff46abd842201b7c1 100644 > --- a/WebCore/inspector/InspectorResource.h > +++ b/WebCore/inspector/InspectorResource.h > @@ -34,6 +34,8 @@ > #include "HTTPHeaderMap.h" > #include "KURL.h" > #include "ScriptString.h" > +#include "WebSocketHandshakeRequest.h" > +#include "WebSocketHandshakeResponse.h" > > #include <wtf/CurrentTime.h> > #include <wtf/OwnPtr.h> > @@ -51,6 +53,11 @@ namespace WebCore { > class ResourceRequest; > class ResourceResponse; > > +#if ENABLE(WEB_SOCKETS) > + class WebSocketHandshakeRequest; > + class WebSocketHandshakeResponse; > +#endif > + > class InspectorResource : public RefCounted<InspectorResource> { > public: > > @@ -63,16 +70,19 @@ namespace WebCore { > Script, > XHR, > Media, > + WebSocket, > Other > }; > > - static PassRefPtr<InspectorResource> create(unsigned long identifier, DocumentLoader* loader, const KURL& requestURL) > - { > - return adoptRef(new InspectorResource(identifier, loader, requestURL)); > - } > + static PassRefPtr<InspectorResource> create(unsigned long identifier, DocumentLoader* loader, const KURL& requestURL); > > static PassRefPtr<InspectorResource> createCached(unsigned long identifier, DocumentLoader*, const CachedResource*); > > +#if ENABLE(WEB_SOCKETS) > + // WebSocket resource doesn't have its loader. For WebSocket resources, m_loader and m_frame will become null. > + static PassRefPtr<InspectorResource> createWebSocket(unsigned long identifier, const KURL& requestURL, const KURL& documentURL); > +#endif > + > ~InspectorResource(); > > PassRefPtr<InspectorResource> appendRedirect(unsigned long identifier, const KURL& redirectURL); > @@ -82,6 +92,11 @@ namespace WebCore { > void updateRequest(const ResourceRequest&); > void updateResponse(const ResourceResponse&); > > +#if ENABLE(WEB_SOCKETS) > + void updateWebSocketRequest(const WebSocketHandshakeRequest&); > + void updateWebSocketResponse(const WebSocketHandshakeResponse&); > +#endif > + > void setOverrideContent(const ScriptString& data, Type); > > String sourceString() const; > @@ -149,10 +164,15 @@ namespace WebCore { > Type cachedResourceType() const; > CachedResource* cachedResource() const; > > +#if ENABLE(WEB_SOCKETS) > + void markWebSocket() { m_isWebSocket = true; } > +#endif > + > unsigned long m_identifier; > RefPtr<DocumentLoader> m_loader; > RefPtr<Frame> m_frame; > KURL m_requestURL; > + KURL m_documentURL; > HTTPHeaderMap m_requestHeaderFields; > HTTPHeaderMap m_responseHeaderFields; > String m_mimeType; > @@ -179,6 +199,15 @@ namespace WebCore { > String m_requestMethod; > String m_requestFormData; > Vector<RefPtr<InspectorResource> > m_redirects; > + > +#if ENABLE(WEB_SOCKETS) > + bool m_isWebSocket; > + > + // The following fields are not used for resources other than WebSocket. > + // We allocate them dynamically to reduce memory consumption for regular resources. > + OwnPtr<WebSocketHandshakeRequest::Key3> m_webSocketRequestKey3; > + OwnPtr<WebSocketHandshakeResponse::ChallengeResponse> m_webSocketChallengeResponse; > +#endif > }; > > } // namespace WebCore > diff --git a/WebCore/inspector/front-end/Resource.js b/WebCore/inspector/front-end/Resource.js > index 06a610d0962da838f16774dffb9620a687b11c1f..cedded357036010f311327d1c226082095fe90f0 100644 > --- a/WebCore/inspector/front-end/Resource.js > +++ b/WebCore/inspector/front-end/Resource.js > @@ -45,7 +45,8 @@ WebInspector.Resource.Type = { > Script: 4, > XHR: 5, > Media: 6, > - Other: 7, > + WebSocket: 7, > + Other: 8, > > isTextType: function(type) > { > @@ -67,6 +68,8 @@ WebInspector.Resource.Type = { > return WebInspector.UIString("script"); > case this.XHR: > return WebInspector.UIString("XHR"); > + case this.WebSocket: > + return WebInspector.UIString("WebSocket"); > case this.Other: > default: > return WebInspector.UIString("other"); > @@ -363,6 +366,9 @@ WebInspector.Resource.prototype = { > case WebInspector.Resource.Type.XHR: > this.category = WebInspector.resourceCategories.xhr; > break; > + case WebInspector.Resource.Type.WebSocket: > + this.category = WebInspector.resourceCategories.websocket; > + break; > case WebInspector.Resource.Type.Other: > default: > this.category = WebInspector.resourceCategories.other; > @@ -575,7 +581,8 @@ WebInspector.Resource.prototype = { > > if (typeof this.type === "undefined" > || this.type === WebInspector.Resource.Type.Other > - || this.type === WebInspector.Resource.Type.XHR) > + || this.type === WebInspector.Resource.Type.XHR > + || this.type === WebInspector.Resource.Type.WebSocket) > return true; > > if (this.mimeType in WebInspector.MIMETypes) > diff --git a/WebCore/inspector/front-end/ResourceView.js b/WebCore/inspector/front-end/ResourceView.js > index 7ce09b6a64ed904f1bd74b20265267648f86048b..1c2574f72eb47024697e60503249f1ed8ebb9619 100644 > --- a/WebCore/inspector/front-end/ResourceView.js > +++ b/WebCore/inspector/front-end/ResourceView.js > @@ -279,13 +279,19 @@ WebInspector.ResourceView.prototype = { > > _refreshRequestHeaders: function() > { > - this._refreshHeaders(WebInspector.UIString("Request Headers"), this.resource.sortedRequestHeaders, this.requestHeadersTreeElement); > + var additionalRow = null; > + if (typeof this.resource.webSocketRequestKey3 !== "undefined") > + additionalRow = {header: "(Key3)", value: this.resource.webSocketRequestKey3}; > + this._refreshHeaders(WebInspector.UIString("Request Headers"), this.resource.sortedRequestHeaders, additionalRow, this.requestHeadersTreeElement); > this._refreshFormData(); > }, > > _refreshResponseHeaders: function() > { > - this._refreshHeaders(WebInspector.UIString("Response Headers"), this.resource.sortedResponseHeaders, this.responseHeadersTreeElement); > + var additionalRow = null; > + if (typeof this.resource.webSocketChallengeResponse !== "undefined") > + additionalRow = {header: "(Challenge Response)", value: this.resource.webSocketChallengeResponse}; > + this._refreshHeaders(WebInspector.UIString("Response Headers"), this.resource.sortedResponseHeaders, additionalRow, this.responseHeadersTreeElement); > }, > > _refreshHTTPInformation: function() > @@ -316,7 +322,7 @@ WebInspector.ResourceView.prototype = { > } > }, > > - _refreshHeaders: function(title, headers, headersTreeElement) > + _refreshHeaders: function(title, headers, additionalRow, headersTreeElement) > { > headersTreeElement.removeChildren(); > > @@ -333,6 +339,15 @@ WebInspector.ResourceView.prototype = { > headerTreeElement.selectable = false; > headersTreeElement.appendChild(headerTreeElement); > } > + > + if (additionalRow) { > + var title = "<div class=\"header-name\">" + additionalRow.header.escapeHTML() + ":</div>"; > + title += "<div class=\"header-value source-code\">" + additionalRow.value.escapeHTML() + "</div>" > + > + var headerTreeElement = new TreeElement(title, null, false); > + headerTreeElement.selectable = false; > + headersTreeElement.appendChild(headerTreeElement); > + } > } > } > > diff --git a/WebCore/inspector/front-end/inspector.css b/WebCore/inspector/front-end/inspector.css > index 431981652638b17406fec463cb7d163dfe540c92..28234183676608656aea5c04a7088968b2c18309 100644 > --- a/WebCore/inspector/front-end/inspector.css > +++ b/WebCore/inspector/front-end/inspector.css > @@ -2835,7 +2835,8 @@ button.enable-toggle-status-bar-item.toggled-on .glyph { > } > > .resources-category-documents, .resources-category-stylesheets, .resources-category-images, > -.resources-category-scripts, .resources-category-xhr, .resources-category-fonts, .resources-category-other { > +.resources-category-scripts, .resources-category-xhr, .resources-category-fonts, > +.resources-category-websockets, .resources-category-other { > display: none; > } > > @@ -2845,6 +2846,7 @@ button.enable-toggle-status-bar-item.toggled-on .glyph { > .filter-all .resources-category-scripts, .filter-scripts .resources-category-scripts, > .filter-all .resources-category-xhr, .filter-xhr .resources-category-xhr, > .filter-all .resources-category-fonts, .filter-fonts .resources-category-fonts, > +.filter-all .resources-category-websockets, .filter-websockets .resources-category-websockets, > .filter-all .resources-category-other, .filter-other .resources-category-other, > .resource-sidebar-tree-item.selected { > display: list-item; > @@ -2920,6 +2922,15 @@ button.enable-toggle-status-bar-item.toggled-on .glyph { > -webkit-border-image: url(Images/timelineHollowPillYellow.png) 6 7 6 7; > } > > +/* FIXME: Create bar images for WebSocket. */ > +.resources-category-websockets .resources-graph-bar { > + -webkit-border-image: url(Images/timelinePillGray.png) 6 7 6 7; > +} > + > +.resources-category-websockets.resource-cached .resources-graph-bar { > + -webkit-border-image: url(Images/timelineHollowPillGray.png) 6 7 6 7; > +} > + > #resource-views { > position: absolute; > top: 23px; > diff --git a/WebCore/inspector/front-end/inspector.js b/WebCore/inspector/front-end/inspector.js > index db89e20e25fb90c5f894ac358dbebc4ce4a3bd0f..12280d34d827c69e08f5e56f10976718c28b5f79 100644 > --- a/WebCore/inspector/front-end/inspector.js > +++ b/WebCore/inspector/front-end/inspector.js > @@ -515,6 +515,7 @@ WebInspector.doLoadedDone = function() > scripts: new WebInspector.ResourceCategory("scripts", WebInspector.UIString("Scripts"), "rgb(255,121,0)"), > xhr: new WebInspector.ResourceCategory("xhr", WebInspector.UIString("XHR"), "rgb(231,231,10)"), > fonts: new WebInspector.ResourceCategory("fonts", WebInspector.UIString("Fonts"), "rgb(255,82,62)"), > + websocket: new WebInspector.ResourceCategory("websockets", WebInspector.UIString("WebSocket"), "rgb(186,186,186)"), // FIXME: Decide the color. > other: new WebInspector.ResourceCategory("other", WebInspector.UIString("Other"), "rgb(186,186,186)") > }; > > @@ -1219,6 +1220,8 @@ WebInspector.updateResource = function(payload) > resource.requestMethod = payload.requestMethod; > resource.requestFormData = payload.requestFormData; > resource.documentURL = payload.documentURL; > + if (typeof payload.webSocketRequestKey3 !== "undefined") > + resource.webSocketRequestKey3 = payload.webSocketRequestKey3; > > if (resource.mainResource) > this.mainResource = resource; > @@ -1243,6 +1246,8 @@ WebInspector.updateResource = function(payload) > resource.connectionReused = payload.connectionReused; > resource.timing = payload.timing; > resource.cached = payload.cached; > + if (typeof payload.webSocketChallengeResponse !== "undefined") > + resource.webSocketChallengeResponse = payload.webSocketChallengeResponse; > } > > if (payload.didTypeChange) { > diff --git a/WebCore/websockets/WebSocketChannel.cpp b/WebCore/websockets/WebSocketChannel.cpp > index 54be16a495be1a8c3224ca363d627dcf3dc0cc92..d58b902ab5f10e43cfbd8daacb8c4ce124c9b5ed 100644 > --- a/WebCore/websockets/WebSocketChannel.cpp > +++ b/WebCore/websockets/WebSocketChannel.cpp > @@ -36,8 +36,11 @@ > > #include "CookieJar.h" > #include "Document.h" > +#include "InspectorController.h" > #include "Logging.h" > +#include "Page.h" > #include "PlatformString.h" > +#include "ProgressTracker.h" > #include "ScriptExecutionContext.h" > #include "SocketStreamError.h" > #include "SocketStreamHandle.h" > @@ -63,7 +66,14 @@ WebSocketChannel::WebSocketChannel(ScriptExecutionContext* context, WebSocketCha > , m_closed(false) > , m_shouldDiscardReceivedData(false) > , m_unhandledBufferedAmount(0) > +#if ENABLE(INSPECTOR) > + , m_identifier(0) > +#endif > { > +#if ENABLE(INSPECTOR) > + if (InspectorController* controller = m_context->inspectorController()) > + controller->didCreateWebSocket(identifier(), url, m_context->url()); > +#endif > } > > WebSocketChannel::~WebSocketChannel() > @@ -112,6 +122,11 @@ void WebSocketChannel::close() > void WebSocketChannel::disconnect() > { > LOG(Network, "WebSocketChannel %p disconnect", this); > +#if ENABLE(INSPECTOR) > + if (m_context) > + if (InspectorController* controller = m_context->inspectorController()) > + controller->didCloseWebSocket(identifier()); > +#endif > m_handshake.clearScriptExecutionContext(); > m_client = 0; > m_context = 0; > @@ -137,6 +152,10 @@ void WebSocketChannel::didOpen(SocketStreamHandle* handle) > ASSERT(handle == m_handle); > if (!m_context) > return; > +#if ENABLE(INSPECTOR) > + if (InspectorController* controller = m_context->inspectorController()) > + controller->willSendWebSocketHandshakeRequest(identifier(), m_handshake.clientHandshakeRequest()); > +#endif > const CString& handshakeMessage = m_handshake.clientHandshakeMessage(); > if (!handle->send(handshakeMessage.data(), handshakeMessage.length())) { > m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Error sending handshake message.", 0, m_handshake.clientOrigin()); > @@ -147,6 +166,11 @@ void WebSocketChannel::didOpen(SocketStreamHandle* handle) > void WebSocketChannel::didClose(SocketStreamHandle* handle) > { > LOG(Network, "WebSocketChannel %p didClose", this); > +#if ENABLE(INSPECTOR) > + if (m_context) > + if (InspectorController* controller = m_context->inspectorController()) > + controller->didCloseWebSocket(identifier()); > +#endif > ASSERT_UNUSED(handle, handle == m_handle || !m_handle); > m_closed = true; > if (m_handle) { > @@ -250,6 +274,10 @@ bool WebSocketChannel::processBuffer() > if (headerLength <= 0) > return false; > if (m_handshake.mode() == WebSocketHandshake::Connected) { > +#if ENABLE(INSPECTOR) > + if (InspectorController* controller = m_context->inspectorController()) > + controller->didReceiveWebSocketHandshakeResponse(identifier(), m_handshake.serverHandshakeResponse()); > +#endif > if (!m_handshake.serverSetCookie().isEmpty()) { > if (m_context->isDocument()) { > Document* document = static_cast<Document*>(m_context); > @@ -367,6 +395,21 @@ void WebSocketChannel::resumeTimerFired(Timer<WebSocketChannel>* timer) > didClose(m_handle.get()); > } > > +#if ENABLE(INSPECTOR) > +unsigned long WebSocketChannel::identifier() > +{ > + if (m_identifier) > + return m_identifier; > + > + if (InspectorController* controller = m_context->inspectorController()) > + if (Page* page = controller->inspectedPage()) > + m_identifier = page->progress()->createUniqueIdentifier(); > + > + ASSERT(m_identifier); > + return m_identifier; > +} > +#endif // ENABLE(INSPECTOR) > + > } // namespace WebCore > > #endif // ENABLE(WEB_SOCKETS) > diff --git a/WebCore/websockets/WebSocketChannel.h b/WebCore/websockets/WebSocketChannel.h > index 43d431a0f899e00309e6a3059eb64c298ce20163..a08e6bb2e6ac8675886787cbf3b6a457d25cc1f8 100644 > --- a/WebCore/websockets/WebSocketChannel.h > +++ b/WebCore/websockets/WebSocketChannel.h > @@ -84,6 +84,10 @@ namespace WebCore { > bool processBuffer(); > void resumeTimerFired(Timer<WebSocketChannel>* timer); > > +#if ENABLE(INSPECTOR) > + unsigned long identifier(); > +#endif > + > ScriptExecutionContext* m_context; > WebSocketChannelClient* m_client; > WebSocketHandshake m_handshake; > @@ -96,6 +100,10 @@ namespace WebCore { > bool m_closed; > bool m_shouldDiscardReceivedData; > unsigned long m_unhandledBufferedAmount; > + > +#if ENABLE(INSPECTOR) > + unsigned long m_identifier; > +#endif > }; > > } // namespace WebCore WebCore/inspector/InspectorResource.cpp:145 + RefPtr<InspectorResource> resource = adoptRef(new InspectorResource(identifier, 0, requestURL)); I think you don't need adoptRef here. (cf. http://webkit.org/coding/RefPtr.html Mixing RefPtr and PassRefPtr) (In reply to comment #36) > WebCore/inspector/InspectorResource.cpp:145 > + RefPtr<InspectorResource> resource = adoptRef(new InspectorResource(identifier, 0, requestURL)); > I think you don't need adoptRef here. (cf. http://webkit.org/coding/RefPtr.html Mixing RefPtr and PassRefPtr) adoptRef has recently become necessary. If we omit adoptRef here, assertion at RefCounted.h:37 (!m_adoptionIsRequired) triggers. I guess we should update the documentation. (In reply to comment #37) > I guess we should update the documentation. Filed bug 44752. (In reply to comment #38) > (In reply to comment #37) > > I guess we should update the documentation. > > Filed bug 44752. OK. LTGM. Today I tried to land this patch again, but I've found the following test regression in inspector/extensions-events.html.
--- /tmp/layout-test-results/inspector/extensions-events-expected.txt 2010-09-07 17:24:22.000000000 +0900
+++ /tmp/layout-test-results/inspector/extensions-events-actual.txt 2010-09-07 17:24:22.000000000 +0900
@@ -9,7 +9,7 @@
{
0 : {
id : <number>
- type : 7
+ type : 8
har : <object>
}
}
It seems this dump is local variable at WebInspector.ExtensionServer._onGetResources (ExtensionServer.js:264, where resourceWrapper function is mapped over WebInspector.resources), and the difference is caused because new resource type is added.
I think it is okay to change the baseline for this test. I will upload a patch for review to make sure this change is right.
Created attachment 66696 [details]
Patch v8 (Test expectation update)
(In reply to comment #40) > Today I tried to land this patch again, but I've found the following test regression in inspector/extensions-events.html. > > --- /tmp/layout-test-results/inspector/extensions-events-expected.txt 2010-09-07 17:24:22.000000000 +0900 > +++ /tmp/layout-test-results/inspector/extensions-events-actual.txt 2010-09-07 17:24:22.000000000 +0900 > @@ -9,7 +9,7 @@ > { > 0 : { > id : <number> > - type : 7 > + type : 8 > har : <object> > } > } > It's ok to rebaseline the test for now, though this highlights the need for us to provide some abstraction in the extensions API from resource type constants used in front-end. (In reply to comment #40) > It seems this dump is local variable at WebInspector.ExtensionServer._onGetResources (ExtensionServer.js:264, where resourceWrapper function is mapped over WebInspector.resources), and the difference is caused because new resource type is added. > > I think it is okay to change the baseline for this test. I will upload a patch for review to make sure this change is right. This problem was resolved, thus I think I can land the without baseline changes. Created attachment 67379 [details]
Patch v9 (Update new test expectation)
(In reply to comment #44) > Created an attachment (id=67379) [details] > Patch v9 (Update new test expectation) It turned out that I need to update one test expectation: * inspector/extensions-api-expected.txt I think this is pretty obvious, but I want to make sure this is okay, so I'm requesting a review for this. Pavel, could you r+ this patch if you are okay with this? Diffs under WebCore/ has not changed since you r+'ed the previous patch (except for a few updates needed to merge with latest Inspector code). I'm sorry that it took too long to land this patch... (In reply to comment #44) > Created an attachment (id=67379) [details] > Patch v9 (Update new test expectation) LGTM Comment on attachment 67379 [details]
Patch v9 (Update new test expectation)
This has been up in the air for too long, so let us land now. However, please consider extracting web resource into a separate class to get rid of multiple conditions and ifdefs. We probably should talk about how to approach it in a way that would be consistent with the rest of the code.
Comment on attachment 67379 [details] Patch v9 (Update new test expectation) Clearing flags on attachment: 67379 Committed r67447: <http://trac.webkit.org/changeset/67447> All reviewed patches have been landed. Closing bug. http://trac.webkit.org/changeset/67447 might have broken Qt Linux Release minimal Created attachment 67522 [details]
Fix Qt builds
Comment on attachment 67522 [details]
Fix Qt builds
ok.
(In reply to comment #52) > (From update of attachment 67522 [details]) > ok. It was landed as r67448. |