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

Issue 11293157: Add a new ContentRendererClient::HandleNavigation callback in RenderViewImpl::decidePolicyForNaviga… (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 4 months ago by Marshall
Modified:
2 years, 4 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org
Visibility:
Public.

Description

Add a new ContentRendererClient::HandleNavigation callback in RenderViewImpl::decidePolicyForNavigation that allows the client to intercept/ignore any navigation request. BUG=159923 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=167694

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Total comments: 7

Patch Set 3 : #

Total comments: 2

Patch Set 4 : #

Messages

Total messages: 15 (0 generated)
Marshall
Please review. jam=FYI
2 years, 4 months ago (2012-11-07 23:16:32 UTC) #1
Charlie Reis
See my comments on the bug about whether we should add this API, but here's ...
2 years, 4 months ago (2012-11-08 19:06:12 UTC) #2
Marshall
https://codereview.chromium.org/11293157/diff/1/content/renderer/render_view_impl.cc File content/renderer/render_view_impl.cc (right): https://codereview.chromium.org/11293157/diff/1/content/renderer/render_view_impl.cc#newcode2677 content/renderer/render_view_impl.cc:2677: if (GetContentClient()->renderer()->HandleNavigation(frame, request, type, On 2012/11/08 19:06:12, creis wrote: ...
2 years, 4 months ago (2012-11-08 19:15:59 UTC) #3
Charlie Reis
I'm ok with this behavior, with a few nits below. I'm going to defer to ...
2 years, 4 months ago (2012-11-08 19:24:51 UTC) #4
Marshall
https://codereview.chromium.org/11293157/diff/6001/content/public/renderer/content_renderer_client.h File content/public/renderer/content_renderer_client.h (right): https://codereview.chromium.org/11293157/diff/6001/content/public/renderer/content_renderer_client.h#newcode135 content/public/renderer/content_renderer_client.h:135: // Returns true if the navigation was handled and ...
2 years, 4 months ago (2012-11-08 19:32:36 UTC) #5
jam
On 2012/11/08 19:24:51, creis wrote: > I'm ok with this behavior, with a few nits ...
2 years, 4 months ago (2012-11-08 21:10:07 UTC) #6
Charlie Reis
LGTM, then.
2 years, 4 months ago (2012-11-08 22:04:17 UTC) #7
darin (slow to review)
https://codereview.chromium.org/11293157/diff/8002/content/public/renderer/content_renderer_client.h File content/public/renderer/content_renderer_client.h (right): https://codereview.chromium.org/11293157/diff/8002/content/public/renderer/content_renderer_client.h#newcode137 content/public/renderer/content_renderer_client.h:137: virtual bool HandleNavigation(WebKit::WebFrame* frame, One question: How do Chromium ...
2 years, 4 months ago (2012-11-09 00:30:24 UTC) #8
jam
https://codereview.chromium.org/11293157/diff/8002/content/public/renderer/content_renderer_client.h File content/public/renderer/content_renderer_client.h (right): https://codereview.chromium.org/11293157/diff/8002/content/public/renderer/content_renderer_client.h#newcode137 content/public/renderer/content_renderer_client.h:137: virtual bool HandleNavigation(WebKit::WebFrame* frame, On 2012/11/09 00:30:24, darin wrote: ...
2 years, 4 months ago (2012-11-09 00:32:37 UTC) #9
darin (slow to review)
On Thu, Nov 8, 2012 at 4:32 PM, <jam@chromium.org> wrote: > > https://codereview.chromium.**org/11293157/diff/8002/** > content/public/renderer/**content_renderer_client.h<https://codereview.chromium.org/11293157/diff/8002/content/public/renderer/content_renderer_client.h> ...
2 years, 4 months ago (2012-11-09 08:00:54 UTC) #10
jam
On 2012/11/09 08:00:54, darin wrote: > On Thu, Nov 8, 2012 at 4:32 PM, <mailto:jam@chromium.org> ...
2 years, 4 months ago (2012-11-09 17:23:10 UTC) #11
Marshall
On 2012/11/09 17:23:10, John Abd-El-Malek wrote: > Another option can be to comment the function ...
2 years, 4 months ago (2012-11-09 18:40:05 UTC) #12
darin (slow to review)
On Fri, Nov 9, 2012 at 9:23 AM, <jam@chromium.org> wrote: > On 2012/11/09 08:00:54, darin ...
2 years, 4 months ago (2012-11-12 21:26:34 UTC) #13
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/marshall@chromium.org/11293157/5004
2 years, 4 months ago (2012-11-14 16:05:45 UTC) #14
I haz the power (commit-bot)
2 years, 4 months ago (2012-11-14 17:44:06 UTC) #15
Change committed as 167694
Sign in to reply to this message.

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