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

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:
2 months, 2 weeks ago by arthursonzogni OOO until tue
Modified:
2 months 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 (OOO until 5-2), nasko
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 OOO until tue
Hi Camille, Please, could you take a look?
2 months, 1 week 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 ...
2 months, 1 week ago (2017-02-16 13:45:59 UTC) #14
arthursonzogni OOO until tue
Thanks Camille. I have the same failure with and without this patch, but I only ...
2 months, 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 ...
2 months, 1 week ago (2017-02-21 15:32:25 UTC) #32
arthursonzogni OOO until tue
Thanks Camille. It works % these two things that might be unrelated. * I found ...
2 months, 1 week 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 months, 1 week ago (2017-02-22 16:40:16 UTC) #34
arthursonzogni OOO until tue
> The bug looks weird, might not be PlzNavigate related? I don't know, it is ...
2 months, 1 week 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 ...
2 months ago (2017-02-23 12:09:52 UTC) #36
arthursonzogni OOO until tue
On 2017/02/23 12:09:52, clamy wrote: > On 2017/02/22 17:31:13, arthursonzogni wrote: > > > The ...
2 months ago (2017-02-23 14:23:43 UTC) #37
arthursonzogni OOO until tue
On 2017/02/23 14:23:43, arthursonzogni wrote: > On 2017/02/23 12:09:52, clamy wrote: > > On 2017/02/22 ...
2 months ago (2017-02-23 14:51:08 UTC) #38
arthursonzogni OOO until tue
So, there is no more problems with PlzNavigate when an error page to the ChromeWebStore ...
2 months 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
2 months ago (2017-02-24 13:36:09 UTC) #60
commit-bot: I haz the power
2 months 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 cc6ac46