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

8278980: Update WebKit to 613.1 #723

Closed
wants to merge 2 commits into from

Conversation

arapte
Copy link
Member

@arapte arapte commented Feb 2, 2022

Update JavaFX WebKit to GTK WebKit 2.34 (613.1).

Verified the updated version build, tests run and sanity testing.
This does not cause any issues except a unit test failure IrresponsiveScriptTest.
It is recorded and ignored using JDK-8280421

/contributor add kevinrushforth
/contributor add aghaisas
/contributor add Jay Bhaskar jay.bhaskar@oracle.com


Progress

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

Issue

Reviewers

Contributors

  • Ajit Ghaisas <aghaisas@openjdk.org>
  • Jay Bhaskar <jay.bhaskar@oracle.com>
  • Kevin Rushforth <kcr@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jfx pull/723/head:pull/723
$ git checkout pull/723

Update a local copy of the PR:
$ git checkout pull/723
$ git pull https://git.openjdk.java.net/jfx pull/723/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 723

View PR using the GUI difftool:
$ git pr show -t 723

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jfx/pull/723.diff

@arapte
Copy link
Member Author

arapte commented Feb 2, 2022

/contributor add kevinrushforth aghaisas jaybhaskar

@bridgekeeper
Copy link

bridgekeeper bot commented Feb 2, 2022

👋 Welcome back arapte! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr Ready for review label Feb 2, 2022
@openjdk
Copy link

openjdk bot commented Feb 2, 2022

@arapte Could not parse kevinrushforth aghaisas jaybhaskar as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

  • /contributor add @openjdk-bot
  • /contributor add duke
  • /contributor add J. Duke <duke@openjdk.org>

@arapte
Copy link
Member Author

arapte commented Feb 2, 2022

/contributor add @kcr
/contributor add @aghaisas
/contributor add Jay Bhaskar jay.bhaskar@oracle.com

@openjdk
Copy link

openjdk bot commented Feb 2, 2022

@arapte Could not parse @kevinrushforth @aghaisas as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

  • /contributor add @openjdk-bot
  • /contributor add duke
  • /contributor add J. Duke <duke@openjdk.org>

@openjdk
Copy link

openjdk bot commented Feb 2, 2022

@arapte
Contributor Ajit Ghaisas <aghaisas@openjdk.org> successfully added.

@openjdk
Copy link

openjdk bot commented Feb 2, 2022

@arapte
Contributor Jay Bhaskar <jay.bhaskar@oracle.com> successfully added.

@mlbridge
Copy link

mlbridge bot commented Feb 2, 2022

Webrevs

@openjdk
Copy link

openjdk bot commented Feb 2, 2022

@arapte Could not parse kevinrushforth as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

  • /contributor add @openjdk-bot
  • /contributor add duke
  • /contributor add J. Duke <duke@openjdk.org>

@openjdk
Copy link

openjdk bot commented Feb 2, 2022

@arapte
Contributor Ajit Ghaisas <aghaisas@openjdk.org> successfully added.

@openjdk
Copy link

openjdk bot commented Feb 2, 2022

@arapte Could not parse Jay Bhaskar jay.bhaskar@oracle.com as a valid contributor.
Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

  • /contributor add @openjdk-bot
  • /contributor add duke
  • /contributor add J. Duke <duke@openjdk.org>

@arapte
Copy link
Member Author

arapte commented Feb 2, 2022

/contributor add kcr

@openjdk
Copy link

openjdk bot commented Feb 2, 2022

@arapte
Contributor Kevin Rushforth <kcr@openjdk.org> successfully added.

@kevinrushforth
Copy link
Member

/reviewers 3

@arapte arapte changed the title 8278980: Update to 613.1 version of WebKit 8278980: Update WebKit to 613.1 Feb 2, 2022
@openjdk
Copy link

openjdk bot commented Feb 2, 2022

@kevinrushforth
The number of required reviews for this PR is now set to 3 (with at least 1 of role reviewers).

@kevinrushforth
Copy link
Member

One other thing to note about this update is that WebKit 613.1 will not compile with Visual Studio 2017 -- it requires VS 2019. This is similar to the previous 612.1 update, JDK-8268849, where a follow-on fix, JDK-8270479, was needed. I filed a similar bug this time:

