diff options
8 files changed, 60 insertions, 142 deletions
diff --git a/chrome/browser/extensions/web_view_browsertest.cc b/chrome/browser/extensions/web_view_browsertest.cc index da1faa2..38a70a5 100644 --- a/chrome/browser/extensions/web_view_browsertest.cc +++ b/chrome/browser/extensions/web_view_browsertest.cc @@ -967,6 +967,19 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, ConsoleMessage) { << message_; } +// Disabled on win debug bots due to flaky timeouts. +// See http://crbug.com/222618 . +#if defined(OS_WIN) && !defined(NDEBUG) +#define MAYBE_NewWindow DISABLED_NewWindow +#else +#define MAYBE_NewWindow NewWindow +#endif +IN_PROC_BROWSER_TEST_F(WebViewTest, MAYBE_NewWindow) { + ASSERT_TRUE(StartTestServer()); // For serving guest pages. + ASSERT_TRUE(RunPlatformAppTest("platform_apps/web_view/newwindow")) + << message_; +} + IN_PROC_BROWSER_TEST_F(WebViewTest, DownloadPermission) { ASSERT_TRUE(StartTestServer()); // For serving guest pages. content::WebContents* guest_web_contents = diff --git a/chrome/browser/extensions/web_view_interactive_browsertest.cc b/chrome/browser/extensions/web_view_interactive_browsertest.cc index faaac20..6f22906 100644 --- a/chrome/browser/extensions/web_view_interactive_browsertest.cc +++ b/chrome/browser/extensions/web_view_interactive_browsertest.cc @@ -208,9 +208,3 @@ IN_PROC_BROWSER_TEST_F(WebViewInteractiveTest, DISABLED_Focus) { ASSERT_TRUE(RunPlatformAppTest("platform_apps/web_view/focus")) << message_; } - -IN_PROC_BROWSER_TEST_F(WebViewInteractiveTest, NewWindow) { - ASSERT_TRUE(StartTestServer()); // For serving guest pages. - ASSERT_TRUE(RunPlatformAppTest("platform_apps/web_view/newwindow")) - << message_; -} diff --git a/chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js b/chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js index 6466913..f91bf34 100644 --- a/chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js +++ b/chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js @@ -18,7 +18,9 @@ embedder.setUp = function(config) { /** @private */ embedder.setUpGuest_ = function(partitionName) { document.querySelector('#webview-tag-container').innerHTML = - '<webview style="width: 100px; height: 100px;"></webview>'; + '<webview style="width: 100px; height: 100px;"' + + ' src="' + embedder.guestURL + '"' + + '></webview>'; var webview = document.querySelector('webview'); if (partitionName) { webview.partition = partitionName; @@ -30,27 +32,27 @@ embedder.setUpGuest_ = function(partitionName) { }; /** @private */ -embedder.setUpNewWindowRequest_ = function(webview, url, frameName, testName) { +embedder.setUpNewWindowRequest_ = function(webview, url, frameName) { var onWebViewLoadStop = function(e) { // Send post message to <webview> when it's ready to receive them. - var redirect = testName.indexOf("_blank") != -1; webview.contentWindow.postMessage( - JSON.stringify( - ['open-window', '' + url, '' + frameName, redirect]), '*'); + JSON.stringify(['open-window', '' + url, '' + frameName]), '*'); }; webview.addEventListener('loadstop', onWebViewLoadStop); - webview.setAttribute('src', embedder.guestURL); }; -/** @private */ -embedder.requestFrameName_ = - function(webview, openerWebview, testName, expectedFrameName) { +embedder.setUpFrameNameRequest_ = function(webview, testName) { var onWebViewLoadStop = function(e) { // Send post message to <webview> when it's ready to receive them. webview.contentWindow.postMessage( JSON.stringify(['get-frame-name', testName]), '*'); }; webview.addEventListener('loadstop', onWebViewLoadStop); +}; + +/** @private */ +embedder.requestFrameName_ = + function(webview, openerwebview, testName, expectedFrameName) { var onPostMessageReceived = function(e) { var data = JSON.parse(e.data); if (data[0] == 'get-frame-name') { @@ -60,7 +62,7 @@ embedder.requestFrameName_ = var frameName = data[2]; chrome.test.assertEq(expectedFrameName, frameName); chrome.test.assertEq(expectedFrameName, webview.name); - chrome.test.assertEq(openerWebview.partition, webview.partition); + chrome.test.assertEq(openerwebview.partition, webview.partition); chrome.test.succeed(); } }; @@ -71,6 +73,7 @@ embedder.requestFrameName_ = embedder.assertCorrectEvent_ = function(e) { chrome.test.assertEq('newwindow', e.type); chrome.test.assertTrue(!!e.targetUrl); + chrome.test.assertTrue(e.targetUrl.indexOf(embedder.baseGuestURL) == 0); }; // Tests begin. @@ -101,6 +104,7 @@ var testNewWindow = function(testName, var w = newwindow.contentWindow; var newwebview = w.document.querySelector('webview'); newwebview.name = webViewName; + embedder.setUpFrameNameRequest_(newwebview, testName); embedder.requestFrameName_( newwebview, webview, testName, expectedFrameName); try { @@ -114,7 +118,7 @@ var testNewWindow = function(testName, webview.addEventListener('newwindow', onNewWindow); // Load a new window with the given name. - embedder.setUpNewWindowRequest_(webview, 'guest.html', guestName, testName); + embedder.setUpNewWindowRequest_(webview, 'guest.html', guestName); }; // Loads a guest which requests a new window. @@ -147,23 +151,13 @@ embedder.tests.testNoName = function testNoName() { webViewName, guestName, partitionName, expectedName); }; -embedder.tests.testNewWindowRedirect = function testNewWindowRedirect() { - var webViewName = 'foo'; - var guestName = ''; - var partitionName = 'persist:foobar'; - var expectedName = webViewName; - testNewWindow('testNewWindowRedirect_blank', - webViewName, guestName, partitionName, expectedName); -}; - onload = function() { chrome.test.getConfig(function(config) { embedder.setUp(config); chrome.test.runTests([ embedder.tests.testNewWindowNameTakesPrecedence, embedder.tests.testWebViewNameTakesPrecedence, - embedder.tests.testNoName, - embedder.tests.testNewWindowRedirect + embedder.tests.testNoName ]); }); }; diff --git a/chrome/test/data/extensions/platform_apps/web_view/newwindow/guest.html b/chrome/test/data/extensions/platform_apps/web_view/newwindow/guest.html index a84b816..e7d8345 100644 --- a/chrome/test/data/extensions/platform_apps/web_view/newwindow/guest.html +++ b/chrome/test/data/extensions/platform_apps/web_view/newwindow/guest.html @@ -18,30 +18,18 @@ embedderWindowChannel.postMessage(JSON.stringify(msg_array), '*'); }; - var openWindow = function(url, frameName) { + var startTest = function(url, frameName) { window.open(url, frameName); }; - var openWindowBlankRedirect = function(url, frameName) { - var w = window.open('', frameName); - w.opener = null; - w.document.write( - '<META HTTP-EQUIV="refresh" content="0; url=' + url + '">'); - w.document.close(); - } - var onPostMessageReceived = function(e) { embedderWindowChannel = e.source; var data = JSON.parse(e.data); if (data[0] == 'open-window') { var url = data[1]; var frameName = data[2]; - var redirect = data[3]; - if (redirect) { - openWindowBlankRedirect(url, frameName); - } else { - openWindow(url, frameName); - } + // Start the test once we have |embedderWindowChannel|. + startTest(url, frameName); } else if (data[0] == 'get-frame-name') { var testName = data[1]; notifyEmbedder(['get-frame-name', testName, window.name]); diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc index fc04b7d..d45a7f0 100644 --- a/content/browser/browser_plugin/browser_plugin_guest.cc +++ b/content/browser/browser_plugin/browser_plugin_guest.cc @@ -124,9 +124,7 @@ class BrowserPluginGuest::EmbedderRenderViewHostObserver BrowserPluginGuest::BrowserPluginGuest( int instance_id, - WebContentsImpl* web_contents, - BrowserPluginGuest* opener, - bool has_render_view) + WebContentsImpl* web_contents) : WebContentsObserver(web_contents), weak_ptr_factory_(this), embedder_web_contents_(NULL), @@ -140,9 +138,8 @@ BrowserPluginGuest::BrowserPluginGuest( mouse_locked_(false), pending_lock_request_(false), embedder_visible_(true), - opener_(opener), - next_permission_request_id_(0), - has_render_view_(has_render_view) { + opener_(NULL), + next_permission_request_id_(0) { DCHECK(web_contents); web_contents->SetDelegate(this); GetWebContents()->GetBrowserPluginGuestManager()->AddGuest(instance_id_, @@ -288,8 +285,6 @@ void BrowserPluginGuest::Initialize( if (!params.src.empty()) OnNavigateGuest(instance_id_, params.src); - has_render_view_ = true; - GetContentClient()->browser()->GuestWebContentsCreated( GetWebContents(), embedder_web_contents_); } @@ -304,19 +299,7 @@ BrowserPluginGuest* BrowserPluginGuest::Create( RecordAction(UserMetricsAction("BrowserPlugin.Guest.Create")); if (factory_) return factory_->CreateBrowserPluginGuest(instance_id, web_contents); - return new BrowserPluginGuest(instance_id, web_contents, NULL, false); -} - -// static -BrowserPluginGuest* BrowserPluginGuest::CreateWithOpener( - int instance_id, - WebContentsImpl* web_contents, - BrowserPluginGuest* opener, - bool has_render_view) { - return new BrowserPluginGuest(instance_id, - web_contents, - opener, - has_render_view); + return new BrowserPluginGuest(instance_id, web_contents); } RenderWidgetHostView* BrowserPluginGuest::GetEmbedderRenderWidgetHostView() { @@ -402,29 +385,6 @@ bool BrowserPluginGuest::HandleContextMenu( return true; } -WebContents* BrowserPluginGuest::OpenURLFromTab(WebContents* source, - const OpenURLParams& params) { - // If the guest wishes to navigate away prior to attachment then we save the - // navigation to perform upon attachment. Navigation initializes a lot of - // state that assumes an embedder exists, such as RenderWidgetHostViewGuest. - // Navigation also resumes resource loading which we don't want to allow - // until attachment. - if (!attached()) { - PendingWindowMap::iterator it = opener()->pending_new_windows_.find(this); - if (it == opener()->pending_new_windows_.end()) - return NULL; - const TargetURL& old_target_url = it->second; - TargetURL new_target_url(params.url); - new_target_url.changed = new_target_url.url != old_target_url.url; - it->second = new_target_url; - return NULL; - } - // This can happen for cross-site redirects. - source->GetController().LoadURL( - params.url, params.referrer, params.transition, std::string()); - return source; -} - void BrowserPluginGuest::WebContentsCreated(WebContents* source_contents, int64 source_frame_id, const string16& frame_name, @@ -433,11 +393,12 @@ void BrowserPluginGuest::WebContentsCreated(WebContents* source_contents, WebContentsImpl* new_contents_impl = static_cast<WebContentsImpl*>(new_contents); BrowserPluginGuest* guest = new_contents_impl->GetBrowserPluginGuest(); + guest->opener_ = this; guest->name_ = UTF16ToUTF8(frame_name); // Take ownership of the new guest until it is attached to the embedder's DOM // tree to avoid leaking a guest if this guest is destroyed before attaching // the new guest. - pending_new_windows_.insert(std::make_pair(guest, TargetURL(target_url))); + pending_new_windows_.insert(make_pair(guest, target_url.spec())); } void BrowserPluginGuest::RendererUnresponsive(WebContents* source) { @@ -522,14 +483,14 @@ void BrowserPluginGuest::RequestNewWindowPermission( PendingWindowMap::iterator it = pending_new_windows_.find(guest); if (it == pending_new_windows_.end()) return; - const TargetURL& target_url = it->second; + const std::string& target_url = it->second; base::DictionaryValue request_info; request_info.Set(browser_plugin::kInitialHeight, base::Value::CreateIntegerValue(initial_bounds.height())); request_info.Set(browser_plugin::kInitialWidth, base::Value::CreateIntegerValue(initial_bounds.width())); request_info.Set(browser_plugin::kTargetURL, - base::Value::CreateStringValue(target_url.url.spec())); + base::Value::CreateStringValue(target_url)); request_info.Set(browser_plugin::kWindowID, base::Value::CreateIntegerValue(guest->instance_id())); request_info.Set(browser_plugin::kWindowOpenDisposition, @@ -783,31 +744,18 @@ bool BrowserPluginGuest::OnMessageReceived(const IPC::Message& message) { void BrowserPluginGuest::Attach( WebContentsImpl* embedder_web_contents, BrowserPluginHostMsg_CreateGuest_Params params) { - // If a RenderView has already been created for this new window, then we need - // to initialize the browser-side state now so that the RenderViewHostManager - // does not create a new RenderView on navigation. - if (has_render_view_) { - static_cast<RenderViewHostImpl*>( - GetWebContents()->GetRenderViewHost())->Init(); - + const std::string target_url = opener()->pending_new_windows_[this]; + if (!GetWebContents()->opener()) { + // For guests that have a suppressed opener, we navigate now. + // Navigation triggers the creation of a RenderWidgetHostViewGuest so + // we don't need to create one manually. + params.src = target_url; + } else { + // Ensure that the newly attached guest gets a RenderWidgetHostViewGuest. WebContentsViewGuest* new_view = static_cast<WebContentsViewGuest*>(GetWebContents()->GetView()); new_view->CreateViewForWidget(web_contents()->GetRenderViewHost()); } - - // We need to do a navigation here if the target URL has changed between - // the time the WebContents was created and the time it was attached. - // We also need to do an initial navigation if a RenderView was never - // created for the new window in cases where there is no referrer. - PendingWindowMap::iterator it = opener()->pending_new_windows_.find(this); - if (it != opener()->pending_new_windows_.end()) { - const TargetURL& target_url = it->second; - if (target_url.changed || !has_render_view_) - params.src = it->second.url.spec(); - } else { - NOTREACHED(); - } - // Once a new guest is attached to the DOM of the embedder page, then the // lifetime of the new guest is no longer managed by the opener guest. opener()->pending_new_windows_.erase(this); @@ -820,6 +768,14 @@ void BrowserPluginGuest::Attach( Initialize(embedder_web_contents, params); + // We initialize the RenderViewHost after a BrowserPlugin has been attached + // to it and is ready to receive pixels. Until a RenderViewHost is + // initialized, it will not allow any resize requests. + if (!GetWebContents()->GetRenderViewHost()->IsRenderViewLive()) { + static_cast<RenderViewHostImpl*>( + GetWebContents()->GetRenderViewHost())->Init(); + } + // Inform the embedder of the guest's information. // We pull the partition information from the site's URL, which is of the form // guest://site/{persist}?{partition_name}. diff --git a/content/browser/browser_plugin/browser_plugin_guest.h b/content/browser/browser_plugin/browser_plugin_guest.h index 7538550..c335c4b 100644 --- a/content/browser/browser_plugin/browser_plugin_guest.h +++ b/content/browser/browser_plugin/browser_plugin_guest.h @@ -90,12 +90,6 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver, int instance_id, WebContentsImpl* web_contents); - static BrowserPluginGuest* CreateWithOpener( - int instance_id, - WebContentsImpl* web_contents, - BrowserPluginGuest* opener, - bool has_render_view); - // Destroys the guest WebContents and all its associated state, including // this BrowserPluginGuest, and its new unattached windows. void Destroy(); @@ -188,8 +182,6 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver, const std::string& request_method, const base::Callback<void(bool)>& callback) OVERRIDE; virtual bool HandleContextMenu(const ContextMenuParams& params) OVERRIDE; - virtual WebContents* OpenURLFromTab(WebContents* source, - const OpenURLParams& params) OVERRIDE; virtual void WebContentsCreated(WebContents* source_contents, int64 source_frame_id, const string16& frame_name, @@ -261,9 +253,7 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver, friend class TestBrowserPluginGuest; BrowserPluginGuest(int instance_id, - WebContentsImpl* web_contents, - BrowserPluginGuest* opener, - bool has_render_view); + WebContentsImpl* web_contents); // Destroy unattached new windows that have been opened by this // BrowserPluginGuest. @@ -448,18 +438,7 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver, gfx::Size max_auto_size_; gfx::Size min_auto_size_; - // Tracks the target URL of the new window and whether or not it has changed - // since the WebContents has been created and before the new window has been - // attached to a BrowserPlugin. Once the first navigation commits, we no - // longer track this URL. - struct TargetURL { - bool changed; - GURL url; - explicit TargetURL(const GURL& url) : - changed(false), - url(url) {} - }; - typedef std::map<BrowserPluginGuest*, TargetURL> PendingWindowMap; + typedef std::map<BrowserPluginGuest*, std::string> PendingWindowMap; PendingWindowMap pending_new_windows_; BrowserPluginGuest* opener_; // A counter to generate a unique request id for a permission request. @@ -477,11 +456,6 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver, typedef std::map<int, base::Callback<void(bool)> > DownloadRequestMap; DownloadRequestMap download_request_callback_map_; - // Indicates that this BrowserPluginGuest has associated renderer-side state. - // This is used to determine whether or not to create a new RenderView when - // this guest is attached. - bool has_render_view_; - DISALLOW_COPY_AND_ASSIGN(BrowserPluginGuest); }; diff --git a/content/browser/browser_plugin/test_browser_plugin_guest.cc b/content/browser/browser_plugin/test_browser_plugin_guest.cc index 083ea5a..5b1ae21 100644 --- a/content/browser/browser_plugin/test_browser_plugin_guest.cc +++ b/content/browser/browser_plugin/test_browser_plugin_guest.cc @@ -17,7 +17,7 @@ class BrowserPluginGuest; TestBrowserPluginGuest::TestBrowserPluginGuest( int instance_id, WebContentsImpl* web_contents) - : BrowserPluginGuest(instance_id, web_contents, NULL, false), + : BrowserPluginGuest(instance_id, web_contents), update_rect_count_(0), damage_buffer_call_count_(0), exit_observed_(false), diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 0e678f6..7ba9ba4 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -1382,8 +1382,7 @@ void WebContentsImpl::CreateNewWindow( WebContentsImpl* new_contents_impl = static_cast<WebContentsImpl*>(new_contents); new_contents_impl->browser_plugin_guest_.reset( - BrowserPluginGuest::CreateWithOpener(instance_id, new_contents_impl, - GetBrowserPluginGuest(), !!new_contents_impl->opener())); + BrowserPluginGuest::Create(instance_id, new_contents_impl)); } new_contents->Init(create_params); |