Don't report blocked web contents from already navigated render views.

Don't report blocked web contents from already navigated render views.

BUG=427477
R=creis@chromium.org
TEST=browser_tests:PopupBlockerTest.Regress427477

Committed: https://crrev.com/3f52f665bbe0041df93210c0114aeba3e5688f89
Cr-Commit-Position: refs/heads/master@{#303615}

[Charlie Reis, 2014-11-10 17:41:26]

Thanks!  LGTM, with a TODO.

Please update the CL title, though-- I'm not sure what "Don't record" means. 
This is more about not opening windows after a cross-process navigation.

https://codereview.chromium.org/707223004/diff/1/chrome/browser/chrome_conten...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/707223004/diff/1/chrome/browser/chrome_conten...
chrome/browser/chrome_content_browser_client.cc:518: RenderViewHost*
render_view_host =
RenderViewHost is going away, and there can be more than one of them per tab in
--site-per-process when some frames are out-of-process.

Can you add a TODO to make this use RenderFrameHosts (which appears to require
changes to BlockedWindowParams and ViewHostMsg_CreateWindow_Params)?  You can
cite bug 431769.

https://codereview.chromium.org/707223004/diff/1/chrome/browser/ui/blocked_co...
File chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc (right):

https://codereview.chromium.org/707223004/diff/1/chrome/browser/ui/blocked_co...
chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc:506: // The popup
from th unload event handler should not show up for about:blank.
nit: th -> the

[jochen (gone - plz use gerrit), 2014-11-10 19:39:13]

the windows get opened (which is for better or worse the intended behavior of
allowed popups in onunload handlers). The bug is about popups getting blocked,
and the fact that they were blocked getting recorded for the wrong history
entry.

The HandleBlockedPopupOnUIThread() just records the blocked popup in the per-tab
popup helper.

Would "Don't report..." be easier to understand?

https://codereview.chromium.org/707223004/diff/1/chrome/browser/chrome_conten...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/707223004/diff/1/chrome/browser/chrome_conten...
chrome/browser/chrome_content_browser_client.cc:518: RenderViewHost*
render_view_host =
On 2014/11/10 at 17:41:26, Charlie Reis wrote:
> RenderViewHost is going away, and there can be more than one of them per tab
in --site-per-process when some frames are out-of-process.
> 
> Can you add a TODO to make this use RenderFrameHosts (which appears to require
changes to BlockedWindowParams and ViewHostMsg_CreateWindow_Params)?  You can
cite bug 431769.

done

https://codereview.chromium.org/707223004/diff/1/chrome/browser/ui/blocked_co...
File chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc (right):

https://codereview.chromium.org/707223004/diff/1/chrome/browser/ui/blocked_co...
chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc:506: // The popup
from th unload event handler should not show up for about:blank.
On 2014/11/10 at 17:41:26, Charlie Reis wrote:
> nit: th -> the

done

[Charlie Reis, 2014-11-10 20:58:47]

Hmm, I'm somewhat less happy about the fix, then.  That means we still open
popups from unload handlers (after the new page is showing) if the user has
popup-blocking disabled.  I can't think of a way to cause serious problems with
that, though, so this UI-only fix still LGTM.

And yes, "Don't report blocked popups after a cross-process navigation" would be
an easier title for me to understand.  Thanks.

[jochen (gone - plz use gerrit), 2014-11-11 08:15:54]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2014-11-11 08:16:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/707223004/20001

[commit-bot: I haz the power, 2014-11-11 09:07:19]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-11-11 09:08:11]

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