From dead1ae1ba5a1374bf6f41a0a79b92da82d7263f Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Mon, 20 Feb 2017 02:13:17 +0530 Subject: [PATCH] webContents: defer url load when there is a pending navigation entry --- atom/browser/api/atom_api_web_contents.cc | 99 ++++++++++++++++++++++++++----- atom/browser/api/atom_api_web_contents.h | 24 +++++++- spec/api-browser-window-spec.js | 18 ++++++ 3 files changed, 125 insertions(+), 16 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 89e7a7d..94de2e6 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -46,6 +46,7 @@ #include "chrome/browser/printing/print_preview_message_handler.h" #include "chrome/browser/printing/print_view_manager_basic.h" #include "chrome/browser/ssl/security_state_tab_helper.h" +#include "content/browser/frame_host/navigation_entry_impl.h" #include "content/browser/renderer_host/render_widget_host_impl.h" #include "content/browser/web_contents/web_contents_impl.h" #include "content/common/view_messages.h" @@ -54,6 +55,9 @@ #include "content/public/browser/navigation_details.h" #include "content/public/browser/navigation_entry.h" #include "content/public/browser/navigation_handle.h" +#include "content/public/browser/notification_details.h" +#include "content/public/browser/notification_source.h" +#include "content/public/browser/notification_types.h" #include "content/public/browser/plugin_service.h" #include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_process_host.h" @@ -242,6 +246,22 @@ void OnCapturePageDone(base::Callback callback, callback.Run(gfx::Image::CreateFrom1xBitmap(bitmap)); } +// Set the background color of RenderWidgetHostView. +void SetBackgroundColor(content::WebContents* web_contents) { + const auto view = web_contents->GetRenderWidgetHostView(); + if (view) { + WebContentsPreferences* web_preferences = + WebContentsPreferences::FromWebContents(web_contents); + std::string color_name; + if (web_preferences->web_preferences()->GetString(options::kBackgroundColor, + &color_name)) { + view->SetBackgroundColor(ParseHexColor(color_name)); + } else { + view->SetBackgroundColor(SK_ColorTRANSPARENT); + } + } +} + } // namespace WebContents::WebContents(v8::Isolate* isolate, @@ -338,7 +358,7 @@ void WebContents::InitWithSessionAndOptions(v8::Isolate* isolate, content::WebContents *web_contents, mate::Handle session, const mate::Dictionary& options) { - Observe(web_contents); + content::WebContentsObserver::Observe(web_contents); InitWithWebContents(web_contents, session->browser_context()); managed_web_contents()->GetView()->SetDelegate(this); @@ -374,6 +394,11 @@ void WebContents::InitWithSessionAndOptions(v8::Isolate* isolate, SetOwnerWindow(owner_window); } + const content::NavigationController* controller = + &web_contents->GetController(); + registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_PENDING, + content::Source(controller)); + Init(isolate); AttachAsUserData(web_contents); } @@ -733,6 +758,30 @@ void WebContents::DidGetRedirectForResourceRequest( details.headers.get()); } +void WebContents::DidStartNavigation( + content::NavigationHandle* navigation_handle) { + if (!navigation_handle->IsInMainFrame() || navigation_handle->IsSamePage()) + return; + + if (deferred_load_url_.id) { + auto web_contents = navigation_handle->GetWebContents(); + auto& controller = web_contents->GetController(); + int id = controller.GetPendingEntry()->GetUniqueID(); + if (id == deferred_load_url_.id) { + if (!deferred_load_url_.params.url.is_empty()) { + auto params = deferred_load_url_.params; + deferred_load_url_.id = 0; + deferred_load_url_.params = + content::NavigationController::LoadURLParams(GURL()); + controller.LoadURLWithParams(params); + SetBackgroundColor(web_contents); + } else { + deferred_load_url_.id = 0; + } + } + } +} + void WebContents::DidFinishNavigation( content::NavigationHandle* navigation_handle) { bool is_main_frame = navigation_handle->IsInMainFrame(); @@ -777,6 +826,33 @@ void WebContents::DidUpdateFaviconURL( Emit("page-favicon-updated", unique_urls); } +void WebContents::Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) { + switch (type) { + case content::NOTIFICATION_NAV_ENTRY_PENDING: { + content::NavigationEntry* entry = + content::Details(details).ptr(); + content::NavigationEntryImpl* entry_impl = + static_cast(entry); + // In NavigatorImpl::DidStartMainFrameNavigation when there is no + // browser side pending entry available it creates a new one based + // on existing pending entry, hence we track the unique id here + // instead in WebContents::LoadURL with controller.GetPendingEntry() + // TODO(deepak1556): Remove once we have + // https://codereview.chromium.org/2661743002. + if (entry_impl->is_renderer_initiated() && + entry_impl->frame_tree_node_id() == -1) { + deferred_load_url_.id = entry->GetUniqueID(); + } + break; + } + default: + NOTREACHED(); + break; + } +} + void WebContents::DevToolsReloadPage() { Emit("devtools-reload-page"); } @@ -921,23 +997,16 @@ void WebContents::LoadURL(const GURL& url, const mate::Dictionary& options) { params.transition_type = ui::PAGE_TRANSITION_TYPED; params.should_clear_history_list = true; params.override_user_agent = content::NavigationController::UA_OVERRIDE_TRUE; - web_contents()->GetController().LoadURLWithParams(params); - // Set the background color of RenderWidgetHostView. + if (deferred_load_url_.id) { + deferred_load_url_.params = params; + return; + } + + web_contents()->GetController().LoadURLWithParams(params); // We have to call it right after LoadURL because the RenderViewHost is only // created after loading a page. - const auto view = web_contents()->GetRenderWidgetHostView(); - if (view) { - WebContentsPreferences* web_preferences = - WebContentsPreferences::FromWebContents(web_contents()); - std::string color_name; - if (web_preferences->web_preferences()->GetString(options::kBackgroundColor, - &color_name)) { - view->SetBackgroundColor(ParseHexColor(color_name)); - } else { - view->SetBackgroundColor(SK_ColorTRANSPARENT); - } - } + SetBackgroundColor(web_contents()); } void WebContents::DownloadURL(const GURL& url) { diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h index a37fb8a..41edcc2 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -13,6 +13,8 @@ #include "atom/browser/api/trackable_object.h" #include "atom/browser/common_web_contents_delegate.h" #include "content/common/cursors/webcursor.h" +#include "content/public/browser/notification_observer.h" +#include "content/public/browser/notification_registrar.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/common/favicon_url.h" #include "native_mate/handle.h" @@ -46,7 +48,8 @@ namespace api { class WebContents : public mate::TrackableObject, public CommonWebContentsDelegate, - public content::WebContentsObserver { + public content::WebContentsObserver, + public content::NotificationObserver { public: enum Type { BACKGROUND_PAGE, // A DevTools extension background page. @@ -308,6 +311,8 @@ class WebContents : public mate::TrackableObject, const content::ResourceRequestDetails& details) override; void DidGetRedirectForResourceRequest( const content::ResourceRedirectDetails& details) override; + void DidStartNavigation( + content::NavigationHandle* navigation_handle) override; void DidFinishNavigation( content::NavigationHandle* navigation_handle) override; bool OnMessageReceived(const IPC::Message& message) override; @@ -325,6 +330,11 @@ class WebContents : public mate::TrackableObject, const MediaPlayerId& id) override; void DidChangeThemeColor(SkColor theme_color) override; + // content::NotificationObserver: + void Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) override; + // brightray::InspectableWebContentsDelegate: void DevToolsReloadPage() override; @@ -334,6 +344,13 @@ class WebContents : public mate::TrackableObject, void DevToolsClosed() override; private: + struct LoadURLParams { + LoadURLParams() : params(GURL()), id(0) {} + + content::NavigationController::LoadURLParams params; + int id; + }; + AtomBrowserContext* GetBrowserContext() const; uint32_t GetNextRequestId() { @@ -384,6 +401,11 @@ class WebContents : public mate::TrackableObject, // Whether to enable devtools. bool enable_devtools_; + // Container to hold url parms for deferred load when + // there is a pending navigation entry. + LoadURLParams deferred_load_url_; + content::NotificationRegistrar registrar_; + DISALLOW_COPY_AND_ASSIGN(WebContents); }; diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 354befd..9631de8 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -229,6 +229,24 @@ describe('BrowserWindow module', function () { w.loadURL(`data:image/png;base64,${data}`) }) + it('should not crash when there is a pending navigation entry', function (done) { + const source = ` + + ` + w.webContents.on('did-fail-load', function (event, code, desc, url, isMainFrame) { + assert.equal(url, 'http://host/') + w.webContents.loadURL('about:blank') + }) + w.webContents.on('did-navigate', function (event, url) { + if (url === 'about:blank') done() + }) + w.loadURL(`data:text/html,${source}`) + }) + describe('POST navigations', function () { afterEach(() => { w.webContents.session.webRequest.onBeforeSendHeaders(null) -- 2.7.4