Implemented Document.contentType

Implemented Document.contentType

http://dom.spec.whatwg.org/#dom-document-contenttype

BUG=279426
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170646

[arv (Not doing code reviews), 2014-02-11 16:36:09]

1. Don't we already have dummy resources for these.
2. We have tests for documentObject.cloneNode. Can you expand those to make sure
we handle contentType while cloning too.

https://codereview.chromium.org/151653004/diff/1/LayoutTests/fast/dom/documen...
File LayoutTests/fast/dom/document-contentType-data-uri.html (right):

https://codereview.chromium.org/151653004/diff/1/LayoutTests/fast/dom/documen...
LayoutTests/fast/dom/document-contentType-data-uri.html:16: if (e.data)
Wrong indentation

https://codereview.chromium.org/151653004/diff/1/LayoutTests/fast/dom/documen...
LayoutTests/fast/dom/document-contentType-data-uri.html:25: var
document_contents = '<script xmlns="http://www.w3.org/1999/xhtml">' +
no underscores in js

https://codereview.chromium.org/151653004/diff/1/LayoutTests/fast/dom/documen...
LayoutTests/fast/dom/document-contentType-data-uri.html:30: var iframes =
document.body.getElementsByTagName('iframe');
document.querySelector('iframe') or document.getElementsByTagName('iframe')

https://codereview.chromium.org/151653004/diff/1/LayoutTests/http/tests/dom/d...
File LayoutTests/http/tests/dom/document-contentType-xhr.html (right):

https://codereview.chromium.org/151653004/diff/1/LayoutTests/http/tests/dom/d...
LayoutTests/http/tests/dom/document-contentType-xhr.html:19: function _(s) {
return '"' + s + '"'; };
Use descriptive names

https://codereview.chromium.org/151653004/diff/1/LayoutTests/http/tests/dom/d...
File LayoutTests/http/tests/dom/document-contentType.html (right):

https://codereview.chromium.org/151653004/diff/1/LayoutTests/http/tests/dom/d...
LayoutTests/http/tests/dom/document-contentType.html:24: iframes =
document.body.getElementsByTagName('iframe');
missing var

https://codereview.chromium.org/151653004/diff/1/LayoutTests/http/tests/dom/d...
LayoutTests/http/tests/dom/document-contentType.html:25: for (var k = 0;
iframes[k]; k++)
Use {} for multiline bodies

https://codereview.chromium.org/151653004/diff/1/Source/core/dom/Document.h
File Source/core/dom/Document.h (right):

https://codereview.chromium.org/151653004/diff/1/Source/core/dom/Document.h#n...
Source/core/dom/Document.h:564: 
remove extra empty line

https://codereview.chromium.org/151653004/diff/1/Source/core/dom/Document.h#n...
Source/core/dom/Document.h:1176: // Mime-type of the XHR response that created
this document.
How is this related to XHR? Maybe remove comment and rename field?

[João Eiras, 2014-02-11 20:05:50]

On 2014/02/11 16:36:09, arv wrote:
> 1. Don't we already have dummy resources for these.

Well, there are files with the given types scatered in the LayoutTests folder,
but not one single spot where everything could be found (as far as I've
searched, including LayoutTests/resources) Pointing to different files in
different folders would be a bit messy, and problem prone if someone else
changes them.

>
https://codereview.chromium.org/151653004/diff/1/Source/core/dom/Document.h#n...
> Source/core/dom/Document.h:1176: // Mime-type of the XHR response that created
> this document.
> How is this related to XHR? Maybe remove comment and rename field?

After your cloneNode suggestion, turns out that the cloned documents do not have
the DocumentLoader either. So, now that field stores the mime type for cloned
documents and xhr. Naming it for such cases would be a bit tricky. And there
still needs to be a comment there if people use the field elsewhere, they know
they can expect it to be unset in most common cases. Anything specific you'd
recommend doing with it ?

[João Eiras, 2014-02-21 14:10:55]

Ping :)

On 2014/02/11 20:05:50, João Eiras wrote:
> On 2014/02/11 16:36:09, arv wrote:
> > 1. Don't we already have dummy resources for these.
> 
> Well, there are files with the given types scatered in the LayoutTests folder,
> but not one single spot where everything could be found (as far as I've
> searched, including LayoutTests/resources) Pointing to different files in
> different folders would be a bit messy, and problem prone if someone else
> changes them.
> 

Do you recommend moving the dummy files somewhere ? Note that currently the
python script that setup the LayoutTests http server do not map the LayoutTests
folder, so I'd have to add an alias, which would be tricky while
http://code.google.com/p/chromium/issues/detail?id=30536 is in progress.

[arv (Not doing code reviews), 2014-02-21 15:38:10]

On 2014/02/21 14:10:55, João Eiras wrote:
> Ping :)

Sorry. Feel free to ping me sooner. A week is not acceptable.

> On 2014/02/11 20:05:50, João Eiras wrote:
> > On 2014/02/11 16:36:09, arv wrote:
> > > 1. Don't we already have dummy resources for these.
> > 
> > Well, there are files with the given types scatered in the LayoutTests
folder,
> > but not one single spot where everything could be found (as far as I've
> > searched, including LayoutTests/resources) Pointing to different files in
> > different folders would be a bit messy, and problem prone if someone else
> > changes them.
> > 
> 
> Do you recommend moving the dummy files somewhere ? Note that currently the
> python script that setup the LayoutTests http server do not map the
LayoutTests
> folder, so I'd have to add an alias, which would be tricky while
> http://code.google.com/p/chromium/issues/detail?id=30536 is in progress.

I'm OK with the dummy files. I was wondering if we could put them under
LayoutTests/http/tests/resources/ at least to make them a bit easier for other
people to discover.

I also really want abarth to look at this. When I touched related code in the
past he pointed out subtleties that I didn't know about.

[arv (Not doing code reviews), 2014-02-21 15:43:52]

https://codereview.chromium.org/151653004/diff/140001/LayoutTests/http/tests/...
File LayoutTests/http/tests/dom/document-contentType-xhr.html (right):

