Bug 174987 – Web Automation: add event to notify service when a page's main frame window object has cleared

• Component: WebKit2
• Status: RESOLVED FIXED

[Blaze Burg]

Needed by safaridriver to prune old element handles that are no longer valid.

[Blaze Burg]

<rdar://problem/36602634>

[Blaze Burg]

Created attachment 316796 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 316796 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=316796&action=review

LGTM, do we need a WebKit2 owner approval for this?

> Source/WebKit/UIProcess/Automation/Automation.json:530
> +        {
> +            "name": "browsingContextCleared",
> +            "description": "Fired when the specified browsing context has navigated or closed. Existing element references from the browsing context are no longer valid.",
> +            "parameters": [
> +                { "name": "browsingContextHandle", "$ref": "BrowsingContextHandle" }
> +            ]

What does safaridriver do in response to this? element references are in the automation proxy in the web process, no? or does safaridriver keep also a cache of element references?

[Blaze Burg]

Comment on attachment 316796 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=316796&action=review

>> Source/WebKit/UIProcess/Automation/Automation.json:530
>> +            ]
> 
> What does safaridriver do in response to this? element references are in the automation proxy in the web process, no? or does safaridriver keep also a cache of element references?

This is part of our implementation of file upload. The W3C specification states that sendKeys to an <input type=file multiple> should append, not replace, the file list. I don't want to encode the append semantics into the protocol. So safaridriver keeps a map of (element handle) -> (last file list selection). This allows it to send a new Automation.setFilesToSelectForFileUpload command with the new file appended. We also double-check the FileList of `element.files` to make sure that it hasn't been cleared out from under us via element.form.reset() or whatever.

If we didn't keep a map of element -> last file selection, then we'd not be able to emulate appending files. We'd also not be able to re-match the already selected files (which lack a full path, we just know the basename+mtime+filesize) to the full file paths that we need to pass to setFilesToSelectForFileUpload.

So, with all that in mind, this event lets safaridriver clear out element handles from the aforementioned map when the corresponding browsing context clears its window object. Otherwise, we'd retain a handle in the map for every element used as a file upload target.

[Blaze Burg]

(In reply to Carlos Garcia Campos from comment #3)
> Comment on attachment 316796 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316796&action=review
> 
> LGTM, do we need a WebKit2 owner approval for this?

Maybe officially, but not really? I'll cc Tim and Brady but usually things unrelated to API are less important to get reviewed by that team.

[Brady Eidson]

Comment on attachment 316796 [details]
Patch

s'fine

[Carlos Garcia Campos]

(In reply to Brian Burg from comment #4)
> Comment on attachment 316796 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316796&action=review
> 
> >> Source/WebKit/UIProcess/Automation/Automation.json:530
> >> +            ]
> > 
> > What does safaridriver do in response to this? element references are in the automation proxy in the web process, no? or does safaridriver keep also a cache of element references?
> 
> This is part of our implementation of file upload. The W3C specification
> states that sendKeys to an <input type=file multiple> should append, not
> replace, the file list. I don't want to encode the append semantics into the
> protocol. So safaridriver keeps a map of (element handle) -> (last file list
> selection). This allows it to send a new
> Automation.setFilesToSelectForFileUpload command with the new file appended.
> We also double-check the FileList of `element.files` to make sure that it
> hasn't been cleared out from under us via element.form.reset() or whatever.
> 
> If we didn't keep a map of element -> last file selection, then we'd not be
> able to emulate appending files. We'd also not be able to re-match the
> already selected files (which lack a full path, we just know the
> basename+mtime+filesize) to the full file paths that we need to pass to
> setFilesToSelectForFileUpload.
> 
> So, with all that in mind, this event lets safaridriver clear out element
> handles from the aforementioned map when the corresponding browsing context
> clears its window object. Otherwise, we'd retain a handle in the map for
> every element used as a file upload target.

Ah, ok, I haven't implemented yet the file uploads, so I didn't know that. I'll do it after list box elements.

[Carlos Garcia Campos]

(In reply to Brian Burg from comment #5)
> (In reply to Carlos Garcia Campos from comment #3)
> > Comment on attachment 316796 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=316796&action=review
> > 
> > LGTM, do we need a WebKit2 owner approval for this?
> 
> Maybe officially, but not really? I'll cc Tim and Brady but usually things
> unrelated to API are less important to get reviewed by that team.

I guess I'm too used to ask owners approval for all cross-platform changes in WebKit2.

[WebKit Commit Bot]

Comment on attachment 316796 [details]
Patch

Clearing flags on attachment: 316796

Committed r220115: <http://trac.webkit.org/changeset/220115>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Blaze Burg]

Reopening to attach new patch.

[Blaze Burg]

Created attachment 316912 [details]
Proposed Fix

[Blaze Burg]

Comment on attachment 316912 [details]
Proposed Fix

webkit-patch got confused.