Skip to content

Commit

Permalink
Revert of Introduce WebPluginContainerBase to abstract WebPluginConta…
Browse files Browse the repository at this point in the history
…inerImpl. (patchset #2 id:20001 of https://codereview.chromium.org/2886113002/ )

Reason for revert:
Causes leak failure in editing/selection/selection-plugin-clear-crash.html.

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak/builds/4790

(and locally, I get crashes in debug)

Original issue's description:
> Introduce WebPluginContainerBase to abstract WebPluginContainerImpl.
>
> Many classes in web/ take a dependancy on WebPluginContainerImpl, in some cases
> this dependency is cyclic. In order to break this dependency we introduce a new
> temporary abstraction WebPluginContainerBase, that derives from PlugingView,
> WebPluginContainer and ContextClient.
>
> Classes that were taking a dependency on WebPluginContainerImpl now take a
> dependency on WebPluginContainerBase. In cases where there we methods that
> were only defined in WebPluginContainerImpl we make them pure virtual in
> WebPluginContainerBase and override them in WebPluginContainerImpl.
>
> As WebPluginContainerImpl is garbage collected, we move this to
> WebPluginContainerBase and define a virtual trace method.
>
> The intention is for this abstraction to be temporary, once we move all of the
> dependencies into core/ we can remove it.
>
> I put the new class in core/exported as it derives from WebPluginContainer.
>
>
> BUG=712963
>
> Review-Url: https://codereview.chromium.org/2886113002
> Cr-Commit-Position: refs/heads/master@{#472810}
> Committed: https://chromium.googlesource.com/chromium/src/+/303acce5cc18d95d46342bd089e06957841bc21b

TBR=tkent@chromium.org,dcheng@chromium.org,sashab@chromium.org,slangley@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=712963

Review-Url: https://codereview.chromium.org/2888283004
Cr-Commit-Position: refs/heads/master@{#472915}
  • Loading branch information
jeremyroman authored and Commit bot committed May 18, 2017
1 parent 6d7a0de commit ba5159d
Show file tree
Hide file tree
Showing 17 changed files with 99 additions and 187 deletions.
2 changes: 0 additions & 2 deletions third_party/WebKit/Source/core/exported/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ blink_core_sources("exported") {
"WebImageDecoder.cpp",
"WebMemoryStatistics.cpp",
"WebPerformance.cpp",
"WebPluginContainerBase.cpp",
"WebPluginContainerBase.h",
"WebRange.cpp",
"WebRenderTheme.cpp",
"WebScriptController.cpp",
Expand Down
17 changes: 0 additions & 17 deletions third_party/WebKit/Source/core/exported/WebPluginContainerBase.cpp

This file was deleted.

76 changes: 0 additions & 76 deletions third_party/WebKit/Source/core/exported/WebPluginContainerBase.h

This file was deleted.

6 changes: 3 additions & 3 deletions third_party/WebKit/Source/web/ChromeClientImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
#include "core/events/UIEventWithKeyState.h"
#include "core/events/WebInputEventConversion.h"
#include "core/exported/WebFileChooserCompletionImpl.h"
#include "core/exported/WebPluginContainerBase.h"
#include "core/exported/WebViewBase.h"
#include "core/frame/FrameView.h"
#include "core/frame/Settings.h"
Expand Down Expand Up @@ -123,6 +122,7 @@
#include "web/PopupMenuImpl.h"
#include "web/WebFrameWidgetImpl.h"
#include "web/WebLocalFrameImpl.h"
#include "web/WebPluginContainerImpl.h"
#include "web/WebRemoteFrameImpl.h"
#include "web/WebSettingsImpl.h"

Expand Down Expand Up @@ -658,8 +658,8 @@ void ChromeClientImpl::ShowMouseOverURL(const HitTestResult& result) {
if (object && object->IsLayoutPart()) {
PluginView* plugin_view = ToLayoutPart(object)->Plugin();
if (plugin_view && plugin_view->IsPluginContainer()) {
WebPluginContainerBase* plugin =
ToWebPluginContainerBase(plugin_view);
WebPluginContainerImpl* plugin =
ToWebPluginContainerImpl(plugin_view);
url = plugin->Plugin()->LinkAtPosition(
result.RoundedPointInInnerNodeFrame());
}
Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#include "core/editing/markers/DocumentMarkerController.h"
#include "core/editing/spellcheck/SpellChecker.h"
#include "core/exported/WebDataSourceImpl.h"
#include "core/exported/WebPluginContainerBase.h"
#include "core/exported/WebViewBase.h"
#include "core/frame/FrameView.h"
#include "core/frame/Settings.h"
Expand Down Expand Up @@ -79,6 +78,7 @@
#include "public/web/WebTextCheckClient.h"
#include "public/web/WebViewClient.h"
#include "web/ContextMenuAllowedScope.h"
#include "web/WebPluginContainerImpl.h"

namespace blink {

Expand Down Expand Up @@ -296,7 +296,7 @@ bool ContextMenuClientImpl::ShowContextMenu(const ContextMenu* default_menu,
PluginView* plugin_view = ToLayoutPart(object)->Plugin();
if (plugin_view && plugin_view->IsPluginContainer()) {
data.media_type = WebContextMenuData::kMediaTypePlugin;
WebPluginContainerBase* plugin = ToWebPluginContainerBase(plugin_view);
WebPluginContainerImpl* plugin = ToWebPluginContainerImpl(plugin_view);
WebString text = plugin->Plugin()->SelectionAsText();
if (!text.IsEmpty()) {
data.selected_text = text;
Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#include "core/editing/InputMethodController.h"
#include "core/editing/PlainTextRange.h"
#include "core/events/WebInputEventConversion.h"
#include "core/exported/WebPluginContainerBase.h"
#include "core/exported/WebViewBase.h"
#include "core/frame/FrameView.h"
#include "core/frame/RemoteFrame.h"
Expand Down Expand Up @@ -74,6 +73,7 @@
#include "web/WebInputMethodControllerImpl.h"
#include "web/WebLocalFrameImpl.h"
#include "web/WebPagePopupImpl.h"
#include "web/WebPluginContainerImpl.h"
#include "web/WebRemoteFrameImpl.h"
#include "web/WebViewFrameWidget.h"

Expand Down Expand Up @@ -1199,7 +1199,7 @@ LocalFrame* WebFrameWidgetImpl::FocusedLocalFrameInWidget() const {

WebPlugin* WebFrameWidgetImpl::FocusedPluginIfInputMethodSupported(
LocalFrame* frame) const {
WebPluginContainerBase* container =
WebPluginContainerImpl* container =
WebLocalFrameImpl::CurrentPluginContainer(frame);
if (container && container->SupportsInputMethod())
return container->Plugin();
Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/Source/web/WebHelperPluginImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@

#include "web/WebHelperPluginImpl.h"

#include "core/exported/WebPluginContainerBase.h"
#include "core/frame/LocalFrameClient.h"
#include "core/frame/WebLocalFrameBase.h"
#include "core/html/HTMLObjectElement.h"
#include "core/loader/FrameLoader.h"
#include "public/web/WebPlugin.h"
#include "web/WebPluginContainerImpl.h"

namespace blink {

Expand Down Expand Up @@ -64,7 +64,7 @@ bool WebHelperPluginImpl::Initialize(const String& plugin_type,
Vector<String> attribute_names;
Vector<String> attribute_values;
DCHECK(frame->GetFrame()->GetDocument()->Url().IsValid());
plugin_container_ = ToWebPluginContainerBase(
plugin_container_ = ToWebPluginContainerImpl(
frame->GetFrame()->Loader().Client()->CreatePlugin(
object_element_.Get(), frame->GetFrame()->GetDocument()->Url(),
attribute_names, attribute_values, plugin_type, false,
Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/Source/web/WebHelperPluginImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace blink {

class HTMLObjectElement;
class WebLocalFrameBase;
class WebPluginContainerBase;
class WebPluginContainerImpl;

// Utility class to host helper plugins for media. Internally, it creates a
// detached HTMLPluginElement to host the plugin and uses
Expand All @@ -66,7 +66,7 @@ class WebHelperPluginImpl final : public WebHelperPlugin {

Timer<WebHelperPluginImpl> destruction_timer_;
Persistent<HTMLObjectElement> object_element_;
Persistent<WebPluginContainerBase> plugin_container_;
Persistent<WebPluginContainerImpl> plugin_container_;
};

} // namespace blink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "core/editing/FrameSelection.h"
#include "core/editing/InputMethodController.h"
#include "core/editing/PlainTextRange.h"
#include "core/exported/WebPluginContainerBase.h"
#include "core/frame/LocalFrame.h"
#include "core/page/FocusController.h"
#include "core/page/Page.h"
Expand All @@ -22,6 +21,7 @@
#include "public/web/WebPlugin.h"
#include "public/web/WebRange.h"
#include "web/WebLocalFrameImpl.h"
#include "web/WebPluginContainerImpl.h"

namespace blink {

Expand Down Expand Up @@ -158,7 +158,7 @@ InputMethodController& WebInputMethodControllerImpl::GetInputMethodController()

WebPlugin* WebInputMethodControllerImpl::FocusedPluginIfInputMethodSupported()
const {
WebPluginContainerBase* container =
WebPluginContainerImpl* container =
WebLocalFrameImpl::CurrentPluginContainer(GetFrame());
if (container && container->SupportsInputMethod())
return container->Plugin();
Expand Down
Loading

0 comments on commit ba5159d

Please sign in to comment.