https://codereview.chromium.org/151653004/diff/140001/LayoutTests/http/tests/...
LayoutTests/http/tests/dom/document-contentType-xhr.html:10: var to_test = [
no underscores please

https://codereview.chromium.org/151653004/diff/140001/LayoutTests/http/tests/...
File LayoutTests/http/tests/dom/document-contentType.html (right):

https://codereview.chromium.org/151653004/diff/140001/LayoutTests/http/tests/...
LayoutTests/http/tests/dom/document-contentType.html:25: iframes =
document.body.getElementsByTagName('iframe');
document.body.getElementsByTagName -> document.getElementsByTagName

or

document.querySelectorAll

[João Eiras, 2014-03-24 13:06:06]

Ping abarth and arv.

So what now ?

[arv (Not doing code reviews), 2014-03-24 16:20:11]

LGTM

It would be nice if abarth would review this too.

https://codereview.chromium.org/151653004/diff/450001/LayoutTests/http/tests/...
File LayoutTests/http/tests/dom/document-contentType-xhr.html (right):

https://codereview.chromium.org/151653004/diff/450001/LayoutTests/http/tests/...
LayoutTests/http/tests/dom/document-contentType-xhr.html:40: }
} else {

https://codereview.chromium.org/151653004/diff/450001/LayoutTests/http/tests/...
File LayoutTests/http/tests/dom/document-contentType.html (right):

https://codereview.chromium.org/151653004/diff/450001/LayoutTests/http/tests/...
LayoutTests/http/tests/dom/document-contentType.html:24: onload = function() {
window.onload = function() {

[abarth-chromium, 2014-03-24 16:42:31]

On 2014/03/24 16:20:11, arv wrote:
> It would be nice if abarth would review this too.

Looks reasonable to me.  Is there anything specific you wanted me to look at?

[arv (Not doing code reviews), 2014-03-24 17:56:55]

On 2014/03/24 16:42:31, abarth wrote:
> On 2014/03/24 16:20:11, arv wrote:
> > It would be nice if abarth would review this too.
> 
> Looks reasonable to me.  Is there anything specific you wanted me to look at?

Thanks. I just remember when I was touching this code before that you pointed
out some subtleties that I did not think of.

[abarth-chromium, 2014-03-24 18:04:59]

On 2014/03/24 17:56:55, arv wrote:
> Thanks. I just remember when I was touching this code before that you pointed
> out some subtleties that I did not think of.

Yeah, that was because SecurityOrigin is a complicated object.  Here we're just
sharing an AtomicString, which should be safe because it's immutable.  Thanks!

[João Eiras, 2014-03-25 12:14:16]

The CQ bit was checked by joaoe@opera.com

[commit-bot: I haz the power, 2014-03-25 12:14:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joaoe@opera.com/151653004/550001

[commit-bot: I haz the power, 2014-03-25 12:25:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-25 12:25:37]

Try jobs failed on following builders:
  tryserver.blink on blink_presubmit

[João Eiras, 2014-03-25 19:50:37]

The CQ bit was checked by joaoe@opera.com

[commit-bot: I haz the power, 2014-03-25 19:50:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joaoe@opera.com/151653004/550001

[commit-bot: I haz the power, 2014-03-25 20:01:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-25 20:01:42]

Try jobs failed on following builders:
  tryserver.blink on blink_presubmit

[João Eiras, 2014-03-26 10:56:51]

The CQ bit was checked by joaoe@opera.com

[commit-bot: I haz the power, 2014-03-26 10:57:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joaoe@opera.com/151653004/620001

[commit-bot: I haz the power, 2014-03-26 11:04:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-26 11:04:55]

Try jobs failed on following builders:
  tryserver.blink on blink_presubmit

[João Eiras, 2014-03-27 13:14:56]

On 2014/03/26 11:04:55, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   tryserver.blink on blink_presubmit

> ** Presubmit ERRORS ** Run the command: svn pset svn:mime-type image/bmp
LayoutTests/(...).bmp

arv, could you help me a bit please and commit the binary files in
LayoutTests/http/tests/resources ? Then I'll commit the rest of the review
normally through commit queue.

Thank you!

[arv (Not doing code reviews), 2014-03-27 14:48:47]

On 2014/03/27 13:14:56, João Eiras wrote:
> On 2014/03/26 11:04:55, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   tryserver.blink on blink_presubmit
> 
> > ** Presubmit ERRORS ** Run the command: svn pset svn:mime-type image/bmp
> LayoutTests/(...).bmp
> 
> arv, could you help me a bit please and commit the binary files in
> LayoutTests/http/tests/resources ? Then I'll commit the rest of the review
> normally through commit queue.
> 
> Thank you!

Sure.

[arv (Not doing code reviews), 2014-03-27 15:10:04]

On 2014/03/27 14:48:47, arv wrote:
> On 2014/03/27 13:14:56, João Eiras wrote:
> > On 2014/03/26 11:04:55, I haz the power (commit-bot) wrote:
> > > Try jobs failed on following builders:
> > >   tryserver.blink on blink_presubmit
> > 
> > > ** Presubmit ERRORS ** Run the command: svn pset svn:mime-type image/bmp
> > LayoutTests/(...).bmp
> > 
> > arv, could you help me a bit please and commit the binary files in
> > LayoutTests/http/tests/resources ? Then I'll commit the rest of the review
> > normally through commit queue.
> > 
> > Thank you!
> 
> Sure.

The patch does not apply cleanly so `git cl patch` fails. I can commit the
images in a separate CL.

[arv (Not doing code reviews), 2014-03-27 15:12:35]

On 2014/03/27 15:10:04, arv wrote:
> On 2014/03/27 14:48:47, arv wrote:
> > On 2014/03/27 13:14:56, João Eiras wrote:
> > > On 2014/03/26 11:04:55, I haz the power (commit-bot) wrote:
> > > > Try jobs failed on following builders:
> > > >   tryserver.blink on blink_presubmit
> > > 
> > > > ** Presubmit ERRORS ** Run the command: svn pset svn:mime-type image/bmp
> > > LayoutTests/(...).bmp
> > > 
> > > arv, could you help me a bit please and commit the binary files in
> > > LayoutTests/http/tests/resources ? Then I'll commit the rest of the review
> > > normally through commit queue.
> > > 
> > > Thank you!
> > 
> > Sure.
> 
> The patch does not apply cleanly so `git cl patch` fails. I can commit the
> images in a separate CL.

Images landed in r170187

[João Eiras, 2014-03-27 16:44:30]

> Images landed in r170187

ah, thank you so much :)

[João Eiras, 2014-03-28 20:09:58]

The CQ bit was checked by joaoe@opera.com

[commit-bot: I haz the power, 2014-03-28 20:10:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joaoe@opera.com/151653004/720001

[commit-bot: I haz the power, 2014-03-28 20:53:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 20:53:28]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[João Eiras, 2014-04-01 18:59:45]

The CQ bit was checked by joaoe@opera.com

[commit-bot: I haz the power, 2014-04-01 18:59:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joaoe@opera.com/151653004/740001

[commit-bot: I haz the power, 2014-04-01 19:26:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-01 19:26:51]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[João Eiras, 2014-04-02 10:02:03]

The CQ bit was checked by joaoe@opera.com

[commit-bot: I haz the power, 2014-04-02 10:02:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joaoe@opera.com/151653004/740001

[commit-bot: I haz the power, 2014-04-02 10:32:29]

Change committed as 170646