Add 'exposed' objects and embeds to a document's named properties.

Add 'exposed' objects and embeds to a document's named properties.

The spec tells us that exposed <embed> and <object> are to be included
in a document's "supported property names"

  http://www.whatwg.org/specs/web-apps/current-work/#dom-document-nameditem
  http://www.whatwg.org/specs/web-apps/current-work/#exposed

(along with various other side conditions.) 

crbug.com/313292 retired a partial notion of "exposed"ness, this
provides an implementation close to the spec's definition.

R=
BUG=316988
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=163287

[sof, 2013-11-18 15:02:19]

At your leisure, please take a look.

[abarth-chromium, 2013-11-18 17:38:57]

lgtm

https://codereview.chromium.org/75273004/diff/1/Source/core/html/HTMLObjectEl...
File Source/core/html/HTMLObjectElement.cpp (right):

https://codereview.chromium.org/75273004/diff/1/Source/core/html/HTMLObjectEl...
Source/core/html/HTMLObjectElement.cpp:392: for (Node* ancestor = parentNode();
ancestor; ancestor = ancestor->parentNode()) {
Are these ancestor traversals correct w.r.t. shadow DOM?  Can you add some tests
for the cases where there's a shadow DOM boundary between the <object> and an
ancestor <object>?

[sof, 2013-11-18 20:37:34]

https://codereview.chromium.org/75273004/diff/1/Source/core/html/HTMLObjectEl...
File Source/core/html/HTMLObjectElement.cpp (right):

https://codereview.chromium.org/75273004/diff/1/Source/core/html/HTMLObjectEl...
Source/core/html/HTMLObjectElement.cpp:392: for (Node* ancestor = parentNode();
ancestor; ancestor = ancestor->parentNode()) {
On 2013/11/18 17:38:58, abarth wrote:
> Are these ancestor traversals correct w.r.t. shadow DOM?  Can you add some
tests
> for the cases where there's a shadow DOM boundary between the <object> and an
> ancestor <object>?

Thanks. Does

  https://codereview.chromium.org/75273004/patch/40001/50004

address it? I'm on thin ice wrt shadow DOM know-how.

[abarth-chromium, 2013-11-18 23:08:49]

Looks reasonable to me.

[commit-bot: I haz the power, 2013-11-19 03:45:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sigbjornf@opera.com/75273004/40001

[commit-bot: I haz the power, 2013-11-19 05:46:03]

Retried try job too often on win_layout for step(s) webkit_lint
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...

[commit-bot: I haz the power, 2013-11-19 07:23:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sigbjornf@opera.com/75273004/40001

[commit-bot: I haz the power, 2013-11-19 09:11:34]

Retried try job too often on mac_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_...

[commit-bot: I haz the power, 2013-11-19 09:34:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sigbjornf@opera.com/75273004/40001

[commit-bot: I haz the power, 2013-11-19 12:05:59]

Retried try job too often on win_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...

[sof, 2013-11-19 12:19:05]

More test expectations needs to be updated here; will attend to.

[sof, 2013-11-20 08:32:20]

On 2013/11/19 12:19:05, sof wrote:
> More test expectations needs to be updated here; will attend to.

Ought to have tested plugins/ on a non-'linux' platform, but didn't. Had to
update two test outcomes -- here's why:

  * for plugins/embed-inside-object.html

      <object name="plugin"><span><embed name="plugin"></embed></span></object>

    should match the inner <embed>, because the outer <object> is non-exposed
(has an <embed>.)
    The expectation that it would return a NodeList containing two elements
doesn't make sense,
    as the no-duplicate tree order traversal will elements with same names.
Gecko does have a bug
    there, which isn't implemented elsewhere.

  * for plugins/object-embed-plugin-scripting.html

    <object name="Plugin"><param .../><embed name="Plugin"></embed></object>

    should definitely match "Plugin", but was expected not to. Reverted to
previous expectation,
    and made the test clear(er) on which of the outer or inner <object> and
<embed>s it must
    match.

However, going over these tests in some detail across engines I uncovered a bug
in the definition of "is exposed" in the original patch, which the uploaded
patchset also addresses (along with a long comment on why that is so.) We may
want to file a spec issue on the current definition of "exposed".

If you could take another look at the patch, I'd appreciate it. (No trybot
access here, so not able to kick off any test builds to help reviewers.)

[sof, 2013-11-20 15:48:58]

On 2013/11/20 08:32:20, sof wrote:
> On 2013/11/19 12:19:05, sof wrote:
> > More test expectations needs to be updated here; will attend to.
> 
> Ought to have tested plugins/ on a non-'linux' platform, but didn't. Had to
> update two test outcomes -- here's why:
> 
>   * for plugins/embed-inside-object.html
> 
>       <object name="plugin"><span><embed
name="plugin"></embed></span></object>
> 
>     should match the inner <embed>, because the outer <object> is non-exposed
> (has an <embed>.)
>     The expectation that it would return a NodeList containing two elements
> doesn't make sense,
>     as the no-duplicate tree order traversal will elements with same names.
> Gecko does have a bug
>     there, which isn't implemented elsewhere.
> 
>   * for plugins/object-embed-plugin-scripting.html
> 
>     <object name="Plugin"><param .../><embed name="Plugin"></embed></object>
> 
>     should definitely match "Plugin", but was expected not to. Reverted to
> previous expectation,
>     and made the test clear(er) on which of the outer or inner <object> and
> <embed>s it must
>     match.
> 
> However, going over these tests in some detail across engines I uncovered a
bug
> in the definition of "is exposed" in the original patch, which the uploaded
> patchset also addresses (along with a long comment on why that is so.) We may
> want to file a spec issue on the current definition of "exposed".
> 
> If you could take another look at the patch, I'd appreciate it. (No trybot
> access here, so not able to kick off any test builds to help reviewers.)

After having briefly discussed and gone through the various cases locally, the
change doesn't merit the added comment - it is implementing what's required.

That is, if an <object> has a descendant <object> or <embed> it isn't exposed.
But in all other cases it needs to be considered as being exposed. i.e., we do
want

  <object name="f"><div>x</div></object>

to provide the name "f".

Hence, uploaded a patch with that comment retired. Tricky terrain.

[sof, 2013-11-26 23:13:00]

Does the updated version of the "exposed"ness predicate appear reasonable?

[sof, 2013-12-05 09:59:26]

Rebased and took care of the conflict&issues wrt the revert that happened in

  https://codereview.chromium.org/61213008/

(don't fully understand that CL going ahead, but be that as it may.)

Could you give this a final look-over, please? trybots w/ plugin support reports
ok.

[abarth-chromium, 2013-12-05 19:05:39]

lgtm

[commit-bot: I haz the power, 2013-12-05 19:05:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sigbjornf@opera.com/75273004/490001

[sof, 2013-12-05 20:58:04]

On 2013/12/05 19:05:39, abarth wrote:
> lgtm

Terrific, thanks for going over it again.

[commit-bot: I haz the power, 2013-12-05 21:21:34]

Change committed as 163287