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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 5 months ago by Marshall
Modified:
1 year, 5 months ago
Reviewers:
Charlie Reis, jam, darin
CC:
chromium-reviews_chromium.org, 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 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -0 lines) Lint Patch
M content/public/renderer/content_renderer_client.h View 1 2 3 2 chunks +10 lines, -0 lines 0 comments 0 errors Download
M content/public/renderer/content_renderer_client.cc View 1 chunk +9 lines, -0 lines 0 comments 0 errors Download
M content/renderer/render_view_impl.cc View 1 2 1 chunk +7 lines, -0 lines 0 comments 0 errors Download
Commit:

Messages

Total messages: 15
Marshall
Please review. jam=FYI
1 year, 5 months ago #1
Charlie Reis
See my comments on the bug about whether we should add this API, but here's ...
1 year, 5 months ago #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: ...
1 year, 5 months ago #3
Charlie Reis
I'm ok with this behavior, with a few nits below. I'm going to defer to ...
1 year, 5 months ago #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 ...
1 year, 5 months ago #5
jam
On 2012/11/08 19:24:51, creis wrote: > I'm ok with this behavior, with a few nits ...
1 year, 5 months ago #6
Charlie Reis
LGTM, then.
1 year, 5 months ago #7
darin
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 ...
1 year, 5 months ago #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: ...
1 year, 5 months ago #9
darin
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> ...
1 year, 5 months ago #10
jam
On 2012/11/09 08:00:54, darin wrote: > On Thu, Nov 8, 2012 at 4:32 PM, <mailto:jam@chromium.org> ...
1 year, 5 months ago #11
Marshall
On 2012/11/09 17:23:10, John Abd-El-Malek wrote: > Another option can be to comment the function ...
1 year, 5 months ago #12
darin
On Fri, Nov 9, 2012 at 9:23 AM, <jam@chromium.org> wrote: > On 2012/11/09 08:00:54, darin ...
1 year, 5 months ago #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
1 year, 5 months ago #14
I haz the power (commit-bot)
1 year, 5 months ago #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 1275:d14800f88434