Common up the determination of navigation policy.

Common up the determination of navigation policy.

Share the (little bit of) logic that determines the WebNavigationPolicy
to use when navigating to new content, including when creating a new
window to hold it.

R=mkwst,jochen
BUG=none

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=195521

[sof, 2015-05-18 12:31:57]

sigbjornf@opera.com changed reviewers:
+ jochen@chromium.org, mkwst@chromium.org

[sof, 2015-05-18 12:31:58]

please take a look.

[Mike West, 2015-05-18 15:07:56]

LGTM % nit.

https://codereview.chromium.org/1140463003/diff/1/Source/web/ChromeClientImpl...
File Source/web/ChromeClientImpl.cpp (right):

https://codereview.chromium.org/1140463003/diff/1/Source/web/ChromeClientImpl...
Source/web/ChromeClientImpl.cpp:209: inline void updatePolicyForEvent(const
WebInputEvent* inputEvent, NavigationPolicy* policy)
Why drop 'static'?

[Mike West, 2015-05-18 15:09:44]

Moar nits. All the same nit, really. Why drop `static`? I guess we should either
move everything to an anonymous namespace a la Chromium, or mark the methods
static when defining them.

https://codereview.chromium.org/1140463003/diff/1/Source/web/ChromeClientImpl...
File Source/web/ChromeClientImpl.cpp (right):

https://codereview.chromium.org/1140463003/diff/1/Source/web/ChromeClientImpl...
Source/web/ChromeClientImpl.cpp:243: WebNavigationPolicy
getNavigationPolicy(const WindowFeatures& features)
Nit: static?

https://codereview.chromium.org/1140463003/diff/1/Source/web/ChromeClientImpl...
Source/web/ChromeClientImpl.cpp:262: WebNavigationPolicy
effectiveNavigationPolicy(NavigationPolicy navigationPolicy, const
WindowFeatures& features)
Nit: static?

[sof, 2015-05-18 15:15:43]

why the preference for static over an anon namespace here, does it matter?

[Mike West, 2015-05-18 20:43:40]

On 2015/05/18 at 15:15:43, sigbjornf wrote:
> why the preference for static over an anon namespace here, does it matter?

It doesn't. Which is why it's a nit. :)

[jochen (gone - plz use gerrit), 2015-05-18 22:45:15]

lgtm

https://codereview.chromium.org/1140463003/diff/1/Source/web/ChromeClientImpl...
File Source/web/ChromeClientImpl.cpp (right):

https://codereview.chromium.org/1140463003/diff/1/Source/web/ChromeClientImpl...
Source/web/ChromeClientImpl.cpp:209: inline void updatePolicyForEvent(const
WebInputEvent* inputEvent, NavigationPolicy* policy)
On 2015/05/18 at 15:07:56, Mike West (holiday in DE) wrote:
> Why drop 'static'?

it's in an anon namespace now

[sof, 2015-05-19 06:46:49]

On 2015/05/18 20:43:40, Mike West (holiday in DE) wrote:
> On 2015/05/18 at 15:15:43, sigbjornf wrote:
> > why the preference for static over an anon namespace here, does it matter?
> 
> It doesn't. Which is why it's a nit. :)

ok, i opted to use an anon namespace here to group these local helpers together
within one block. As you say, that aligns with the upstream coding style.

[sof, 2015-05-19 06:47:04]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-05-19 06:47:05]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1140463003/#ps20001
(title: "rebased")

[commit-bot: I haz the power, 2015-05-19 06:47:13]

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

[commit-bot: I haz the power, 2015-05-19 08:04:23]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195521