Do not expire permission bubbles for in-page navigations.

chrome/browser/ui/website_settings/permission_bubble_manager.cc

Index: chrome/browser/ui/website_settings/permission_bubble_manager.cc
diff --git a/chrome/browser/ui/website_settings/permission_bubble_manager.cc b/chrome/browser/ui/website_settings/permission_bubble_manager.cc
index 6d62d7a42d2d64dadadef47001164f56009dd3e8..a2ace674da4eeb7e68fd9d0f0648eea5cbe140ae 100644
--- a/chrome/browser/ui/website_settings/permission_bubble_manager.cc
+++ b/chrome/browser/ui/website_settings/permission_bubble_manager.cc
@@ -273,7 +273,8 @@ void PermissionBubbleManager::NavigationEntryCommitted(
   // the navigation is really to the same page.
   if ((request_url_.GetAsReferrer() !=
        web_contents()->GetLastCommittedURL().GetAsReferrer()) ||
-      details.type == content::NAVIGATION_TYPE_EXISTING_PAGE) {
+      (details.type == content::NAVIGATION_TYPE_EXISTING_PAGE &&
+       !details.is_in_page)) {

[mlamouri (slow - plz ping), 2015/08/04 11:56:38]

Why not use ::DidNavigateAnyFrame() to merge this and
main_frame_has_fully_loaded_ reset?

[Avi (use Gerrit), 2015/08/04 14:28:16]

Because I don't know this code well, and I was trying to make the smallest
change to keep the intent the same. If you know this code better, and think
that's a better choice, please do so.

[hcarmona, 2015/08/04 23:10:44]

It looks like using GetAsReferrer to compare the request and previous
URLs is trying to achieve the same as details.is_in_page.

I think it might be best to change this if condition to:

if (!details.is_in_page ||
    details.type == content::NAVIGATION_TYPE_SAME_PAGE ||
    details.type == content::NAVIGATION_TYPE_EXISTING_PAGE)

Because we want to clear permissions if we navigate to a different URL,
explicitly navigate to the same page, or refresh the page.

[Avi (use Gerrit), 2015/08/05 15:11:48]

But that's different behavior. The bug calls out going back over an in-page
navigation which is EXISTING_PAGE + is_in_page.

If you want to argue that the current behavior is incorrect, that's fine with
me; I'm no expert. Go make a CL to do it. But my change _restored_ the behavior
that this had before my change.

[hcarmona, 2015/08/05 18:51:04]

Interesting. I didn't know there were cases where the permission bubbles
should survive back/forward navigation, but it does makes sense.

I can make a change to get to the correct behavior, but I want to
clarify what that is exactly. Are there other cases I'm not aware of?
go/when-to-dismiss-permissions

     // Kill off existing bubble and cancel any pending requests.
     CancelPendingQueues();
     FinalizeBubble();