From 50ac7b2eddfec20130ff943ad07e93b193fd88d7 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Fri, 7 Apr 2017 23:04:14 +0530 Subject: [PATCH] Revert "Merge pull request #8724 from electron/defer_load_url" Possible cause for #9121 This reverts commit 886b47e71344e68a69c16d387ac83f429c074993, reversing changes made to 479af3c9e2549fff7a24ccf7e9f158a020842ba8. --- atom/browser/api/atom_api_web_contents.cc | 107 +++------------------- atom/browser/api/atom_api_web_contents.h | 25 +---- spec/api-browser-window-spec.js | 5 - spec/static/main.js | 18 ---- 4 files changed, 16 insertions(+), 139 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 659daae18b9..78190dbacbe 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -50,7 +50,6 @@ #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/renderer_host/render_widget_host_view_base.h" #include "content/browser/web_contents/web_contents_impl.h" @@ -60,9 +59,6 @@ #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" @@ -272,22 +268,6 @@ void OnCapturePageDone(const 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, @@ -400,7 +380,7 @@ void WebContents::InitWithSessionAndOptions(v8::Isolate* isolate, content::WebContents *web_contents, mate::Handle session, const mate::Dictionary& options) { - content::WebContentsObserver::Observe(web_contents); + Observe(web_contents); InitWithWebContents(web_contents, session->browser_context()); managed_web_contents()->GetView()->SetDelegate(this); @@ -436,11 +416,6 @@ 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); } @@ -827,30 +802,6 @@ 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(); @@ -893,41 +844,6 @@ 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->frame_tree_node_id() == -1) { - deferred_load_url_.id = entry->GetUniqueID(); - } - break; - } - default: - NOTREACHED(); - break; - } -} - -void WebContents::BeforeUnloadDialogCancelled() { - if (deferred_load_url_.id) { - auto& controller = web_contents()->GetController(); - if (!controller.GetPendingEntry()) { - deferred_load_url_.id = 0; - } - } -} - void WebContents::DevToolsReloadPage() { Emit("devtools-reload-page"); } @@ -1096,16 +1012,23 @@ 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; - - if (deferred_load_url_.id) { - deferred_load_url_.params = params; - return; - } - web_contents()->GetController().LoadURLWithParams(params); + + // Set the background color of RenderWidgetHostView. // We have to call it right after LoadURL because the RenderViewHost is only // created after loading a page. - SetBackgroundColor(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); + } + } } 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 3c8124bf499..a65ff5148eb 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -14,8 +14,6 @@ #include "atom/browser/common_web_contents_delegate.h" #include "atom/browser/ui/autofill_popup.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" @@ -51,8 +49,7 @@ namespace api { class WebContents : public mate::TrackableObject, public CommonWebContentsDelegate, - public content::WebContentsObserver, - public content::NotificationObserver { + public content::WebContentsObserver { public: enum Type { BACKGROUND_PAGE, // A DevTools extension background page. @@ -326,8 +323,6 @@ 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; @@ -347,12 +342,6 @@ 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; - void BeforeUnloadDialogCancelled() override; - // brightray::InspectableWebContentsDelegate: void DevToolsReloadPage() override; @@ -362,13 +351,6 @@ 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() { @@ -420,11 +402,6 @@ 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 5c084cb178f..e6e4661f7bb 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -298,11 +298,6 @@ describe('BrowserWindow module', function () { w.loadURL(`data:image/png;base64,${data}`) }) - it('should not crash when there is a pending navigation entry', function (done) { - ipcRenderer.once('navigated-with-pending-entry', () => done()) - ipcRenderer.send('navigate-with-pending-entry', w.id) - }) - describe('POST navigations', function () { afterEach(() => { w.webContents.session.webRequest.onBeforeSendHeaders(null) diff --git a/spec/static/main.js b/spec/static/main.js index 9e59037a550..3d1f7c0a95b 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -329,24 +329,6 @@ ipcMain.on('handle-unhandled-rejection', (event, message) => { }) }) -ipcMain.on('navigate-with-pending-entry', (event, id) => { - const w = BrowserWindow.fromId(id) - - w.webContents.on('did-start-loading', () => { - w.loadURL('about:blank') - }) - - w.webContents.on('did-navigate', (e, url) => { - if (url === 'about:blank') { - event.sender.send('navigated-with-pending-entry') - } - }) - - w.webContents.session.clearHostResolverCache(() => { - w.loadURL('http://host') - }) -}) - ipcMain.on('crash-service-pid', (event, pid) => { process.crashServicePid = pid event.returnValue = null