Hide webkitPersistentStorage infobar on navigation

Hide webkitPersistentStorage infobar on navigation

The infobar for navigator.webkitPersistentStorage.requestQuota doesn't
disappear even if the user navigates away from a page.

Updated ShouldExpireInternal() to remove infobar on navigation.

BUG=350029
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274737

[Nikhil, 2014-05-30 15:35:23]

PTAL. Thanks!

Adding meacer being issue owner, and sky based on presubmit suggestion. My
apologies in advance if you feel someone else should be added.

If you have any review comments, please let me know and I'll incorporate them :)

[meacer, 2014-05-30 17:44:21]

Thanks for picking this up!

https://codereview.chromium.org/308673007/diff/1/chrome/browser/chrome_quota_...
File chrome/browser/chrome_quota_permission_context.cc (right):

https://codereview.chromium.org/308673007/diff/1/chrome/browser/chrome_quota_...
chrome/browser/chrome_quota_permission_context.cc:216: bool
RequestQuotaInfoBarDelegate::ShouldExpireInternal(
You should be able to remove this override altogether.

[Nikhil, 2014-06-02 13:22:39]

On 2014/05/30 17:44:21, Mustafa Emre Acer wrote:
> Thanks for picking this up!
> 
>
https://codereview.chromium.org/308673007/diff/1/chrome/browser/chrome_quota_...
> File chrome/browser/chrome_quota_permission_context.cc (right):
> 
>
https://codereview.chromium.org/308673007/diff/1/chrome/browser/chrome_quota_...
> chrome/browser/chrome_quota_permission_context.cc:216: bool
> RequestQuotaInfoBarDelegate::ShouldExpireInternal(
> You should be able to remove this override altogether.

The current implementation is similar to other classes (for eg
SimpleAlertInfoBarDelegate) except no unique identifier or boolean is maintained
as of now. If you think we should remove ShouldExpireInternal() completely then
please let me know. I've locally tested removing the override completely and it
works fine.

[meacer, 2014-06-02 17:55:56]

On 2014/06/02 13:22:39, Nikhil wrote:
> On 2014/05/30 17:44:21, Mustafa Emre Acer wrote:
> > Thanks for picking this up!
> > 
> >
>
https://codereview.chromium.org/308673007/diff/1/chrome/browser/chrome_quota_...
> > File chrome/browser/chrome_quota_permission_context.cc (right):
> > 
> >
>
https://codereview.chromium.org/308673007/diff/1/chrome/browser/chrome_quota_...
> > chrome/browser/chrome_quota_permission_context.cc:216: bool
> > RequestQuotaInfoBarDelegate::ShouldExpireInternal(
> > You should be able to remove this override altogether.
> 
> The current implementation is similar to other classes (for eg
> SimpleAlertInfoBarDelegate) except no unique identifier or boolean is
maintained
> as of now. If you think we should remove ShouldExpireInternal() completely
then
> please let me know. I've locally tested removing the override completely and
it
> works fine.

Right, let's remove the override here since it's delegating directly to the base
class.

[Nikhil, 2014-06-03 06:36:02]

Thanks for reviewing! I've removed the override. PTAL. Thanks

https://codereview.chromium.org/308673007/diff/1/chrome/browser/chrome_quota_...
File chrome/browser/chrome_quota_permission_context.cc (right):

https://codereview.chromium.org/308673007/diff/1/chrome/browser/chrome_quota_...
chrome/browser/chrome_quota_permission_context.cc:216: bool
RequestQuotaInfoBarDelegate::ShouldExpireInternal(
On 2014/05/30 17:44:21, Mustafa Emre Acer wrote:
> You should be able to remove this override altogether.

Done.

[meacer, 2014-06-03 18:14:40]

On 2014/06/03 06:36:02, Nikhil wrote:
> Thanks for reviewing! I've removed the override. PTAL. Thanks
> 
>
https://codereview.chromium.org/308673007/diff/1/chrome/browser/chrome_quota_...
> File chrome/browser/chrome_quota_permission_context.cc (right):
> 
>
https://codereview.chromium.org/308673007/diff/1/chrome/browser/chrome_quota_...
> chrome/browser/chrome_quota_permission_context.cc:216: bool
> RequestQuotaInfoBarDelegate::ShouldExpireInternal(
> On 2014/05/30 17:44:21, Mustafa Emre Acer wrote:
> > You should be able to remove this override altogether.
> 
> Done.

Lgtm, thanks.

[Nikhil, 2014-06-03 18:41:13]

The CQ bit was checked by n.bansal@samsung.com

[commit-bot: I haz the power, 2014-06-03 18:42:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/n.bansal@samsung.com/308673007/20001

[meacer, 2014-06-03 18:45:35]

(You'll still need an lg tm from sky or someone else for OWNERS)

[sky, 2014-06-03 20:09:17]

LGTM

[commit-bot: I haz the power, 2014-06-03 22:52:37]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-06-03 23:33:28]

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

[commit-bot: I haz the power, 2014-06-03 23:33:29]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[Nikhil, 2014-06-04 04:54:24]

The CQ bit was checked by n.bansal@samsung.com

[commit-bot: I haz the power, 2014-06-04 04:55:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/n.bansal@samsung.com/308673007/20001

[commit-bot: I haz the power, 2014-06-04 06:14:18]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[Nikhil, 2014-06-04 07:57:30]

On 2014/06/04 06:14:18, I haz the power (commit-bot) wrote:
> FYI, CQ is re-trying this CL (attempt #1).
> The failing builders are:
>   android_dbg_triggered_tests on tryserver.chromium
>
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

Could you please help me understand why is bot failing? I'm not very sure :/

[commit-bot: I haz the power, 2014-06-04 09:07:55]

Change committed as 274737