Commit 5e03ad76 authored by Daniel Cheng's avatar Daniel Cheng Committed by Commit Bot

Make it harder to construct the wrong kind of WebFrameWidget.

Current, there's a different internal implementation of WebFrameWidget
for main frames, to disguise the fact that WebView happens to be a
WebWidget as well.

Rather than exposing this implementation detail, just make the Create()
function do the right thing.

TBR=thestig@chromium.org

Change-Id: Ic6c5701953b16350ddbc927fb6355956d5724b54
Reviewed-on: https://chromium-review.googlesource.com/538737
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#480299}
parent 57947f0d
...@@ -262,9 +262,7 @@ WebViewPlugin::WebViewHelper::WebViewHelper( ...@@ -262,9 +262,7 @@ WebViewPlugin::WebViewHelper::WebViewHelper(
WebLocalFrame* web_frame = WebLocalFrame::Create( WebLocalFrame* web_frame = WebLocalFrame::Create(
blink::WebTreeScopeType::kDocument, this, nullptr, nullptr); blink::WebTreeScopeType::kDocument, this, nullptr, nullptr);
web_view_->SetMainFrame(web_frame); web_view_->SetMainFrame(web_frame);
// TODO(dcheng): The main frame widget currently has a special case. WebFrameWidget::Create(this, web_frame);
// Eliminate this once WebView is no longer a WebWidget.
WebFrameWidget::Create(this, web_view_, web_frame);
} }
WebViewPlugin::WebViewHelper::~WebViewHelper() { WebViewPlugin::WebViewHelper::~WebViewHelper() {
......
...@@ -586,7 +586,7 @@ void PrintWebViewHelper::PrintHeaderAndFooter( ...@@ -586,7 +586,7 @@ void PrintWebViewHelper::PrintHeaderAndFooter(
blink::WebTreeScopeType::kDocument, &frame_client, nullptr, nullptr); blink::WebTreeScopeType::kDocument, &frame_client, nullptr, nullptr);
web_view->SetMainFrame(frame); web_view->SetMainFrame(frame);
blink::WebWidgetClient web_widget_client; blink::WebWidgetClient web_widget_client;
blink::WebFrameWidget::Create(&web_widget_client, web_view, frame); blink::WebFrameWidget::Create(&web_widget_client, frame);
base::Value html( base::Value html(
base::UTF8ToUTF16(ResourceBundle::GetSharedInstance().GetRawDataResource( base::UTF8ToUTF16(ResourceBundle::GetSharedInstance().GetRawDataResource(
...@@ -808,7 +808,7 @@ void PrepareFrameAndViewForPrint::CopySelection( ...@@ -808,7 +808,7 @@ void PrepareFrameAndViewForPrint::CopySelection(
blink::WebLocalFrame* main_frame = blink::WebLocalFrame::Create( blink::WebLocalFrame* main_frame = blink::WebLocalFrame::Create(
blink::WebTreeScopeType::kDocument, this, nullptr, nullptr); blink::WebTreeScopeType::kDocument, this, nullptr, nullptr);
web_view->SetMainFrame(main_frame); web_view->SetMainFrame(main_frame);
blink::WebFrameWidget::Create(this, web_view, main_frame); blink::WebFrameWidget::Create(this, main_frame);
frame_.Reset(web_view->MainFrame()->ToWebLocalFrame()); frame_.Reset(web_view->MainFrame()->ToWebLocalFrame());
node_to_print_.Reset(); node_to_print_.Reset();
......
...@@ -489,11 +489,6 @@ RenderWidget* RenderWidget::CreateForFrame( ...@@ -489,11 +489,6 @@ RenderWidget* RenderWidget::CreateForFrame(
blink::WebFrameWidget* RenderWidget::CreateWebFrameWidget( blink::WebFrameWidget* RenderWidget::CreateWebFrameWidget(
RenderWidget* render_widget, RenderWidget* render_widget,
blink::WebLocalFrame* frame) { blink::WebLocalFrame* frame) {
if (!frame->Parent()) {
// TODO(dcheng): The main frame widget currently has a special case.
// Eliminate this once WebView is no longer a WebWidget.
return blink::WebFrameWidget::Create(render_widget, frame->View(), frame);
}
return blink::WebFrameWidget::Create(render_widget, frame); return blink::WebFrameWidget::Create(render_widget, frame);
} }
......
...@@ -264,10 +264,7 @@ WebViewBase* WebViewHelper::InitializeWithOpener( ...@@ -264,10 +264,7 @@ WebViewBase* WebViewHelper::InitializeWithOpener(
web_frame_client->GetInterfaceProviderForTesting(), nullptr, opener); web_frame_client->GetInterfaceProviderForTesting(), nullptr, opener);
web_frame_client->Bind(frame, std::move(owned_web_frame_client)); web_frame_client->Bind(frame, std::move(owned_web_frame_client));
web_view_->SetMainFrame(frame); web_view_->SetMainFrame(frame);
blink::WebFrameWidget::Create(web_widget_client, frame);
// TODO(dcheng): The main frame widget currently has a special case.
// Eliminate this once WebView is no longer a WebWidget.
blink::WebFrameWidget::Create(web_widget_client, web_view_, frame);
test_web_view_client_ = web_view_client; test_web_view_client_ = web_view_client;
......
...@@ -83,16 +83,22 @@ namespace blink { ...@@ -83,16 +83,22 @@ namespace blink {
WebFrameWidget* WebFrameWidget::Create(WebWidgetClient* client, WebFrameWidget* WebFrameWidget::Create(WebWidgetClient* client,
WebLocalFrame* local_root) { WebLocalFrame* local_root) {
DCHECK(client) << "A valid WebWidgetClient must be supplied."; DCHECK(client) << "A valid WebWidgetClient must be supplied.";
// Pass the WebFrameWidget's self-reference to the caller. if (!local_root->Parent()) {
return WebFrameWidgetImpl::Create(client, local_root); // Note: this isn't a leak, as the object has a self-reference that the
} // caller needs to release by calling Close().
WebLocalFrameBase& main_frame = ToWebLocalFrameBase(*local_root);
DCHECK(main_frame.ViewImpl());
// Note: this can't DCHECK that the view's main frame points to
// |main_frame|, as provisional frames violate this precondition.
// TODO(dcheng): Remove the special bridge class for main frame widgets.
return new WebViewFrameWidget(*client, *main_frame.ViewImpl(), main_frame);
}
WebFrameWidget* WebFrameWidget::Create(WebWidgetClient* client, DCHECK(local_root->Parent()->IsWebRemoteFrame())
WebView* web_view, << "Only local roots can have web frame widgets.";
WebLocalFrame* main_frame) { // Note: this isn't a leak, as the object has a self-reference that the
DCHECK(client) << "A valid WebWidgetClient must be supplied."; // caller needs to release by calling Close().
return new WebViewFrameWidget(*client, static_cast<WebViewBase&>(*web_view), return WebFrameWidgetImpl::Create(client, local_root);
ToWebLocalFrameBase(*main_frame));
} }
WebFrameWidgetImpl* WebFrameWidgetImpl::Create(WebWidgetClient* client, WebFrameWidgetImpl* WebFrameWidgetImpl::Create(WebWidgetClient* client,
......
...@@ -2064,10 +2064,7 @@ TEST_P(WebViewTest, ClientTapHandlingNullWebViewClient) { ...@@ -2064,10 +2064,7 @@ TEST_P(WebViewTest, ClientTapHandlingNullWebViewClient) {
WebTreeScopeType::kDocument, &web_frame_client, nullptr, nullptr); WebTreeScopeType::kDocument, &web_frame_client, nullptr, nullptr);
web_frame_client.Bind(local_frame); web_frame_client.Bind(local_frame);
web_view->SetMainFrame(local_frame); web_view->SetMainFrame(local_frame);
blink::WebFrameWidget::Create(&web_widget_client, local_frame);
// TODO(dcheng): The main frame widget currently has a special case.
// Eliminate this once WebView is no longer a WebWidget.
blink::WebFrameWidget::Create(&web_widget_client, web_view, local_frame);
WebGestureEvent event(WebInputEvent::kGestureTap, WebInputEvent::kNoModifiers, WebGestureEvent event(WebInputEvent::kGestureTap, WebInputEvent::kNoModifiers,
WebInputEvent::kTimeStampForTesting); WebInputEvent::kTimeStampForTesting);
......
...@@ -41,18 +41,11 @@ namespace blink { ...@@ -41,18 +41,11 @@ namespace blink {
class WebDragData; class WebDragData;
class WebLocalFrame; class WebLocalFrame;
class WebInputMethodController; class WebInputMethodController;
class WebView;
class WebWidgetClient; class WebWidgetClient;
class WebFrameWidget : public WebWidget { class WebFrameWidget : public WebWidget {
public: public:
BLINK_EXPORT static WebFrameWidget* Create(WebWidgetClient*, WebLocalFrame*); BLINK_EXPORT static WebFrameWidget* Create(WebWidgetClient*, WebLocalFrame*);
// Creates a frame widget for a WebView. Temporary helper to help transition
// away from WebView inheriting WebWidget.
// TODO(dcheng): Remove once transition is complete.
BLINK_EXPORT static WebFrameWidget* Create(WebWidgetClient*,
WebView*,
WebLocalFrame* main_frame);
// Sets the visibility of the WebFrameWidget. // Sets the visibility of the WebFrameWidget.
// We still track page-level visibility, but additionally we need to notify a // We still track page-level visibility, but additionally we need to notify a
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment