| Summary: | WKAutomationDelegate's requestAutomationSession should take a suggested session identifier | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||||
| Component: | Web Inspector | Assignee: | BJ Burg <bburg> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bburg, commit-queue, graouts, joepeck, keith_miller, mark.lam, mattbaker, msaboff, nvasilyev, saam, timothy, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 154260 | ||||||||
| Attachments: |
|
||||||||
|
Description
BJ Burg
2016-02-08 15:28:00 PST
Created attachment 270889 [details]
Proposed Fix
Comment on attachment 270889 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=270889&action=review > Source/WebKit2/UIProcess/API/APIAutomationClient.h:42 > + virtual void didRequestAutomationSession(WebKit::WebProcessPool*, const WTF::String&) { } Should be able to just say "const String&" without the namespace. > Source/WebKit2/UIProcess/Cocoa/AutomationClient.h:53 > + virtual void didRequestAutomationSession(WebKit::WebProcessPool*, const String& sessionIdentifier) override > + { > + requestAutomationSession(sessionIdentifier); > + } Please put this function body in the .mm file, not the header. Keeps the class definition easier to read when the bodies are bigger than a single line. (In reply to comment #3) > Comment on attachment 270889 [details] > Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=270889&action=review > > > Source/WebKit2/UIProcess/API/APIAutomationClient.h:42 > > + virtual void didRequestAutomationSession(WebKit::WebProcessPool*, const WTF::String&) { } > > Should be able to just say "const String&" without the namespace. I thought that too, but Xcode was insistent that it was ambiguous with API::String. I didn't even know about that class! > > Source/WebKit2/UIProcess/Cocoa/AutomationClient.h:53 > > + virtual void didRequestAutomationSession(WebKit::WebProcessPool*, const String& sessionIdentifier) override > > + { > > + requestAutomationSession(sessionIdentifier); > > + } > > Please put this function body in the .mm file, not the header. Keeps the > class definition easier to read when the bodies are bigger than a single > line. OK Created attachment 270950 [details]
For Landing
Committed r196562: <http://trac.webkit.org/changeset/196562> |