Remove WebContents::HasManifest.

Remove WebContents::HasManifest.

WebContents::GetManifest now returns a manifest URL and a manifest
object. The non-emptiness of the URL correlates to whether HasManifest
returned true or false. This CL removes the HasManifest method from
WebContents and refactors its sole client, app banners, to check the
manifest URL returned in GetManifest instead. This eliminates a roundtrip
IPC from the app banner flow, which runs on every page load on mobile.

Minor changes to when the manifest URL is set in the manifest parsing
process are also made to ensure the correctness of this approach.

BUG=626969
Committed: https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d
Cr-Commit-Position: refs/heads/master@{#404973}

[dominickn, 2016-07-11 02:51:58]

Description was changed from

==========
Remove WebContents::HasManifest.

WebContents::GetManifest now returns a manifest URL and a manifest
object. The non-emptiness of the URL correlates to whether HasManifest
returned true or false. This CL removes the HasManifest method from
WebContents and refactors its sole client, app banners, to check the
manifest URL returned in GetManifest instead. Minor changes to when the
manifest URL is set in the manifest parsing process are also made to
ensure the correctness of this approach.

BUG=626969
==========

to

==========
Remove WebContents::HasManifest.

WebContents::GetManifest now returns a manifest URL and a manifest
object. The non-emptiness of the URL correlates to whether HasManifest
returned true or false. This CL removes the HasManifest method from
WebContents and refactors its sole client, app banners, to check the
manifest URL returned in GetManifest instead. Minor changes to when the
manifest URL is set in the manifest parsing process are also made to
ensure the correctness of this approach.

BUG=626969
==========

[dominickn, 2016-07-11 02:51:59]

dominickn@chromium.org changed reviewers:
+ creis@chromium.org, dfalcantara@chromium.org, mlamouri@chromium.org

[dominickn, 2016-07-11 02:52:29]

dominickn@chromium.org changed reviewers:
+ palmer@chromium.org

[dominickn, 2016-07-11 02:53:36]

dominickn@chromium.org changed reviewers:
+ mkwst@chromium.org
- palmer@chromium.org

[dominickn, 2016-07-11 02:56:03]

PTAL, thanks!

mkwst: please review manifest_messages.h (removed IPC)

creis: please review content/public (method removed)

mlamouri: please review all the other manifest stuff

dfalcantara: please review banners

[dominickn, 2016-07-11 02:57:21]

Description was changed from

==========
Remove WebContents::HasManifest.

WebContents::GetManifest now returns a manifest URL and a manifest
object. The non-emptiness of the URL correlates to whether HasManifest
returned true or false. This CL removes the HasManifest method from
WebContents and refactors its sole client, app banners, to check the
manifest URL returned in GetManifest instead. Minor changes to when the
manifest URL is set in the manifest parsing process are also made to
ensure the correctness of this approach.

BUG=626969
==========

to

==========
Remove WebContents::HasManifest.

WebContents::GetManifest now returns a manifest URL and a manifest
object. The non-emptiness of the URL correlates to whether HasManifest
returned true or false. This CL removes the HasManifest method from
WebContents and refactors its sole client, app banners, to check the
manifest URL returned in GetManifest instead. This eliminates a roundtrip
IPC from the app banner flow, which runs on every page load on mobile.

Minor changes to when the manifest URL is set in the manifest parsing
process are also made to ensure the correctness of this approach.

BUG=626969
==========

[Mike West, 2016-07-11 08:14:18]

Less IPC is better IPC. LGTM % question.

https://codereview.chromium.org/2140463002/diff/1/content/renderer/manifest/m...
File content/renderer/manifest/manifest_manager.cc (right):

https://codereview.chromium.org/2140463002/diff/1/content/renderer/manifest/m...
content/renderer/manifest/manifest_manager.cc:191: manifest_ = Manifest();
Why don't you need to clear out |manifest_url_| anymore? If you're clearing it
elsewhere, can you add a DCHECK here?

[dominickn, 2016-07-11 10:51:18]

https://codereview.chromium.org/2140463002/diff/1/content/renderer/manifest/m...
File content/renderer/manifest/manifest_manager.cc (right):

https://codereview.chromium.org/2140463002/diff/1/content/renderer/manifest/m...
content/renderer/manifest/manifest_manager.cc:191: manifest_ = Manifest();
On 2016/07/11 08:14:18, Mike West wrote:
> Why don't you need to clear out |manifest_url_| anymore? If you're clearing it
> elsewhere, can you add a DCHECK here?

It's explicitly meant to not be cleared (it gets reset in either
DidChangeManifest or DidCommitProvisionalLoad). We want this to remain set at
the value it received in FetchManifest so that a 404 manifest still sends a URL
to the callbacks (but an empty manifest object). I'll add a comment to explain.

