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.

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

feat(chromium): roll Chromium to r682225 #4844

Merged
merged 1 commit into from Aug 14, 2019

Conversation

aslushnikov
Copy link
Contributor

@aslushnikov aslushnikov commented Aug 14, 2019

This roll includes:

The SiteInstance by default was breaking "devtools: true" option, so
there's a new feature we disable now by default.

This keeps pressuring us towards OOPIF support since that's an
inevitable future.

@JoelEinbinder
Copy link
Collaborator

Is the headless team still using site-per-process=off? If so, I imagine other people will want it off for performance reasons. Maybe this fix should happen on the devtools side, so that devtools doesnt crash with this flag?

@@ -116,6 +117,13 @@ module.exports.addTests = function({testRunner, expect, puppeteer, defaultBrowse
await page.click('body');
await browser.close();
});
it('should open devtools when "devtools: true" option is given', async({server}) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

firefox won't like this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? We run headful tests on CHROME only

@aslushnikov
Copy link
Contributor Author

If so, I imagine other people will want it off for performance reasons.

I don't think there are any embedders that will use this in the long run - this mode is going away.

This roll includes:
- https://crrev.com/681997 - Turn on default SiteInstance by default.

The SiteInstance by default was breaking "devtools: true" option, so
there's a new feature we disable now by default.

This keeps pressuring us towards OOPIF support since that's an
inevitable future.
@aslushnikov aslushnikov changed the title fix(launcher): fix "devtools: true" option feat(chromium): roll Chromium to r682225 Aug 14, 2019
Copy link
Collaborator

@JoelEinbinder JoelEinbinder left a comment

Choose a reason for hiding this comment

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

Description is out of date

@aslushnikov aslushnikov merged commit 0e0a679 into puppeteer:master Aug 14, 2019
@aslushnikov aslushnikov deleted the fix-devtools branch August 14, 2019 22:26
rfojtik pushed a commit to rfojtik/puppeteer that referenced this pull request Dec 21, 2019
This roll includes:
- https://crrev.com/681997 - Turn on default SiteInstance by default.

The SiteInstance by default was breaking "devtools: true" option, so
there's a new feature we disable now by default.

This keeps pressuring us towards OOPIF support since that's an
inevitable future.
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.

None yet

2 participants