Use WebContents::GetManifest in ServiceWorker FetchTest instead of RequestAppBannerFromDevTools

Use WebContents::GetManifest in ServiceWorker FetchTest instead of RequestAppBannerFromDevTools

This is blocking the refactoring of AppBannerDataFetcher.
https://codereview.chromium.org/2156113002/

BUG=628921
Committed: https://crrev.com/482eeff448654b3eec603009deee13d402941e31
Cr-Commit-Position: refs/heads/master@{#406408}

[horo, 2016-07-19 02:29:34]

The CQ bit was checked by horo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 02:30:08]

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

[horo, 2016-07-19 02:30:11]

Description was changed from

==========
Use WebContents::GetManifest in ServiceWorkerManifestFetchTest instead of
RequestAppBannerFromDevTools

BUG=
==========

to

==========
Use WebContents::GetManifest in ServiceWorker FetchTest instead of
RequestAppBannerFromDevTools

BUG=
==========

[horo, 2016-07-19 02:32:44]

horo@chromium.org changed reviewers:
+ nhiroki@chromium.org

[horo, 2016-07-19 02:32:44]

nhiroki@
Could you please review this?

[dominickn, 2016-07-19 03:06:27]

Thanks for this! I just have a naming suggestion.

https://codereview.chromium.org/2158253002/diff/1/chrome/browser/chrome_servi...
File chrome/browser/chrome_service_worker_browsertest.cc (right):

https://codereview.chromium.org/2158253002/diff/1/chrome/browser/chrome_servi...
chrome/browser/chrome_service_worker_browsertest.cc:270: std::string
RequestAppBannerAndGetIssuedRequests() {
Nit: rename this GetManifestAndIssuedRequests?

[horo, 2016-07-19 03:13:06]

The CQ bit was checked by horo@chromium.org to run a CQ dry run

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

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

[horo, 2016-07-19 03:13:40]

Thank you.

https://codereview.chromium.org/2158253002/diff/1/chrome/browser/chrome_servi...
File chrome/browser/chrome_service_worker_browsertest.cc (right):

https://codereview.chromium.org/2158253002/diff/1/chrome/browser/chrome_servi...
chrome/browser/chrome_service_worker_browsertest.cc:270: std::string
RequestAppBannerAndGetIssuedRequests() {
On 2016/07/19 03:06:27, dominickn wrote:
> Nit: rename this GetManifestAndIssuedRequests?

Done.

[nhiroki, 2016-07-19 03:40:56]

Can you fill in the BUG= line? I'd like to know the context of this change.

[horo, 2016-07-19 03:46:49]

Description was changed from

==========
Use WebContents::GetManifest in ServiceWorker FetchTest instead of
RequestAppBannerFromDevTools

BUG=
==========

to

==========
Use WebContents::GetManifest in ServiceWorker FetchTest instead of
RequestAppBannerFromDevTools

This is blocking the mojofication of ManifestManager.
https://codereview.chromium.org/1913043002

BUG=577685
==========

[horo, 2016-07-19 03:46:57]

On 2016/07/19 03:40:56, nhiroki (slow) wrote:
> Can you fill in the BUG= line? I'd like to know the context of this change.

done

[horo, 2016-07-19 03:48:59]

Description was changed from

==========
Use WebContents::GetManifest in ServiceWorker FetchTest instead of
RequestAppBannerFromDevTools

This is blocking the mojofication of ManifestManager.
https://codereview.chromium.org/1913043002

BUG=577685
==========

to

==========
Use WebContents::GetManifest in ServiceWorker FetchTest instead of
RequestAppBannerFromDevTools

This is refactoring of AppBannerDataFetcher.
https://codereview.chromium.org/2156113002/

BUG=628921
==========

[horo, 2016-07-19 03:49:06]

Description was changed from

==========
Use WebContents::GetManifest in ServiceWorker FetchTest instead of
RequestAppBannerFromDevTools

This is refactoring of AppBannerDataFetcher.
https://codereview.chromium.org/2156113002/

BUG=628921
==========

to

==========
Use WebContents::GetManifest in ServiceWorker FetchTest instead of
RequestAppBannerFromDevTools

This is blocking the refactoring of AppBannerDataFetcher.
https://codereview.chromium.org/2156113002/

BUG=628921
==========

[dominickn, 2016-07-19 03:53:01]

On 2016/07/19 03:40:56, nhiroki (slow) wrote:
> Can you fill in the BUG= line? I'd like to know the context of this change.

I'm refactoring app banners in crrev.com/2156113002, and this test is using
RequestAppBannerFromDevtools to kick off a manifest fetch. However, my refactor
changes the banner flow and causes these tests to hang.

WebContents::GetManifest is the canonical way to initiate a manifest fetch, so
this CL is changing to use that. Generally, people shouldn't rely on the app
banner implementation to fetch a manifest - they should fetch the manifest. :)

[commit-bot: I haz the power, 2016-07-19 04:14:02]

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

[commit-bot: I haz the power, 2016-07-19 04:14:03]

Dry run: This issue passed the CQ dry run.

[nhiroki, 2016-07-19 05:08:00]

Thank you! LGTM

[horo, 2016-07-19 05:21:19]

horo@chromium.org changed reviewers:
+ jochen@chromium.org

[horo, 2016-07-19 05:21:20]

jochen@
Could you please review this?

[jochen (gone - plz use gerrit), 2016-07-19 12:25:21]

lgtm

[horo, 2016-07-19 22:58:11]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2016-07-19 22:59:08]

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-19 23:07:12]

Description was changed from

==========
Use WebContents::GetManifest in ServiceWorker FetchTest instead of
RequestAppBannerFromDevTools

This is blocking the refactoring of AppBannerDataFetcher.
https://codereview.chromium.org/2156113002/

BUG=628921
==========

to

==========
Use WebContents::GetManifest in ServiceWorker FetchTest instead of
RequestAppBannerFromDevTools

This is blocking the refactoring of AppBannerDataFetcher.
https://codereview.chromium.org/2156113002/

BUG=628921
==========

[commit-bot: I haz the power, 2016-07-19 23:07:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-19 23:10:27]

Description was changed from

==========
Use WebContents::GetManifest in ServiceWorker FetchTest instead of
RequestAppBannerFromDevTools

This is blocking the refactoring of AppBannerDataFetcher.
https://codereview.chromium.org/2156113002/

BUG=628921
==========

to

==========
Use WebContents::GetManifest in ServiceWorker FetchTest instead of
RequestAppBannerFromDevTools

This is blocking the refactoring of AppBannerDataFetcher.
https://codereview.chromium.org/2156113002/

BUG=628921

Committed: https://crrev.com/482eeff448654b3eec603009deee13d402941e31
Cr-Commit-Position: refs/heads/master@{#406408}
==========

[commit-bot: I haz the power, 2016-07-19 23:10:28]

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