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.

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

Implement initial, experimental BrowserView API #9166

Merged
merged 5 commits into from Apr 13, 2017

Conversation

Projects
None yet
4 participants
@poiru
Member

poiru commented Apr 11, 2017

Right now, <webview> is the only way to embed additional content in a BrowserWindow. Unfortunately <webview> suffers from a number of problems. To make matters worse, many of these are upstream Chromium bugs instead of Electron-specific bugs.

For us at Figma, the main issue is very slow performance.

Despite the upstream improvements to <webview> through the OOPIF work, it is probable that there will continue to be <webview>-specific bugs in the future.

Therefore, this introduces a <webview> alternative to called BrowserView, which...

  • is a thin wrapper around api::WebContents (so bugs in BrowserView will
    likely also be bugs in BrowserWindow web contents)

  • is instantiated in the main process like BrowserWindow (and unlike
    <webview>, which lives in the DOM of a BrowserWindow web contents)

  • needs to be added to a BrowserWindow to display something on the screen

This implements the most basic API. The API is expected to evolve and change in the near future and has consequently been marked as experimental. Please do not use this API in production unless you are prepared to deal with breaking changes.

In the future, we will want to change the API to support multiple BrowserViews per window. We will also want to consider z-ordering, auto-resizing, and possibly even nested views.

@kevinsawicki kevinsawicki self-assigned this Apr 11, 2017

'lib/browser/api/screen.js',
'lib/browser/api/session.js',

This comment has been minimized.

@kevinsawicki

kevinsawicki Apr 11, 2017

Contributor

Thanks for cleaning these up 👍

@kevinsawicki

kevinsawicki Apr 11, 2017

Contributor

Thanks for cleaning these up 👍

@CharlieHess

This comment has been minimized.

Show comment
Hide comment
@CharlieHess

CharlieHess Apr 12, 2017

Contributor

This would be pretty great for Slack, considering our current team-switching approach is basically a stack of webview tags (and resizing them on the fly, which often results in layout jitter). If there is z-index support that would be 💎. A few questions:

  • Will we be able to disable nodeintegration in the same manner as the webview?
  • Would there be any other security loopholes opened by using this instead of webviews?
Contributor

CharlieHess commented Apr 12, 2017

This would be pretty great for Slack, considering our current team-switching approach is basically a stack of webview tags (and resizing them on the fly, which often results in layout jitter). If there is z-index support that would be 💎. A few questions:

  • Will we be able to disable nodeintegration in the same manner as the webview?
  • Would there be any other security loopholes opened by using this instead of webviews?
@poiru

This comment has been minimized.

Show comment
Hide comment
@poiru

poiru Apr 12, 2017

Member

@CharlieHess Yeah, new BrowserView({ webPreferences: { nodeintegration: false }}) works fine. This should be fine for remote content, but please wait until the experimental tag has been removed before using it for that purpose.

Btw. I just added autoresizing support for smooth resizing!

Member

poiru commented Apr 12, 2017

@CharlieHess Yeah, new BrowserView({ webPreferences: { nodeintegration: false }}) works fine. This should be fine for remote content, but please wait until the experimental tag has been removed before using it for that purpose.

Btw. I just added autoresizing support for smooth resizing!

poiru added some commits Apr 11, 2017

Implement initial, experimental BrowserView API
Right now, `<webview>` is the only way to embed additional content in a
`BrowserWindow`. Unfortunately `<webview>` suffers from a [number of
problems](https://github.com/electron/electron/issues?utf8=%E2%9C%93&q=is%3Aissue%20is%3Aopen%20label%3Awebview%20).
To make matters worse, many of these are upstream Chromium bugs instead
of Electron-specific bugs.

For us at [Figma](https://www.figma.com), the main issue is very slow
performance.

Despite the upstream improvements to `<webview>` through the OOPIF work, it is
probable that there will continue to be `<webview>`-specific bugs in the
future.

Therefore, this introduces a `<webview>` alternative to called `BrowserView`,
which...

- is a thin wrapper around `api::WebContents` (so bugs in `BrowserView` will
  likely also be bugs in `BrowserWindow` web contents)

- is instantiated in the main process like `BrowserWindow` (and unlike
  `<webview>`, which lives in the DOM of a `BrowserWindow` web contents)

- needs to be added to a `BrowserWindow` to display something on the screen

This implements the most basic API. The API is expected to evolve and change in
the near future and has consequently been marked as experimental. Please do not
use this API in production unless you are prepared to deal with breaking
changes.

In the future, we will want to change the API to support multiple
`BrowserView`s per window. We will also want to consider z-ordering
auto-resizing, and possibly even nested views.
@deepak1556

LGTM , thanks @poiru !

@poiru poiru merged commit 7f4bd79 into master Apr 13, 2017

6 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
electron-mas-x64 Build #3902 succeeded in 8 min 2 sec
Details
electron-osx-x64 Build #3896 succeeded in 8 min 38 sec
Details
electron-win-ia32 Build #2891 succeeded in 8 min 16 sec
Details
electron-win-x64 Build #2865 succeeded in 8 min 20 sec
Details

@poiru poiru deleted the browserview branch Apr 13, 2017

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