Simplify tests by using Web*Impl types directly.

Simplify tests by using Web*Impl types directly.

Casting between public and impl types is usually just noise
in unit tests, so avoid it when possible.

- Change WebViewHelper::webView() to just return WebViewImpl*
- Removed WebViewHelper::webViewImpl() since it's redundant
- Change FrameTestHelpers::createLocalChild() to return a
  WebLocalFrameImpl*
- Change FrameTestHelpers::createRemoteChild() to return a
  WebRemoteFrameImpl*
- Use WebViewImpl::mainFrameImpl() where possible to get the main frame.
  This should also provide an easy way to grep for how many things depend
  on a local main frame.

BUG=none

Committed: https://crrev.com/95a9b8c38be3669ad42b19ba9a29d08f4a302b34
Cr-Commit-Position: refs/heads/master@{#406506}

[dcheng, 2016-07-20 04:34:09]

Description was changed from

==========
Simplify tests by using Web*Impl types directly.

- Change WebViewHelper::webView() to just return WebViewImpl*
- Removed WebViewHelper::webViewImpl() since it's redundant
- Change FrameTestHelpers::createLocalChild() to return a
  WebLocalFrameImpl*
- Change FrameTestHelpers::createRemoteChild() to return a
  WebRemoteFrameImpl*
- Use WebViewImpl::mainFrameImpl() where possible to get the main frame.
  This should also provide an easy way to grep for how many things depend
  on a local main frame.

BUG=none
==========

to

==========
Simplify tests by using Web*Impl types directly.

Casting between public and impl types is usually just noise
in unit tests, so avoid it when possible.


- Change WebViewHelper::webView() to just return WebViewImpl*
- Removed WebViewHelper::webViewImpl() since it's redundant
- Change FrameTestHelpers::createLocalChild() to return a
  WebLocalFrameImpl*
- Change FrameTestHelpers::createRemoteChild() to return a
  WebRemoteFrameImpl*
- Use WebViewImpl::mainFrameImpl() where possible to get the main frame.
  This should also provide an easy way to grep for how many things depend
  on a local main frame.

BUG=none
==========

[dcheng, 2016-07-20 04:34:09]

dcheng@chromium.org changed reviewers:
+ tkent@chromium.org

[dcheng, 2016-07-20 04:34:17]

The CQ bit was checked by dcheng@chromium.org to run a CQ dry run

[dcheng, 2016-07-20 04:34:31]

Sorry, it's somewhat big but the changes should be straightforward.

[commit-bot: I haz the power, 2016-07-20 04:34:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tkent, 2016-07-20 04:51:01]

lgtm

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebRemoteFrameImpl.h (right):

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebRemoteFrameImpl.h:27: static
WebRemoteFrameImpl* create(WebTreeScopeType, WebRemoteFrameClient*, WebFrame*
opener = nullptr);
Do you need to add the default argument in this CL?

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:3337: WebFrame* iframe =
webViewHelper.webView()->findFrameByName(WebString::fromUTF8("ifr"), nullptr);
Do you need to add nullptr argument in this CL?

[dcheng, 2016-07-20 06:18:07]

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebRemoteFrameImpl.h (right):

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebRemoteFrameImpl.h:27: static
WebRemoteFrameImpl* create(WebTreeScopeType, WebRemoteFrameClient*, WebFrame*
opener = nullptr);
On 2016/07/20 04:51:01, tkent wrote:
> Do you need to add the default argument in this CL?

WebRemoteFrame::create() has this default arg, so it's just to make things
easier. Since this is not a virtual, I opted to just add the default arg, even
though it'd only be useful in unit tests. WDYT?

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:3337: WebFrame* iframe =
webViewHelper.webView()->findFrameByName(WebString::fromUTF8("ifr"), nullptr);
On 2016/07/20 04:51:01, tkent wrote:
> Do you need to add nullptr argument in this CL?

findFrameByName has a default argument on WebView but not on WebViewImpl. The
default arg doesn't apply to overrides, so it breaks once I change this type to
WebViewImpl.

Since default args are forbidden on virtuals per the style guide, I opted to
just specify nullptr here.

[commit-bot: I haz the power, 2016-07-20 06:29:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-20 06:29:45]

Dry run: This issue passed the CQ dry run.

[tkent, 2016-07-20 06:31:53]

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebRemoteFrameImpl.h (right):

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebRemoteFrameImpl.h:27: static
WebRemoteFrameImpl* create(WebTreeScopeType, WebRemoteFrameClient*, WebFrame*
opener = nullptr);
On 2016/07/20 at 06:18:07, dcheng wrote:
> On 2016/07/20 04:51:01, tkent wrote:
> > Do you need to add the default argument in this CL?
> 
> WebRemoteFrame::create() has this default arg, so it's just to make things
easier. Since this is not a virtual, I opted to just add the default arg, even
though it'd only be useful in unit tests. WDYT?

Thank you for the explanation.  This is ok as is.

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/2165573003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:3337: WebFrame* iframe =
webViewHelper.webView()->findFrameByName(WebString::fromUTF8("ifr"), nullptr);
On 2016/07/20 at 06:18:07, dcheng wrote:
> On 2016/07/20 04:51:01, tkent wrote:
> > Do you need to add nullptr argument in this CL?
> 
> findFrameByName has a default argument on WebView but not on WebViewImpl. The
default arg doesn't apply to overrides, so it breaks once I change this type to
WebViewImpl.
> 
> Since default args are forbidden on virtuals per the style guide, I opted to
just specify nullptr here.

ok, I understand.

[dcheng, 2016-07-20 06:33:22]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2016-07-20 06:33:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 06:38:15]

Description was changed from

==========
Simplify tests by using Web*Impl types directly.

Casting between public and impl types is usually just noise
in unit tests, so avoid it when possible.


- Change WebViewHelper::webView() to just return WebViewImpl*
- Removed WebViewHelper::webViewImpl() since it's redundant
- Change FrameTestHelpers::createLocalChild() to return a
  WebLocalFrameImpl*
- Change FrameTestHelpers::createRemoteChild() to return a
  WebRemoteFrameImpl*
- Use WebViewImpl::mainFrameImpl() where possible to get the main frame.
  This should also provide an easy way to grep for how many things depend
  on a local main frame.

BUG=none
==========

to

==========
Simplify tests by using Web*Impl types directly.

Casting between public and impl types is usually just noise
in unit tests, so avoid it when possible.


- Change WebViewHelper::webView() to just return WebViewImpl*
- Removed WebViewHelper::webViewImpl() since it's redundant
- Change FrameTestHelpers::createLocalChild() to return a
  WebLocalFrameImpl*
- Change FrameTestHelpers::createRemoteChild() to return a
  WebRemoteFrameImpl*
- Use WebViewImpl::mainFrameImpl() where possible to get the main frame.
  This should also provide an easy way to grep for how many things depend
  on a local main frame.

BUG=none
==========

[commit-bot: I haz the power, 2016-07-20 06:38:16]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-20 06:40:27]

Description was changed from

==========
Simplify tests by using Web*Impl types directly.

Casting between public and impl types is usually just noise
in unit tests, so avoid it when possible.


- Change WebViewHelper::webView() to just return WebViewImpl*
- Removed WebViewHelper::webViewImpl() since it's redundant
- Change FrameTestHelpers::createLocalChild() to return a
  WebLocalFrameImpl*
- Change FrameTestHelpers::createRemoteChild() to return a
  WebRemoteFrameImpl*
- Use WebViewImpl::mainFrameImpl() where possible to get the main frame.
  This should also provide an easy way to grep for how many things depend
  on a local main frame.

BUG=none
==========

to

==========
Simplify tests by using Web*Impl types directly.

Casting between public and impl types is usually just noise
in unit tests, so avoid it when possible.

- Change WebViewHelper::webView() to just return WebViewImpl*
- Removed WebViewHelper::webViewImpl() since it's redundant
- Change FrameTestHelpers::createLocalChild() to return a
  WebLocalFrameImpl*
- Change FrameTestHelpers::createRemoteChild() to return a
  WebRemoteFrameImpl*
- Use WebViewImpl::mainFrameImpl() where possible to get the main frame.
  This should also provide an easy way to grep for how many things depend
  on a local main frame.

BUG=none

Committed: https://crrev.com/95a9b8c38be3669ad42b19ba9a29d08f4a302b34
Cr-Commit-Position: refs/heads/master@{#406506}
==========

[commit-bot: I haz the power, 2016-07-20 06:40:29]

Patchset 1 (id:??) landed as
https://crrev.com/95a9b8c38be3669ad42b19ba9a29d08f4a302b34
Cr-Commit-Position: refs/heads/master@{#406506}