Skip to content

Refactor NativeWindow (Part 1): Remove WebContentsObserver methods#12008

Merged
zcbenz merged 15 commits into
masterfrom
window-refactor-1
Feb 23, 2018
Merged

Refactor NativeWindow (Part 1): Remove WebContentsObserver methods#12008
zcbenz merged 15 commits into
masterfrom
window-refactor-1

Conversation

@zcbenz

@zcbenz zcbenz commented Feb 22, 2018

Copy link
Copy Markdown
Contributor

This PR relies on #12004, and should be reviewed and merged after it.

This is part of the changes to decouple NativeWindow from WebContents, so it would be possible for us to implement new APIs based on NativeWindow, e.g. providing APIs to create native GUI elements without using WebContents.

This PR moves WebContentsObserver methods of NativeWindow into api::BrowserWindow. Commits have been organized to make it easier to review.

@zcbenz zcbenz requested a review from a team February 22, 2018 09:03
@zcbenz

zcbenz commented Feb 22, 2018

Copy link
Copy Markdown
Contributor Author

It is weird the last commit is showing in the middle of the list, anyway the CI is correctly running for the last commit.

mate::Handle<class WebContents> web_contents) {
web_contents_.Reset(isolate, web_contents.ToV8());
api_web_contents_ = web_contents.get();
Observe(web_contents->web_contents());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe api_web_contents_->web_contents() for clarity.

}

void BrowserWindow::NotifyWindowUnresponsive() {
window_unresposive_closure_.Cancel();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to Cancel the closure once the callback is run ? Also might be a good chance to fix window_unresposive_closure_ => window_unresponsive_closure_.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is code checking whether the callback is cancelled before scheduling a new unresponsive event, so this is necessary.

@zcbenz zcbenz merged commit 15ce235 into master Feb 23, 2018
@zcbenz zcbenz deleted the window-refactor-1 branch February 23, 2018 01:06
@zcbenz zcbenz mentioned this pull request Feb 23, 2018
@alexeykuzmin

Copy link
Copy Markdown
Contributor

It is weird the last commit is showing in the middle of the list, anyway the CI is correctly running for the last commit.

For some reason GitHub shows commits sorted by date, not in "true" order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants