Introduce the class webLocalFrameBase anf make WebLocalFrameImpl derived it.

Introduce the class webLocalFrameBase anf make WebLocalFrameImpl derived it.

There are many cyclic dependencies in web/ between WebLocalFrameImpl and other
classes. This makes it hard to move WebLocalFrameImpl from web/ into core/ in
small, logical steps.

To remove these cycles we introduce WebLocalFrameBase. Classes that depend on
methods defined in WebLocalFrameImpl will be changed to depend on
WebLocalFrameBase instead, by declaring these dependent methods pure virtual in
WebLocalFrameBase. We can then sever the links to WebLocalFrameImpl and move
classes out of Web.

This CL establises the inheritence of WebLocalFrameImpl on WebLocalFrameBase,
subsequent CL's will start to delcare methods in WebLocalFrameBase.

BUG=708879
Review-Url: https://codereview.chromium.org/2863853002
Cr-Commit-Position: refs/heads/master@{#469586}
Committed: https://chromium.googlesource.com/chromium/src/+/7ba3433c3a317a45c461fb499d65c7f67eecf3e7

[slangley, 2017-05-05 01:08:45]

slangley@chromium.org changed reviewers:
+ sashab@chromium.org

[sashab, 2017-05-05 01:10:14]

lgtm

https://codereview.chromium.org/2863853002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/WebLocalFrameBase.h (right):

https://codereview.chromium.org/2863853002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/WebLocalFrameBase.h:19: // TODO(slangley):
Remove this class once WebLocalFrameImpl is in core/
Full stop at end of comment ;)

[slangley, 2017-05-05 01:18:48]

slangley@chromium.org changed reviewers:
+ haraken@chromium.org

[slangley, 2017-05-05 01:18:49]

haraken@ - ptal

https://codereview.chromium.org/2863853002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/WebLocalFrameBase.h (right):

https://codereview.chromium.org/2863853002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/WebLocalFrameBase.h:19: // TODO(slangley):
Remove this class once WebLocalFrameImpl is in core/
On 2017/05/05 at 01:10:13, sashab wrote:
> Full stop at end of comment ;)

Done

[haraken, 2017-05-05 01:20:14]

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

[haraken, 2017-05-05 01:20:16]

LGTM

dcheng@: Are we planning to keep WebLocalFrameImpl even after we move it into
core/, right? If we plan to merge WebLocalFrameImpl into LocalFrame, it would
make more sense to add the virtual methods on LocalFrame instead of introducing
WebLocalFrameBase.

[slangley, 2017-05-05 01:40:08]

The CQ bit was checked by slangley@chromium.org

[slangley, 2017-05-05 01:40:09]

The patchset sent to the CQ was uploaded after l-g-t-m from sashab@chromium.org,
haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2863853002/#ps40001
(title: "Add WebLocalViewBase.h to the BUILD file.")

[commit-bot: I haz the power, 2017-05-05 01:40:48]

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, 2017-05-05 03:36:32]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493948408468540,
"parent_rev": "16d9d7d9ca1db157277bb1170eda38fac52eed7c", "commit_rev":
"7ba3433c3a317a45c461fb499d65c7f67eecf3e7"}

[commit-bot: I haz the power, 2017-05-05 03:37:00]

Description was changed from

==========
Introduce the class webLocalFrameBase anf make WebLocalFrameImpl derived it.

There are many cyclic dependencies in web/ between WebLocalFrameImpl and other
classes. This makes it hard to move WebLocalFrameImpl from web/ into core/ in
small, logical steps.

To remove these cycles we introduce WebLocalFrameBase. Classes that depend on
methods defined in WebLocalFrameImpl will be changed to depend on
WebLocalFrameBase instead, by declaring these dependent methods pure virtual in
WebLocalFrameBase. We can then sever the links to WebLocalFrameImpl and move
classes out of Web.

This CL establises the inheritence of WebLocalFrameImpl on WebLocalFrameBase,
subsequent CL's will start to delcare methods in WebLocalFrameBase.

BUG=708879
==========

to

==========
Introduce the class webLocalFrameBase anf make WebLocalFrameImpl derived it.

There are many cyclic dependencies in web/ between WebLocalFrameImpl and other
classes. This makes it hard to move WebLocalFrameImpl from web/ into core/ in
small, logical steps.

To remove these cycles we introduce WebLocalFrameBase. Classes that depend on
methods defined in WebLocalFrameImpl will be changed to depend on
WebLocalFrameBase instead, by declaring these dependent methods pure virtual in
WebLocalFrameBase. We can then sever the links to WebLocalFrameImpl and move
classes out of Web.

This CL establises the inheritence of WebLocalFrameImpl on WebLocalFrameBase,
subsequent CL's will start to delcare methods in WebLocalFrameBase.

BUG=708879

Review-Url: https://codereview.chromium.org/2863853002
Cr-Commit-Position: refs/heads/master@{#469586}
Committed:
https://chromium.googlesource.com/chromium/src/+/7ba3433c3a317a45c461fb499d65...
==========

[commit-bot: I haz the power, 2017-05-05 03:37:01]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/7ba3433c3a317a45c461fb499d65...

[dcheng, 2017-05-05 04:47:13]

On 2017/05/05 01:20:16, haraken wrote:
> LGTM
> 
> dcheng@: Are we planning to keep WebLocalFrameImpl even after we move it into
> core/, right? If we plan to merge WebLocalFrameImpl into LocalFrame, it would
> make more sense to add the virtual methods on LocalFrame instead of
introducing
> WebLocalFrameBase.

For now, I think we want to keep two separate classes. I see an argument for
merging them in the future, but if we did so, we want to merge many other
classes as well, to avoid exposing a Web* version and a non-Web* version. I
think this would be a pretty big (separate) project.

https://codereview.chromium.org/2863853002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/WebLocalFrameBase.h (right):

https://codereview.chromium.org/2863853002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/WebLocalFrameBase.h:3: // found in the
LICENSE file.#ifndef WebViewBase_h
Nit: Delete #ifndef...

https://codereview.chromium.org/2863853002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/WebLocalFrameBase.h:23: };
Nit: newline after for consistency

https://codereview.chromium.org/2863853002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/WebLocalFrameBase.h:24: }
Nit:  // namespace blink

[slangley, 2017-05-05 04:50:53]

On 2017/05/05 at 04:47:13, dcheng wrote:
> On 2017/05/05 01:20:16, haraken wrote:
> > LGTM
> > 
> > dcheng@: Are we planning to keep WebLocalFrameImpl even after we move it
into
> > core/, right? If we plan to merge WebLocalFrameImpl into LocalFrame, it
would
> > make more sense to add the virtual methods on LocalFrame instead of
introducing
> > WebLocalFrameBase.
> 
> For now, I think we want to keep two separate classes. I see an argument for
merging them in the future, but if we did so, we want to merge many other
classes as well, to avoid exposing a Web* version and a non-Web* version. I
think this would be a pretty big (separate) project.
> 
>
https://codereview.chromium.org/2863853002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/frame/WebLocalFrameBase.h (right):
> 
>
https://codereview.chromium.org/2863853002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/frame/WebLocalFrameBase.h:3: // found in the
LICENSE file.#ifndef WebViewBase_h
> Nit: Delete #ifndef...
> 
>
https://codereview.chromium.org/2863853002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/frame/WebLocalFrameBase.h:23: };
> Nit: newline after for consistency
> 
>
https://codereview.chromium.org/2863853002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/frame/WebLocalFrameBase.h:24: }
> Nit:  // namespace blink

Thanks Daniel,

Fixes for nits are in cr/2863913002