Summary: | Web Inspector: improve generated types for objects passed to backend commands | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brian Burg <burg> | ||||||||||
Component: | Web Inspector | Assignee: | Brian Burg <burg> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, darin, graouts, hi, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=145658 | ||||||||||||
Attachments: |
|
Description
Brian Burg
2015-06-17 17:44:57 PDT
If there was a good reason to use PassRefPtr before, the newfangled equivalent of PassRefPtr is RefPtr&&, not const RefPtr&&. Created attachment 255086 [details]
WIP, need to get WebCore compiling
(In reply to comment #1) > If there was a good reason to use PassRefPtr before, the newfangled > equivalent of PassRefPtr is RefPtr&&, not const RefPtr&&. I don't think there was ever a good reason to do refcounted objects here, since the backend agents should copy data out of these marshalled, untyped objects and not take ownership. In the JS frontend we are a lot more careful to "firewall" the inspector app's data from the protocol layer by copying out data. In the attached patch, the signatures change to const T& (required param) and const T* (optional param). The objects are owned by the dispatcher implementations in the generated InspectorBackendDispatchers.cpp file. Created attachment 255126 [details]
Proposed Fix
JoePeck should take a look at this, and see what ObjC bindings stuff it broke. Should only require little fixes.
This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`) Created attachment 255130 [details]
[PATCH] ObjC Diff
These are the pieces needed to update the ObjC generator. There will be an Internal piece needed to coordinate with these changes. I'll post a combined patch.
Comment on attachment 255126 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=255126&action=review r+, but I'm going to post a combined patch / handle landing to coordinate with the internal side. > Source/WebCore/inspector/InspectorResourceAgent.cpp:304 > + Nit: trailing whitespace. > Source/WebCore/inspector/InspectorResourceAgent.h:142 > - RefPtr<Inspector::InspectorObject> m_extraRequestHeaders; > + HashMap<String, String> m_extraRequestHeaders; Nice Created attachment 255132 [details]
[PATCH] For Landing
|