JDK-8278983: WebKit 613.1 build fails with Visual Studio 2017

but it doesn't seem so straight-forward. We are likely at the point where we need to require VS 2019 to build (I note that WebKit itself has required VS 2019 for about 1.5 years). In order to eventually backport this to JavaFX 11, we will need to solve a long-standing problem when jlinking the JavaFX jmods into a JDK that was built with an earlier version of the Visual Studio compiler than the one used to build JavaFX. I filed the following to track this:

JDK-8281089: JavaFX built with VS2019 and jlinked into JDK 11.x fails to start

@mlbridge
Copy link

mlbridge bot commented Feb 2, 2022

Mailing list message from Johan Vos on openjfx-dev:

Hi Kevin,

Thanks for filing [JDK-8281089]. I think this is the way forward indeed.
The longer we delay moving to VS 2019, the more problems we can expect.
I'll build/test the WebKit 613.1 as well.

- Johan

On Wed, Feb 2, 2022 at 1:49 PM Kevin Rushforth <kcr at openjdk.java.net> wrote:

@yososs
Copy link

yososs commented Feb 2, 2022

You may have noticed ...
The change file contains changes that are not related to the WebKit upgrade.
It looks like you're reverting to an older version.

@kevinrushforth
Copy link
Member

The change file contains changes that are not related to the WebKit upgrade.

Thanks for pointing this out! It looks like the patch was applied incorrectly.

@arapte please fix this.

8279013: ES2Pipeline fails to detect AMD vega20 graphics card
8277853: With Touch enabled devices scrollbar disappears and the table is scrolled to the beginning
8277122: SplitPane divider drag can hang the layout
@arapte
Copy link
Member Author

arapte commented Feb 2, 2022

You may have noticed ... The change file contains changes that are not related to the WebKit upgrade. It looks like you're reverting to an older version.

Thanks for pointing it out. It is corrected now.
Thanks @kevinrushforth for quick fix.

Copy link
Member

@kevinrushforth kevinrushforth left a comment

Choose a reason for hiding this comment

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

Testing complete on all platforms. Looks good.

Copy link
Collaborator

@johanvos johanvos left a comment

Choose a reason for hiding this comment

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

builds on all platforms, basic tests are ok. We should do an EA release soon after this is merged, so we can have more testers.

@aghaisas
Copy link
Collaborator

aghaisas commented Feb 4, 2022

I have tested on macOS and Windows. No new issue observed.

@openjdk
Copy link

openjdk bot commented Feb 4, 2022

@arapte This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8278980: Update WebKit to 613.1

Co-authored-by: Ajit Ghaisas <aghaisas@openjdk.org>
Co-authored-by: Jay Bhaskar <jay.bhaskar@oracle.com>
Co-authored-by: Kevin Rushforth <kcr@openjdk.org>
Reviewed-by: kcr, jvos, aghaisas

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 2 new commits pushed to the master branch:

  • 929e7c9: 8280951: Testbug: fix commented asserts in XXViewTest.test_rt_29650
  • 59cd8ec: 8251481: TableCell accessing row: NPE on auto-sizing

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready Ready to be integrated label Feb 4, 2022
@arapte
Copy link
Member Author

arapte commented Feb 4, 2022

/integrate

@openjdk
Copy link

openjdk bot commented Feb 4, 2022

Going to push as commit 6f28d91.
Since your change was applied there have been 2 commits pushed to the master branch:

  • 929e7c9: 8280951: Testbug: fix commented asserts in XXViewTest.test_rt_29650
  • 59cd8ec: 8251481: TableCell accessing row: NPE on auto-sizing

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Feb 4, 2022
@openjdk openjdk bot closed this Feb 4, 2022
@openjdk openjdk bot removed ready Ready to be integrated rfr Ready for review labels Feb 4, 2022
@openjdk
Copy link

openjdk bot commented Feb 4, 2022

@arapte Pushed as commit 6f28d91.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@arapte arapte deleted the webkit-update-613 branch May 10, 2022 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrated Pull request has been integrated
5 participants