Chromium Code Reviews
Help | Chromium Project | Sign in
(24)

Issue 2697713005: DCHECK: Browser asking renderer to commit URLs it is not allowed to. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 2 days ago by arthursonzogni
Modified:
12 hours, 35 minutes ago
Reviewers:
clamy
CC:
chromium-reviews, jam, nasko+codewatch_chromium.org, darin-cc_chromium.org, creis+watch_chromium.org, mlamouri+watch-content_chromium.org, alexmos, nasko (out until Feb 27th)
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

DCHECK: Browser asking renderer to commit URLs it is not allowed to. This CL adds a DCHECK when the browser ask the renderer to commit an URL and then kill the renderer when it does. This CL was previously related with a problem when the renderer tries to commit an error page to the chrome webstore inside an iframe. It turns out that there was no more problems with PlzNavigate since the ChromeWebStore error page uses a different process. BUG=588314, 622385 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2697713005 Cr-Commit-Position: refs/heads/master@{#452808} Committed: https://chromium.googlesource.com/chromium/src/+/4ba80de54a09324ae69bde773f194df0eec6e8b3

Patch Set 1 #

Patch Set 2 : I forgot to remove the change in render_frame_impl #

Total comments: 4

Patch Set 3 : Addressed comments clamy@ #

Patch Set 4 : Move things into the NavigationRequest. #

Total comments: 2

Patch Set 5 : Move implementation of CanCommitURL. #

Patch Set 6 : Turns the condition into a DCHECK. #

Patch Set 7 : git cl try #

Messages

Total messages: 63 (50 generated)
arthursonzogni
Hi Camille, Please, could you take a look?
1 week, 1 day ago (2017-02-16 08:55:38 UTC) #13
clamy
Thanks! A few comments below. Also, it seems quite a few tests are failing on ...
1 week, 1 day ago (2017-02-16 13:45:59 UTC) #14
arthursonzogni
Thanks Camille. I have the same failure with and without this patch, but I only ...
1 week ago (2017-02-17 09:33:20 UTC) #17
clamy
https://codereview.chromium.org/2697713005/diff/20001/content/browser/frame_host/render_frame_host_impl.cc File content/browser/frame_host/render_frame_host_impl.cc (right): https://codereview.chromium.org/2697713005/diff/20001/content/browser/frame_host/render_frame_host_impl.cc#newcode2774 content/browser/frame_host/render_frame_host_impl.cc:2774: CommonNavigationParams new_common_params; On 2017/02/17 09:33:19, arthursonzogni wrote: > On ...
3 days, 10 hours ago (2017-02-21 15:32:25 UTC) #32
arthursonzogni
Thanks Camille. It works % these two things that might be unrelated. * I found ...
2 days, 9 hours ago (2017-02-22 16:25:03 UTC) #33
clamy
Thanks! Current version lgtm with one comment. The bug looks weird, might not be PlzNavigate ...
2 days, 9 hours ago (2017-02-22 16:40:16 UTC) #34
arthursonzogni
> The bug looks weird, might not be PlzNavigate related? I don't know, it is ...
2 days, 8 hours ago (2017-02-22 17:31:13 UTC) #35
clamy
On 2017/02/22 17:31:13, arthursonzogni wrote: > > The bug looks weird, might not be PlzNavigate ...
1 day, 14 hours ago (2017-02-23 12:09:52 UTC) #36
arthursonzogni
On 2017/02/23 12:09:52, clamy wrote: > On 2017/02/22 17:31:13, arthursonzogni wrote: > > > The ...
1 day, 11 hours ago (2017-02-23 14:23:43 UTC) #37
arthursonzogni
On 2017/02/23 14:23:43, arthursonzogni wrote: > On 2017/02/23 12:09:52, clamy wrote: > > On 2017/02/22 ...
1 day, 11 hours ago (2017-02-23 14:51:08 UTC) #38
arthursonzogni
So, there is no more problems with PlzNavigate when an error page to the ChromeWebStore ...
1 day, 9 hours ago (2017-02-23 17:05:44 UTC) #43
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697713005/180001
12 hours, 39 minutes ago (2017-02-24 13:36:09 UTC) #60
commit-bot: I haz the power
12 hours, 35 minutes ago (2017-02-24 13:40:16 UTC) #63
Message was sent while issue was closed.
Committed patchset #7 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/4ba80de54a09324ae69bde773f19...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f8e48bd