Skip to content

Conversation

@joyceerhl
Copy link
Contributor

Motivation for features / changes

Hi! I'm a dev working on adding TensorBoard support to the Python and Jupyter extensions for VS Code. We'd like to enable users to view TensorBoard inline in our Jupyter notebook experience within VS Code.

Currently this doesn't work because the frame-ancestors * directive prevents VS Code from framing TensorBoard. This is because VS Code is an Electron application, and Electron appears to be unable to frame websites which set frame-ancestors * in its response headers: electron/electron#26369

Technical description of changes

This PR essentially reverses #2797.

If I'm reading the CSP specification correctly, omitting the frame-ancestors directive altogether is equivalent to setting frame-ancestors *, so to my knowledge this PR should not result in a behavior change for environments which correctly implement the CSP spec. From https://w3c.github.io/webappsec-csp/2/#directive-frame-ancestors:

The term allowed frame ancestors refers to the result of parsing the frame-ancestors directive’s value as a source list. If a frame-ancestors directive is not explicitly included in the policy, then allowed frame ancestors is "*".

Screenshots of UI changes

With this PR, users can now display TensorBoard inline in a Jupyter notebook in VS Code:
image
image

Detailed steps to verify changes work correctly (as executed by you)

  1. Remove line 223 which sets frame-ancestors * from backend/http_util.py
  2. Install tensorboard package locally
  3. Install Visual Studio Code from https://code.visualstudio.com/download and add it to your PATH during the installation stage
  4. Install the Python extension for VS Code by running code --install-extension ms-python.python in your shell
  5. Open VS Code
  6. Hit 'Cmd/Ctrl + Shift + P' to bring up the command palette. Type 'Jupyter: Create New Blank Jupyter Notebook' and press enter. This will create a blank Jupyter notebook
  7. In a cell, run

from IPython.display import IFrame
IFrame('http://localhost:6060', width=700, height=350)

  1. tensorboard should now be rendered inline in the cell output

Thank you and please let me know if you have questions about this proposed change!

@google-cla
Copy link

google-cla bot commented Nov 16, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Nov 16, 2020
@joyceerhl
Copy link
Contributor Author

@googlebot I signed it!

@google-cla google-cla bot added cla: yes and removed cla: no labels Nov 17, 2020
@stephanwlee
Copy link
Contributor

I will try to cherry pick this change to make sure it does not negatively impact Colab and other services.

@stephanwlee
Copy link
Contributor

Manually checked internal version and Colab and noticed no regression.

LGTM!

@stephanwlee stephanwlee merged commit c58ea35 into tensorflow:master Nov 17, 2020
@wchargin
Copy link
Contributor

Weird. Thanks for filing the upstream issue!

@wchargin
Copy link
Contributor

…and congrats on the launch! :-)

@wchargin wchargin mentioned this pull request Jan 13, 2021
2 tasks
wchargin pushed a commit that referenced this pull request Jan 14, 2021
This PR essentially reverses #2797.

Currently this doesn't work because the `frame-ancestors *` directive prevents VS Code from framing TensorBoard. This is because VS Code is an Electron application, and Electron appears to be unable to frame websites which set `frame-ancestors *` in its response headers: electron/electron#26369

If I'm reading the CSP specification correctly, omitting the frame-ancestors directive altogether is equivalent to setting `frame-ancestors *`, so to my knowledge this PR should not result in a behavior change for environments which correctly implement the CSP spec. From https://w3c.github.io/webappsec-csp/2/#directive-frame-ancestors:

> The term allowed frame ancestors refers to the result of parsing the frame-ancestors directive’s value as a source list. If a frame-ancestors directive is not explicitly included in the policy, then allowed frame ancestors is "*".
wchargin added a commit that referenced this pull request Jan 14, 2021
Backport of #4332 to 2.4. Cf. #4547.

---

This PR essentially reverses #2797.

Currently this doesn't work because the `frame-ancestors *` directive prevents VS Code from framing TensorBoard. This is because VS Code is an Electron application, and Electron appears to be unable to frame websites which set `frame-ancestors *` in its response headers: electron/electron#26369

If I'm reading the CSP specification correctly, omitting the frame-ancestors directive altogether is equivalent to setting `frame-ancestors *`, so to my knowledge this PR should not result in a behavior change for environments which correctly implement the CSP spec. From https://w3c.github.io/webappsec-csp/2/#directive-frame-ancestors:

> The term allowed frame ancestors refers to the result of parsing the frame-ancestors directive’s value as a source list. If a frame-ancestors directive is not explicitly included in the policy, then allowed frame ancestors is "*".

Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
@wchargin wchargin mentioned this pull request Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants