Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "Merge pull request #8724 from electron/defer_load_url" #9674

Merged
merged 1 commit into from
Jun 5, 2017
Merged

Revert "Merge pull request #8724 from electron/defer_load_url" #9674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 15 additions & 92 deletions atom/browser/api/atom_api_web_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -272,22 +268,6 @@ void OnCapturePageDone(const base::Callback<void(const gfx::Image&)>& 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,
Expand Down Expand Up @@ -400,7 +380,7 @@ void WebContents::InitWithSessionAndOptions(v8::Isolate* isolate,
content::WebContents *web_contents,
mate::Handle<api::Session> 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);
Expand Down Expand Up @@ -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<content::NavigationController>(controller));

Init(isolate);
AttachAsUserData(web_contents);
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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<content::NavigationEntry>(details).ptr();
content::NavigationEntryImpl* entry_impl =
static_cast<content::NavigationEntryImpl*>(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");
}
Expand Down Expand Up @@ -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) {
Expand Down
25 changes: 1 addition & 24 deletions atom/browser/api/atom_api_web_contents.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -51,8 +49,7 @@ namespace api {

class WebContents : public mate::TrackableObject<WebContents>,
public CommonWebContentsDelegate,
public content::WebContentsObserver,
public content::NotificationObserver {
public content::WebContentsObserver {
public:
enum Type {
BACKGROUND_PAGE, // A DevTools extension background page.
Expand Down Expand Up @@ -326,8 +323,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
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;
Expand All @@ -347,12 +342,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
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;

Expand All @@ -362,13 +351,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
void DevToolsClosed() override;

private:
struct LoadURLParams {
LoadURLParams() : params(GURL()), id(0) {}

content::NavigationController::LoadURLParams params;
int id;
};

AtomBrowserContext* GetBrowserContext() const;

uint32_t GetNextRequestId() {
Expand Down Expand Up @@ -420,11 +402,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
// 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);
};

Expand Down
5 changes: 0 additions & 5 deletions spec/api-browser-window-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 0 additions & 18 deletions spec/static/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down