I think what you perhaps is missing is that WebFrameImplBase is GC'd object and GC objects aren't exposed in the public interface (aka WebFrame/WebLocalFrame/WebRemoteFrame)

The other motivation of this base class is to avoid polluting the public headers with internal class names like blink::Frame.

Re #1: Both WebFrame and Web{Local,Remote}Frame are pure virtual classes, so Web{Local,Remote}FrameImpl objects could be defined as GC'ed I believe.

Re #2: WebFrameImplBase is internal; and WebFrame is already exposing it to public headers along with other internal classes. So it is not a big concern I guess. But even if it is so, I have a patch that seems to keep the number of exposed classes the same. I will send it out soon.

#1. No GC'd objects are not exposed outside of blink. See the definition of a GC'd object https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/heap/GarbageCollected.h?type=cs&l=206

To clarify, I meant making the impl classes GC'ed which is fine I believe. Source/web/WebFrameImplBase is currently GC'ed so perhaps it means its subclass Source/web/WebLocalFrameImpl is currently GC'ed as well. I am just making the latter GC'ed through class decl.

Here is my patch: https://codereview.chromium.org/2837593002/

You are exposing a forward defined class publicly as a virtual that nobody could actually call or implement.

ie; Frame & Page is in core/*  These classes are used inside a #ifdef INSIDE_BLINK_IMPL ...

And defining a virtual inside is a #ifdef isn't going to work.

Another possibility is to just change these back to static helper functions. That's what the code used to do, but it read a bit awkwardly.

I did some poking around, and while the secret base class can be convenient, it's ultimately still less efficient (two virtual calls) than the original implementation (which did one virtual call and then a downcast to a final class).

(We would make these static helpers on WebFrame, hidden by BLINK_IMPLEMENTATION macros. This would let us abbreviate the method names a bit inside Web*Frame.*, and still provide sufficient context outside of WebFrame implementations)

Hidden static helpers to avoid a redundant virtual call seems like the best solution to me, thanks. I will update my CL accordingly.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a56addf7e3a7f7a70558980a216b2d7081251ba

commit 6a56addf7e3a7f7a70558980a216b2d7081251ba
Author: mustaq <mustaq@chromium.org>
Date: Thu Apr 27 16:34:05 2017

Nuked WebFrameImplBase to make Frame access more efficient.

This "hidden" base class was forcing two virtual calls to reach a Frame from a WebFrame. It is now one virtual call away.

BUG= 713798 

Review-Url: https://codereview.chromium.org/2837593002
Cr-Commit-Position: refs/heads/master@{#467706}

[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/BUILD.gn
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/WebDOMMessageEvent.cpp
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/WebFrame.cpp
[delete] https://crrev.com/b71e98cdf14f18cb967a73857826f6e8c568cea0/third_party/WebKit/Source/web/WebFrameImplBase.cpp
[delete] https://crrev.com/b71e98cdf14f18cb967a73857826f6e8c568cea0/third_party/WebKit/Source/web/WebFrameImplBase.h
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/WebRemoteFrameImpl.cpp
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/WebRemoteFrameImpl.h
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/tests/TouchActionTest.cpp
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/public/web/WebFrame.h
[modify] https://crrev.com/6a56addf7e3a7f7a70558980a216b2d7081251ba/third_party/WebKit/public/web/WebView.h