New issue
Advanced search Search tips

Issue 626969 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Remove WebContents::HasManifest

Project Member Reported by dominickn@chromium.org, Jul 11 2016

Issue description

WebContents::GetManifest now returns a manifest URL and a manifest object. Instead of the duplicate IPC calls, we can simply use the emptiness of the URL from GetManifest to indicate that a site has no manifest.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea77d54f9f92549a5f565eff096caeab1247b22d

commit ea77d54f9f92549a5f565eff096caeab1247b22d
Author: dominickn <dominickn@chromium.org>
Date: Wed Jul 13 03:42:00 2016

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 

Review-Url: https://codereview.chromium.org/2140463002
Cr-Commit-Position: refs/heads/master@{#404973}

[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/chrome/browser/banners/app_banner_data_fetcher.cc
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/chrome/browser/banners/app_banner_data_fetcher.h
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/browser/manifest/manifest_browsertest.cc
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/browser/manifest/manifest_manager_host.cc
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/browser/manifest/manifest_manager_host.h
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/common/manifest_manager_messages.h
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/public/browser/web_contents.h
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/renderer/manifest/manifest_manager.cc
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/renderer/manifest/manifest_manager.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 13 2016

Labels: merge-merged-2795
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea77d54f9f92549a5f565eff096caeab1247b22d

commit ea77d54f9f92549a5f565eff096caeab1247b22d
Author: dominickn <dominickn@chromium.org>
Date: Wed Jul 13 03:42:00 2016

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 

Review-Url: https://codereview.chromium.org/2140463002
Cr-Commit-Position: refs/heads/master@{#404973}

[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/chrome/browser/banners/app_banner_data_fetcher.cc
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/chrome/browser/banners/app_banner_data_fetcher.h
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/browser/manifest/manifest_browsertest.cc
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/browser/manifest/manifest_manager_host.cc
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/browser/manifest/manifest_manager_host.h
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/common/manifest_manager_messages.h
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/public/browser/web_contents.h
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/renderer/manifest/manifest_manager.cc
[modify] https://crrev.com/ea77d54f9f92549a5f565eff096caeab1247b22d/content/renderer/manifest/manifest_manager.h

Status: Fixed (was: Started)
Components: -Manifest Blink>AppManifest
Deprecate (and bulk edit/ move) Manifest

Sign in to add a comment