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

8211308: Support HTTP/2 in WebView #14

Closed
wants to merge 2 commits into from
Closed

Conversation

@arajkumar
Copy link
Contributor

arajkumar commented Oct 11, 2019

The goal of this enhancement is to use new HttpClient APIs available from JDK 11.

Reference:
[1] https://openjdk.java.net/groups/net/httpclient/intro.html
[2] https://docs.oracle.com/en/java/javase/11/docs/api/java.net.http/java/net/http/HttpClient.html

Though this uses JDK 11 HttpClient APIs, it needs latest JDK 12 to work correctly due to the dependency on following issues,

JDK-8218546 Unable to connect to https://google.com using java.net.HttpClient
JDK-8218662 Allow 204 responses with Content-Length:0
JDK-8203850 java.net.http HTTP client should allow specifying Origin and Referer headers

Task List

  • simple GET requests
  • Runtime setting to fallback to legacy client
  • Runtime settings to use only HTTP/1.1
  • sync requests
  • Error Handling & Propagation
  • POST with form data
  • AccessController association for HttpClient.sendAsync / send
  • Redirection
  • Check for possibilities to write unit tests
  • Sync request handling from WebCore java platform layer
  • Make use of singleton instance of direct ByteBuffer instead of using allocator pool
  • gzip, deflate encoding support

HTTP/2 Test pages

Redirection Test

Here is a http2 demo page result,

screen shot 2018-10-11 at 12 01 12 am

More details are available at javafxports/openjdk-jfx#247.

Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

JDK-8211308: Support HTTP/2 in WebView

Approvers

  • Guru Hb (ghb - Reviewer)
  • Kevin Rushforth (kcr - Reviewer)
Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
@bridgekeeper
Copy link

bridgekeeper bot commented Oct 11, 2019

👋 Welcome back arajkumar! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request (refresh this page to view it).

@openjdk openjdk bot added the rfr label Oct 11, 2019
@mlbridge
Copy link

mlbridge bot commented Oct 11, 2019

Webrevs

@arajkumar
Copy link
Contributor Author

arajkumar commented Oct 11, 2019

Still few changes need to be done as suggested by @kevinrushforth.

@johanvos
Copy link
Collaborator

johanvos commented Oct 11, 2019

Good work. Should the title be prefixed with WIP until it's ready for review, so that Skara will send the RFR when it is ready for review?

@arajkumar arajkumar changed the title 8211308: Support HTTP/2 in WebView WIP: 8211308: Support HTTP/2 in WebView Oct 11, 2019
@openjdk openjdk bot removed the rfr label Oct 11, 2019
@arajkumar
Copy link
Contributor Author

arajkumar commented Oct 11, 2019

I was wondering why @skara had sent RFR when this PR is still in draft stage. Actually @skara should consider the "Draft" attribute associated with the PR.

@rwestberg
Copy link
Member

rwestberg commented Oct 11, 2019

Good point, I've created https://bugs.openjdk.java.net/browse/SKARA-129 to track this.

@arajkumar arajkumar changed the title WIP: 8211308: Support HTTP/2 in WebView 8211308: Support HTTP/2 in WebView Oct 16, 2019
@arajkumar arajkumar marked this pull request as ready for review Oct 16, 2019
@openjdk openjdk bot added the rfr label Oct 16, 2019
@arajkumar
Copy link
Contributor Author

arajkumar commented Oct 16, 2019

@jfx team, now it is ready for a fresh review :)

@kevinrushforth
Copy link
Member

kevinrushforth commented Oct 16, 2019

@guruhb please also review this.

@kevinrushforth
Copy link
Member

kevinrushforth commented Oct 16, 2019

I note that this change needs two reviewers (so should not be integrated until there are two approved reviews).

@kevinrushforth kevinrushforth mentioned this pull request Oct 21, 2019
9 of 12 tasks complete
@arajkumar
Copy link
Contributor Author

arajkumar commented Nov 13, 2019

/ping

@openjdk
Copy link

openjdk bot commented Nov 13, 2019

@arajkumar Unknown command ping - for a list of valid commands use /help.

@kevinrushforth
Copy link
Member

kevinrushforth commented Nov 27, 2019

I did a fair bit of testing yesterday on all three platforms, and it all looks good to me. I verified the behavior on JDK 11 LTS (not enabled by default) and on JDK 13 (is enabled by default). I have one more things to check next week.

@guruhb can you also review?

@guruhb
guruhb approved these changes Nov 28, 2019
Copy link
Contributor

guruhb left a comment

Looks good to me.

@openjdk openjdk bot removed the rfr label Nov 28, 2019
@openjdk
Copy link

openjdk bot commented Nov 28, 2019

