Detect <link rel='manifest'> and notify embedder.

Detect <link rel='manifest'> and notify embedder.

WebFrameClient gets a dispatchManifestChange method that will
be called by the FrameLoaderClient when the manifest
information might have changed. The embedder is not aware of
the manifest URL.

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

[mlamouri (slow - plz ping), 2014-05-01 15:20:52]

All the logic regarding whether the manifest should be loaded and how to parse
it will live in Chromium.

[abarth-chromium, 2014-05-01 17:51:15]

https://codereview.chromium.org/249633002/diff/40001/Source/web/FrameLoaderCl...
File Source/web/FrameLoaderClientImpl.cpp (right):

https://codereview.chromium.org/249633002/diff/40001/Source/web/FrameLoaderCl...
Source/web/FrameLoaderClientImpl.cpp:812:
m_webFrame->client()->didChangeManifest(m_webFrame);
Why not pass the manifest URL along in this notification?  I'd imagine the first
thing the embedder will do is query for the manifest URL.

Also, won't think notification fire even when the manifest hasn't changed?  For
example, if I have two manifest link elements, we'll fire this notification
twice even though the manifest only changed once (for the first element).

https://codereview.chromium.org/249633002/diff/40001/public/web/WebFrame.h
File public/web/WebFrame.h (right):

https://codereview.chromium.org/249633002/diff/40001/public/web/WebFrame.h#ne...
public/web/WebFrame.h:231: virtual WebURL manifestURL() const = 0;
This API should be on WebDocument rather than WebFrame.  It's a property of the
document itself, not of the frame that contains the document.

https://codereview.chromium.org/249633002/diff/40001/public/web/WebFrameClient.h
File public/web/WebFrameClient.h (right):

https://codereview.chromium.org/249633002/diff/40001/public/web/WebFrameClien...
public/web/WebFrameClient.h:264: 
Two blank lines between sections pls.

[mlamouri (slow - plz ping), 2014-05-01 18:00:09]

https://codereview.chromium.org/249633002/diff/40001/Source/web/FrameLoaderCl...
File Source/web/FrameLoaderClientImpl.cpp (right):

https://codereview.chromium.org/249633002/diff/40001/Source/web/FrameLoaderCl...
Source/web/FrameLoaderClientImpl.cpp:812:
m_webFrame->client()->didChangeManifest(m_webFrame);
On 2014/05/01 17:51:16, abarth wrote:
> Why not pass the manifest URL along in this notification?  I'd imagine the
first
> thing the embedder will do is query for the manifest URL.

I tried to stay consistent with how icons update are propagated.

> Also, won't think notification fire even when the manifest hasn't changed? 
For
> example, if I have two manifest link elements, we'll fire this notification
> twice even though the manifest only changed once (for the first element).

True. The chances that more than one manifest will be present are pretty low.
The cost of keeping track of the current manifest in Blink would be way higher.

https://codereview.chromium.org/249633002/diff/40001/public/web/WebFrame.h
File public/web/WebFrame.h (right):

https://codereview.chromium.org/249633002/diff/40001/public/web/WebFrame.h#ne...
public/web/WebFrame.h:231: virtual WebURL manifestURL() const = 0;
On 2014/05/01 17:51:16, abarth wrote:
> This API should be on WebDocument rather than WebFrame.  It's a property of
the
> document itself, not of the frame that contains the document.

Will look into that.

https://codereview.chromium.org/249633002/diff/40001/public/web/WebFrameClient.h
File public/web/WebFrameClient.h (right):

https://codereview.chromium.org/249633002/diff/40001/public/web/WebFrameClien...
public/web/WebFrameClient.h:264: 
On 2014/05/01 17:51:16, abarth wrote:
> Two blank lines between sections pls.

Ok.

[abarth-chromium, 2014-05-01 18:10:24]

On 2014/05/01 18:00:09, Mounir Lamouri wrote:
> I tried to stay consistent with how icons update are propagated.

IMHO, consistency with icons isn't worthwhile.  That's very old code and was
implemented under much different constraints than exist today.  We can modernize
the icon code path in a subsequent CL if you like.

> > Also, won't think notification fire even when the manifest hasn't changed? 
> For
> > example, if I have two manifest link elements, we'll fire this notification
> > twice even though the manifest only changed once (for the first element).
> 
> True. The chances that more than one manifest will be present are pretty low.
> The cost of keeping track of the current manifest in Blink would be way
higher.

Both costs are pretty negligible.  You just need to walk the head for the
manifest link element and check whether it's the current element.

[mlamouri (slow - plz ping), 2014-05-08 16:41:23]

I've updated the patch following our discussion, PTAL.

For the records, we discussed about the fact that I want to handle the
crossorigin attribute for fetching the manifest. Adam pointed that it would be
better to do the fetching in Blink in that case so this CL has been stripped to
be only about notifying the embedder. Another CL will allow the embedder to
request the manifest to be fetched and parsed and get a WebManifest structure.

[abarth-chromium, 2014-05-08 16:45:11]

lgtm

[commit-bot: I haz the power, 2014-05-08 16:46:03]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mlamouri@chromium.org/249633002/60001

[commit-bot: I haz the power, 2014-05-08 18:47:52]

Change committed as 173664