[mlamouri (slow - plz ping), 2016-07-11 10:56:14]

lgtm

[gone, 2016-07-11 17:47:54]

banner stuff lgtm

[Charlie Reis, 2016-07-11 18:34:41]

creis@chromium.org changed reviewers:
+ nasko@chromium.org

[Charlie Reis, 2016-07-11 18:34:42]

Thanks for simplifying it!  content/ LGTM.

[+nasko for IPC review of manifest_manager_messages.h]

[nasko, 2016-07-11 21:52:45]

Code removal is always welcome, so thanks! IPC LGTM

[dominickn, 2016-07-12 00:47:22]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2016-07-12 00:47:22]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org,
mkwst@chromium.org, dfalcantara@chromium.org, nasko@chromium.org,
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/2140463002/#ps20001
(title: "Explanatory comment for manifest_url_ resetting")

[commit-bot: I haz the power, 2016-07-12 00:48:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 01:36:48]

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

[commit-bot: I haz the power, 2016-07-12 01:36:50]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[dominickn, 2016-07-12 02:59:06]

The CQ bit was checked by dominickn@chromium.org

[commit-bot: I haz the power, 2016-07-12 03:00:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 04:30:45]

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

[commit-bot: I haz the power, 2016-07-12 04:30:46]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[dominickn, 2016-07-12 10:50:19]

The CQ bit was checked by dominickn@chromium.org

[commit-bot: I haz the power, 2016-07-12 10:50:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 12:26:09]

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

[commit-bot: I haz the power, 2016-07-12 12:26:11]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[dominickn, 2016-07-13 01:46:56]

The CQ bit was checked by dominickn@chromium.org

[commit-bot: I haz the power, 2016-07-13 01:48:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 03:42:12]

Description was changed from

==========
Remove WebContents::HasManifest.

WebContents::GetManifest now returns a manifest URL and a manifest
object. The non-emptiness of the URL correlates to whether HasManifest
returned true or false. This CL removes the HasManifest method from
WebContents and refactors its sole client, app banners, to check the
manifest URL returned in GetManifest instead. This eliminates a roundtrip
IPC from the app banner flow, which runs on every page load on mobile.

Minor changes to when the manifest URL is set in the manifest parsing
process are also made to ensure the correctness of this approach.

BUG=626969
==========

to

==========
Remove WebContents::HasManifest.

WebContents::GetManifest now returns a manifest URL and a manifest
object. The non-emptiness of the URL correlates to whether HasManifest
returned true or false. This CL removes the HasManifest method from
WebContents and refactors its sole client, app banners, to check the
manifest URL returned in GetManifest instead. This eliminates a roundtrip
IPC from the app banner flow, which runs on every page load on mobile.

Minor changes to when the manifest URL is set in the manifest parsing
process are also made to ensure the correctness of this approach.

BUG=626969
==========

[commit-bot: I haz the power, 2016-07-13 03:42:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-13 03:44:10]

Description was changed from

==========
Remove WebContents::HasManifest.

WebContents::GetManifest now returns a manifest URL and a manifest
object. The non-emptiness of the URL correlates to whether HasManifest
returned true or false. This CL removes the HasManifest method from
WebContents and refactors its sole client, app banners, to check the
manifest URL returned in GetManifest instead. This eliminates a roundtrip
IPC from the app banner flow, which runs on every page load on mobile.

Minor changes to when the manifest URL is set in the manifest parsing
process are also made to ensure the correctness of this approach.

BUG=626969
==========

to

==========
Remove WebContents::HasManifest.

WebContents::GetManifest now returns a manifest URL and a manifest
object. The non-emptiness of the URL correlates to whether HasManifest
returned true or false. This CL removes the HasManifest method from
WebContents and refactors its sole client, app banners, to check the
manifest URL returned in GetManifest instead. This eliminates a roundtrip
IPC from the app banner flow, which runs on every page load on mobile.

Minor changes to when the manifest URL is set in the manifest parsing
process are also made to ensure the correctness of this approach.

BUG=626969

Committed: https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d
Cr-Commit-Position: refs/heads/master@{#404973}
==========

[commit-bot: I haz the power, 2016-07-13 03:44:12]

Patchset 2 (id:??) landed as
https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d
Cr-Commit-Position: refs/heads/master@{#404973}