Remove AppBannerManager::event_request_id().

Remove AppBannerManager::event_request_id().

This was previously used to ensure we did not get a resolution to the
beforeinstallpromptevent that didn't match the current request. However,
AppBannerManager wipes its weak pointers when the current request
finishes, meaning that event_request_id is unneccessary.

BUG=728022
Review-Url: https://codereview.chromium.org/2969163002
Cr-Commit-Position: refs/heads/master@{#484820}
Committed: https://chromium.googlesource.com/chromium/src/+/c5bed25edfa875b182516849e1d36b64bce464e9

[dominickn, 2017-07-05 04:18:59]

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

[commit-bot: I haz the power, 2017-07-05 04:19:15]

Dry run: 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, 2017-07-05 04:55:26]

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

[commit-bot: I haz the power, 2017-07-05 04:55:27]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[dominickn, 2017-07-06 01:13:11]

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

[dominickn, 2017-07-06 01:13:11]

PTAL. thanks!

[dominickn, 2017-07-06 01:23:54]

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

[commit-bot: I haz the power, 2017-07-06 01:24:09]

Dry run: CQ is trying da patch.

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

[dominickn, 2017-07-06 01:48:02]

Description was changed from

==========
Remove AppBannerManager::event_request_id().

This was previously used to ensure we did not get a resolution to the
beforeinstallpromptevent that didn't match the current request. However,
AppBannerManager wipes its weak pointers when the current request
finishes, meaning that event_request_id is unneccessary.

BUG=728022
==========

to

==========
Remove AppBannerManager::event_request_id().

This was previously used to ensure we did not get a resolution to the
beforeinstallpromptevent that didn't match the current request. However,
AppBannerManager wipes its weak pointers when the current request
finishes, meaning that event_request_id is unneccessary.

BUG=728022
==========

[commit-bot: I haz the power, 2017-07-06 02:33:02]

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

[commit-bot: I haz the power, 2017-07-06 02:33:03]

Dry run: This issue passed the CQ dry run.

[benwells, 2017-07-06 05:00:35]

https://codereview.chromium.org/2969163002/diff/20001/chrome/browser/banners/...
File chrome/browser/banners/app_banner_manager.cc (right):

https://codereview.chromium.org/2969163002/diff/20001/chrome/browser/banners/...
chrome/browser/banners/app_banner_manager.cc:67:
weak_factory_.InvalidateWeakPtrs();
Here we just invalidate weak pointers, but don't reset the event / close the
binding etc. Shouldn't we do that too?

https://codereview.chromium.org/2969163002/diff/20001/chrome/browser/banners/...
File chrome/browser/banners/app_banner_manager.h (right):

https://codereview.chromium.org/2969163002/diff/20001/chrome/browser/banners/...
chrome/browser/banners/app_banner_manager.h:290: int event_request_id_;
Is this still needed?

[dominickn, 2017-07-06 05:36:08]

Thanks!

https://codereview.chromium.org/2969163002/diff/20001/chrome/browser/banners/...
File chrome/browser/banners/app_banner_manager.cc (right):

https://codereview.chromium.org/2969163002/diff/20001/chrome/browser/banners/...
chrome/browser/banners/app_banner_manager.cc:67:
weak_factory_.InvalidateWeakPtrs();
On 2017/07/06 05:00:35, benwells wrote:
> Here we just invalidate weak pointers, but don't reset the event / close the
> binding etc. Shouldn't we do that too?

Good point, done.

https://codereview.chromium.org/2969163002/diff/20001/chrome/browser/banners/...
File chrome/browser/banners/app_banner_manager.h (right):

https://codereview.chromium.org/2969163002/diff/20001/chrome/browser/banners/...
chrome/browser/banners/app_banner_manager.h:290: int event_request_id_;
On 2017/07/06 05:00:35, benwells wrote:
> Is this still needed?

Oops. Removed.

[benwells, 2017-07-06 07:22:40]

lgtm with a nit.

https://codereview.chromium.org/2969163002/diff/40001/chrome/browser/banners/...
File chrome/browser/banners/app_banner_manager.cc (right):

https://codereview.chromium.org/2969163002/diff/40001/chrome/browser/banners/...
chrome/browser/banners/app_banner_manager.cc:67:
weak_factory_.InvalidateWeakPtrs();
Nit: can you factor this out into a separate function?

[dominickn, 2017-07-07 00:20:22]

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

[commit-bot: I haz the power, 2017-07-07 00:20:36]

Dry run: CQ is trying da patch.

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

[dominickn, 2017-07-07 00:35:40]

Thanks!

https://codereview.chromium.org/2969163002/diff/40001/chrome/browser/banners/...
File chrome/browser/banners/app_banner_manager.cc (right):

https://codereview.chromium.org/2969163002/diff/40001/chrome/browser/banners/...
chrome/browser/banners/app_banner_manager.cc:67:
weak_factory_.InvalidateWeakPtrs();
On 2017/07/06 07:22:39, benwells wrote:
> Nit: can you factor this out into a separate function?

Done.

[commit-bot: I haz the power, 2017-07-07 01:03:18]

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

[commit-bot: I haz the power, 2017-07-07 01:03:19]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[dominickn, 2017-07-07 01:07:36]

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

[commit-bot: I haz the power, 2017-07-07 01:07:44]

Dry run: 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, 2017-07-07 03:29:19]

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

[commit-bot: I haz the power, 2017-07-07 03:29:20]

Dry run: This issue passed the CQ dry run.

[dominickn, 2017-07-07 03:36:51]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2017-07-07 03:36:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
benwells@chromium.org
Link to the patchset: https://codereview.chromium.org/2969163002/#ps60001
(title: "Refactor into method")

[commit-bot: I haz the power, 2017-07-07 03:37:09]

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, 2017-07-07 03:42:49]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1499398611473700,
"parent_rev": "25ff9b7cb09ab7db6dd453c5d5c43779a58c8159", "commit_rev":
"c5bed25edfa875b182516849e1d36b64bce464e9"}

[commit-bot: I haz the power, 2017-07-07 03:43:07]

Description was changed from

==========
Remove AppBannerManager::event_request_id().

This was previously used to ensure we did not get a resolution to the
beforeinstallpromptevent that didn't match the current request. However,
AppBannerManager wipes its weak pointers when the current request
finishes, meaning that event_request_id is unneccessary.

BUG=728022
==========

to

==========
Remove AppBannerManager::event_request_id().

This was previously used to ensure we did not get a resolution to the
beforeinstallpromptevent that didn't match the current request. However,
AppBannerManager wipes its weak pointers when the current request
finishes, meaning that event_request_id is unneccessary.

BUG=728022

Review-Url: https://codereview.chromium.org/2969163002
Cr-Commit-Position: refs/heads/master@{#484820}
Committed:
https://chromium.googlesource.com/chromium/src/+/c5bed25edfa875b182516849e1d3...
==========

[commit-bot: I haz the power, 2017-07-07 03:43:09]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/c5bed25edfa875b182516849e1d3...