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: extension execution contexts #2812

Closed
wants to merge 1 commit into from

Conversation

JoelEinbinder
Copy link
Collaborator

No description provided.

#### executionContext.name()
- returns: <[string]> A human readable name for the execution context.

Most execution contexts have an empty string for a name. Execution contexts created by extensions will have the title of the extension as thier name.
Copy link
Contributor

Choose a reason for hiding this comment

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

thier -> their :)

@@ -1116,6 +1131,9 @@ puppeteer.launch().then(async browser => {

```

#### page.extensionContexts()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like restricting us to extensions. Execution contexts can exist and might be handy in non-chromium embedders, e.g. in headless.

From this point of view, the ideal API would be like this:

// ExecutionContext lifecycle
page.on('executioncontextcreated');
page.on('executioncontextdestroyed');

// Querying  contexts
const contexts = frame.executionContexts();
const context = frame.defaultExecutionContext();

// Inspecting context
context.name(); // returns extension name, if any
context.isDefault(); // returns `true` if this is a default execution context

The only downside here is that we already have frame.executionContext() instead of frame.defaultExecutionContext(). I think we can keep it like this for now, and rename it later on in the API cleanup for v2.0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

An api that contains all execution contexts is reasonable, but id still want helper methods specifically for extension contexts.

I’m wary of releasing an api that will today be effectively only extension contexts, but in the future might contain some more exotic contexts.

Copy link
Contributor

Choose a reason for hiding this comment

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

An api that contains all execution contexts is reasonable, but id still want helper methods specifically for extension contexts.

I'd move slowly into this direction. For the first-class extension support, we'll need to have an Extension class - there you might have methods like extension.contentScript(frame) and others.

@aslushnikov
Copy link
Contributor

Closing this in favor of #2812 (comment)

aslushnikov added a commit to aslushnikov/puppeteer that referenced this pull request Aug 8, 2018
This patch exposes frame's execution contexts, making it possible
to debug extension's content scripts.

This is a resurrected puppeteer#2812.
aslushnikov added a commit that referenced this pull request Aug 9, 2018
This patch exposes frame's execution contexts, making it possible
to debug extension's content scripts.

This is a resurrected #2812.
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

3 participants