Bug 154012

Summary:

WKAutomationDelegate's requestAutomationSession should take a suggested session identifier

Product:

WebKit

Reporter:

Blaze Burg <bburg>

Component:

Web Inspector

Assignee:

Blaze 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:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

154260

Attachments:

Description	Flags
Proposed Fix	none
For Landing	none

Blaze Burg

Reported 2016-02-08 15:28:00 PST

Patch coming.

Attachments
Proposed Fix (6.93 KB, patch) 2016-02-08 15:55 PST, Blaze Burg	no flags	Details Formatted Diff Diff
For Landing (7.18 KB, patch) 2016-02-09 14:03 PST, Blaze Burg	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2016-02-08 15:29:08 PST

<rdar://problem/24557697>

Blaze Burg

Comment 2 2016-02-08 15:55:57 PST

Created attachment 270889 [details] Proposed Fix

Darin Adler

Comment 3 2016-02-09 07:24:56 PST

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.

Blaze Burg

Comment 4 2016-02-09 09:47:11 PST

(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

Blaze Burg

Comment 5 2016-02-09 14:03:22 PST

Created attachment 270950 [details] For Landing

Blaze Burg

Comment 6 2016-02-14 17:42:31 PST

Committed r196562: <http://trac.webkit.org/changeset/196562>

Note You need to log in before you can comment on or make changes to this bug.