Bug 175130

Created attachment 317116 [details] Patch

It doesn't apply because it depends on bug #174618

Comment on attachment 317116 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317116&action=review Source/JavaScriptCore and Source/WebDriver parts look good to me. Please find a GTK reviewer for the rest. > Source/JavaScriptCore/inspector/remote/RemoteInspector.h:70 > + String browserName; Perhaps we can conditionalize these for GTK/WPE, I don't think Safari/safaridriver needs them at present and this patch doesn't update the capability caching code for these keys. Each Safari (Safari Technology Preview, system Safari, engineering builds) have their own copy of safaridriver so there is no matching to be done on the browser side. I think we'll want to have the ability to report and request capabilities for some other things, however. > Source/JavaScriptCore/inspector/remote/RemoteInspector.h:93 > + std::optional<RemoteInspector::Client::Capabilities> clientCapabilities() const { return m_clientCapabilities; } OK

Comment on attachment 317116 [details] Patch r- until rebased for EWS.

Comment on attachment 317116 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317116&action=review > Source/WebKit/UIProcess/API/glib/WebKitApplicationInfo.cpp:136 > + * Set the application version. If the application doesn't use the format > + * major.minor.micro you can pass 0 as the micro to use major.minor, or pass > + * 0 as both micro and minor to use only major number. Any other format must > + * be converted to major.minor.micro so that it can be used in version comparisons. What about nano versions? I do nano versions for Epiphany on occasion. This API is too limiting. I would make it variadic so the user can add any desired number of decimal places.

(In reply to Michael Catanzaro from comment #5) > Comment on attachment 317116 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=317116&action=review > > > Source/WebKit/UIProcess/API/glib/WebKitApplicationInfo.cpp:136 > > + * Set the application version. If the application doesn't use the format > > + * major.minor.micro you can pass 0 as the micro to use major.minor, or pass > > + * 0 as both micro and minor to use only major number. Any other format must > > + * be converted to major.minor.micro so that it can be used in version comparisons. > > What about nano versions? I do nano versions for Epiphany on occasion. > > This API is too limiting. I would make it variadic so the user can add any > desired number of decimal places. I don't think we need that level of detail. This is used for drivers to request a minimum version of the browser, normally because it has a new feature required or something like that, I doubt we really need to require a nano version ever. Also, this should work for projects using any other version formats, they are expected to convert them into x.y.z ensuring they can be compared. So, the selenium driver is always going to use x.y.x format.

(In reply to Brian Burg from comment #3) > Comment on attachment 317116 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=317116&action=review > > Source/JavaScriptCore and Source/WebDriver parts look good to me. Please > find a GTK reviewer for the rest. > > > Source/JavaScriptCore/inspector/remote/RemoteInspector.h:70 > > + String browserName; > > Perhaps we can conditionalize these for GTK/WPE, I don't think > Safari/safaridriver needs them at present and this patch doesn't update the > capability caching code for these keys. Each Safari (Safari Technology > Preview, system Safari, engineering builds) have their own copy of > safaridriver so there is no matching to be done on the browser side. My idea was to make it glib only initially, but I think this doesn't hurt when unused and we leave cleaner code without ifdefs. > I think we'll want to have the ability to report and request capabilities > for some other things, however. > > > Source/JavaScriptCore/inspector/remote/RemoteInspector.h:93 > > + std::optional<RemoteInspector::Client::Capabilities> clientCapabilities() const { return m_clientCapabilities; } > > OK

Created attachment 317335 [details] Rebased patch

Attachment 317335 [details] did not pass style-queue: WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/WebKitAutomationSession.h" ERROR: Source/WebDriver/SessionHost.h:59: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/SessionHost.h:98: Extra space before ( in function call [whitespace/parens] [4] WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/webkit2.h" WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/WebKitApplicationInfo.h" ERROR: Source/WebDriver/glib/SessionHostGlib.cpp:245: Extra space before ( in function call [whitespace/parens] [4] WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/WebKitApplicationInfo.h" WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/webkit.h" WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/WebKitAutomationSession.h" Total errors found: 3 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 317336 [details] Try to fix WPE build

Attachment 317336 [details] did not pass style-queue: WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/WebKitAutomationSession.h" ERROR: Source/WebDriver/SessionHost.h:59: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/SessionHost.h:98: Extra space before ( in function call [whitespace/parens] [4] WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/webkit2.h" WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/WebKitApplicationInfo.h" ERROR: Source/WebDriver/glib/SessionHostGlib.cpp:245: Extra space before ( in function call [whitespace/parens] [4] WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/WebKitApplicationInfo.h" WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/webkit.h" WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/WebKitAutomationSession.h" Total errors found: 3 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.

GTK API r=me

Created attachment 317359 [details] Patch for landing

Attachment 317359 [details] did not pass style-queue: WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/WebKitAutomationSession.h" ERROR: Source/WebDriver/SessionHost.h:59: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebDriver/SessionHost.h:98: Extra space before ( in function call [whitespace/parens] [4] WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/webkit2.h" WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/WebKitApplicationInfo.h" ERROR: Source/WebDriver/glib/SessionHostGlib.cpp:245: Extra space before ( in function call [whitespace/parens] [4] WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/WebKitApplicationInfo.h" WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/webkit.h" WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/WebKitAutomationSession.h" Total errors found: 3 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.

Committed r220329: <http://trac.webkit.org/changeset/220329>