@arajkumar This change can now be integrated. The commit message will be:

8211308: Support HTTP/2 in WebView

Reviewed-by: ghb, kcr
  • If you would like to add a summary, use the /summary command.
  • To list additional contributors, use the /contributor command.

Since the source branch of this PR was last updated there have been 44 commits pushed to the master branch:

  • 6892fa1: 8232064: Switch FX build to use JDK 13.0.1 as boot JDK
  • 1d670f1: 8200224: Multiple press event when JFXPanel gains focus
  • 83eb0a7: 8193445: JavaFX CSS is applied redundantly leading to significant performance degradation
  • 798afbc: 8230610: Upgrade GStreamer to version 1.16.1
  • 126896d: 8234704: Fix attribution in libxslt.md
  • 4d3c723: 8234593: Mark LeakTest.testGarbageCollectability as unstable
  • 5a39824: 8234056: Upgrade to libxslt 1.1.34
  • 8bea7b7: 8229472: Deprecate for removal JavaBeanXxxPropertyBuilders constructors
  • aad1720: 8233420: Upgrade to gcc 8.3 on Linux
  • 42040c4: 8232063: Upgrade gradle to version 6.0
  • aab07a4: 8234239: [TEST_BUG] Reenable few ignored web tests
  • 95ad601: 8233421: Upgrade to Visual Studio 2017 version 15.9.16
  • 3e0557a: 8234303: [TEST_BUG] Correct ignore tag in graphics unit tests
  • e37cb37: 8234150: Address ignored tests in ComboBoxTest, LabeledTest, HyperLinkTest and TextInputControlTest
  • 4f496d4: 8234194: [TEST_BUG] Reenable few graphics unit tests
  • 927fc8a: 8234174: Change IDEA VCS mapping to Git
  • 3d0cb49: 8234189: [TEST_BUG] Remove ignored and invalid graphics unit tests
  • dc01309: 8234110: SwingFXUtilsTest is unsuitable for unit test framework
  • 5b96ee4: 8231188: Update SQLite to version 3.30.1
  • d46dcae: 8233338: FX javadoc headings are out of sequence
  • 94bcf3f: 8231692: Test Infrastructure: enhance KeyEventFirer to inject keyEvents into scene
  • 286d1b5: 8230492: font-family not set in HTMLEditor if font name has a number in it
  • f74f3af: 8233040: ComboBoxPopupControl: remove eventFilter for F4
  • a1cc4ab: 8232210: Update Mesa 3-D Headers to version 19.2.1
  • dca8df4: 8232943: Gesture support is not initialized on iOS
  • 5a70b0c: 8189092: ArrayIndexOutOfBoundsException on Linux in getCachedGlyph
  • ac71396: 8232929: Duplicate symbols when building static libraries
  • ab6ea3b: 8232158: [macOS] Fallback to command line tools if xcode is missing
  • 2ae171a: 8232687: No static JNI loader for libprism-sw
  • a09a0fa: 8232522: FX: Update copyright year in docs, readme files to 2020
  • 582d999: Merge
  • b6e53f4: 8218640: Update ICU4C to version 64.2
  • bada612: Merge
  • 64aaeb8: 8226754: FX build fails using gradle 5.6+ or 6
  • a4bc22d: Merge
  • 2593dea: Merge
  • 9df20c5: Merge
  • a1aa38a: Merge
  • 0ff02bb: Merge
  • a433bf2: Merge
  • 561153d: Merge
  • 75d439e: Merge
  • 758252f: Merge
  • 35e0cae: 8227402: Improve XSLT processing

Since there are no conflicts, your changes will automatically be rebased on top of the above commits when integrating. If you prefer to do this manually, please merge master into your branch first.

  • To integrate this PR with the above commit message, type /integrate in a new comment.
@openjdk openjdk bot added the ready label Nov 28, 2019
@arajkumar
Copy link
Contributor Author

arajkumar commented Dec 3, 2019

/integrate

@openjdk openjdk bot closed this Dec 3, 2019
@openjdk openjdk bot added integrated and removed ready labels Dec 3, 2019
@openjdk
Copy link

openjdk bot commented Dec 3, 2019

@arajkumar The following commits have been pushed to master since your change was applied:

  • 6892fa1: 8232064: Switch FX build to use JDK 13.0.1 as boot JDK
  • 1d670f1: 8200224: Multiple press event when JFXPanel gains focus
  • 83eb0a7: 8193445: JavaFX CSS is applied redundantly leading to significant performance degradation
  • 798afbc: 8230610: Upgrade GStreamer to version 1.16.1
  • 126896d: 8234704: Fix attribution in libxslt.md
  • 4d3c723: 8234593: Mark LeakTest.testGarbageCollectability as unstable
  • 5a39824: 8234056: Upgrade to libxslt 1.1.34
  • 8bea7b7: 8229472: Deprecate for removal JavaBeanXxxPropertyBuilders constructors
  • aad1720: 8233420: Upgrade to gcc 8.3 on Linux
  • 42040c4: 8232063: Upgrade gradle to version 6.0
  • aab07a4: 8234239: [TEST_BUG] Reenable few ignored web tests
  • 95ad601: 8233421: Upgrade to Visual Studio 2017 version 15.9.16
  • 3e0557a: 8234303: [TEST_BUG] Correct ignore tag in graphics unit tests
  • e37cb37: 8234150: Address ignored tests in ComboBoxTest, LabeledTest, HyperLinkTest and TextInputControlTest
  • 4f496d4: 8234194: [TEST_BUG] Reenable few graphics unit tests
  • 927fc8a: 8234174: Change IDEA VCS mapping to Git
  • 3d0cb49: 8234189: [TEST_BUG] Remove ignored and invalid graphics unit tests
  • dc01309: 8234110: SwingFXUtilsTest is unsuitable for unit test framework
  • 5b96ee4: 8231188: Update SQLite to version 3.30.1
  • d46dcae: 8233338: FX javadoc headings are out of sequence
  • 94bcf3f: 8231692: Test Infrastructure: enhance KeyEventFirer to inject keyEvents into scene
  • 286d1b5: 8230492: font-family not set in HTMLEditor if font name has a number in it
  • f74f3af: 8233040: ComboBoxPopupControl: remove eventFilter for F4
  • a1cc4ab: 8232210: Update Mesa 3-D Headers to version 19.2.1
  • dca8df4: 8232943: Gesture support is not initialized on iOS
  • 5a70b0c: 8189092: ArrayIndexOutOfBoundsException on Linux in getCachedGlyph
  • ac71396: 8232929: Duplicate symbols when building static libraries
  • ab6ea3b: 8232158: [macOS] Fallback to command line tools if xcode is missing
  • 2ae171a: 8232687: No static JNI loader for libprism-sw
  • a09a0fa: 8232522: FX: Update copyright year in docs, readme files to 2020
  • 582d999: Merge
  • b6e53f4: 8218640: Update ICU4C to version 64.2
  • bada612: Merge
  • 64aaeb8: 8226754: FX build fails using gradle 5.6+ or 6
  • a4bc22d: Merge
  • 2593dea: Merge
  • 9df20c5: Merge
  • a1aa38a: Merge
  • 0ff02bb: Merge
  • a433bf2: Merge
  • 561153d: Merge
  • 75d439e: Merge
  • 758252f: Merge
  • 35e0cae: 8227402: Improve XSLT processing

Your commit was automatically rebased without conflicts.

Pushed as commit 98035cb.

@mlbridge
Copy link

mlbridge bot commented Dec 3, 2019

Mailing list message from Arunprasad Rajkumar on openjfx-dev:

Changeset: 98035cb
Author: Arunprasad Rajkumar
Date: 2019-12-03 08:24:01 +0000
URL: https://git.openjdk.java.net/jfx/commit/98035cb2

8211308: Support HTTP/2 in WebView

Reviewed-by: ghb, kcr

! modules/javafx.web/src/main/java/com/sun/webkit/network/ByteBufferPool.java

  • modules/javafx.web/src/main/java/com/sun/webkit/network/HTTP2Loader.java
    ! modules/javafx.web/src/main/java/com/sun/webkit/network/NetworkContext.java
    ! modules/javafx.web/src/main/java/com/sun/webkit/network/URLLoader.java
  • modules/javafx.web/src/main/java/com/sun/webkit/network/URLLoaderBase.java
    ! modules/javafx.web/src/main/java/module-info.java
    ! modules/javafx.web/src/main/native/Source/WebCore/mapfile-macosx
    ! modules/javafx.web/src/main/native/Source/WebCore/mapfile-vers
    ! modules/javafx.web/src/main/native/Source/WebCore/platform/java/IDNJava.cpp
    ! modules/javafx.web/src/main/native/Source/WebCore/platform/network/ResourceHandle.h
    ! modules/javafx.web/src/main/native/Source/WebCore/platform/network/ResourceHandleInternal.h
    ! modules/javafx.web/src/main/native/Source/WebCore/platform/network/java/ResourceHandleJava.cpp
    ! modules/javafx.web/src/main/native/Source/WebCore/platform/network/java/URLLoader.cpp
    ! modules/javafx.web/src/main/native/Source/WebCore/platform/network/java/URLLoader.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.