Re: RFR: 8255940: localStorage is null after window.close() [v2]

2022-02-04 Thread Jay Bhaskar
On Fri, 28 Jan 2022 00:11:40 GMT, Kevin Rushforth  wrote:

>> Jay Bhaskar has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains two additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into PRLocalstorage
>>  - Window.close(), Fix for localstoarge
>
> modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java 
> line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights 
>> reserved.
> 
> Copyright should be a single year (2022)

done

> modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java 
> line 48:
> 
>> 46: final WebEngine webEngine = getEngine();
>> 47: webEngine.setJavaScriptEnabled(true);
>> 48: webEngine.setUserDataDirectory(new File("/tmp/java-store"));
> 
> You should not hard-code /tmp/. You can either use a local subdirectory in 
> the build dir (which some other tests do), or else you will need to use 
> something like `Files.createTempDirectory("webstorage")`. If you use the 
> latter, then you will need to worry about how to clean up after the test, so 
> the former seems better.

i think /tmp/java-store is ok, as it would not require cleanup

> modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java 
> line 70:
> 
>> 68: WebView view = getView();
>> 69: view.getEngine().executeScript("test_local_storage_set();");
>> 70: String s = (String) 
>> view.getEngine().executeScript("document.getElementById('key').innerText;");
> 
> This will work, but it might be cleaner to add a JavaScript `getLocalStorage` 
> method so you don't have to get it from a DOM element.

The method webEngine.executeScript("localStorage;")  returns JSObject and in to 
get data members we need to call js.getMember(...) , getMember is not available 
in our code base, so i used tring s = (String) 
view.getEngine().executeScript("document.getElementById('key').innerText;");

> modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java 
> line 80:
> 
>> 78: submit(() -> {
>> 79: WebView view = getView();
>> 80: view.getEngine().executeScript("delete_items();");
> 
> You need to set some items first before deleting them if you want it to be an 
> effective test of this case.

The test runs after testLocalStorageSet , so there would be items in 
localstorage

-

PR: https://git.openjdk.java.net/jfx/pull/703


Re: RFR: 8273336: Clicking a selected cell from a group of selected cells in a TableView clears the selected items list but remains selected

2022-02-04 Thread yosbits
On Fri, 7 Jan 2022 19:36:45 GMT, Jose Pereda  wrote:

> This PR adds a predicate to TableView and TreeTableView selection models 
> order to remove rows from the selection only when there are no selected cells 
> in that given row, when cell selection is enabled.
> 
> Two tests have been added as well, that fail without this PR and pass with it.

Why not use IntPredicate?

before

``` java
public static  void updateSelectedIndices(MultipleSelectionModelBase sm,
 ListChangeListener.Change> c, 
Predicate removeRowFilter) {


after

``` java
public static  void updateSelectedIndices(MultipleSelectionModelBase sm, 
ListChangeListener.Change> c, 
IntPredicate removeRowFilter) {


before
``` java
.map(TablePositionBase::getRow)


after
``` java
.mapToInt(TablePositionBase::getRow)

-

PR: https://git.openjdk.java.net/jfx/pull/709


Re: RFR: 8255940: localStorage is null after window.close() [v2]

2022-02-04 Thread Jay Bhaskar
> Issue: The current implementation of DOMWindow ::localStorage(..) returns 
> null pointer in case of page is being closed. 
> Fix: It should not return nullptr , as per the [w3c web storage 
> spec](https://www.w3.org/TR/2016/REC-webstorage-20160419/) 
> "User agents should expire data from the local storage areas only for 
> security reasons or when requested to do so by the user. User agents should 
> always avoid deleting data while a script that could access that data is 
> running."

Jay Bhaskar has updated the pull request with a new target base due to a merge 
or a rebase. The incremental webrev excludes the unrelated changes brought in 
by the merge/rebase. The pull request contains two additional commits since the 
last revision:

 - Merge branch 'openjdk:master' into PRLocalstorage
 - Window.close(), Fix for localstoarge

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/703/files
  - new: https://git.openjdk.java.net/jfx/pull/703/files/12f97feb..73299577

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=703&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=703&range=00-01

  Stats: 416909 lines in 6905 files changed: 234954 ins; 135768 del; 46187 mod
  Patch: https://git.openjdk.java.net/jfx/pull/703.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/703/head:pull/703

PR: https://git.openjdk.java.net/jfx/pull/703


Re: [jfx18] RFR: 8279345: Realign class docs of LightBase and subclasses [v5]

2022-02-04 Thread Kevin Rushforth
On Fri, 4 Feb 2022 18:26:57 GMT, Nir Lisker  wrote:

>> Now that the standard concrete light types were added, there is an 
>> opportunity to rearrange and rewrite some of the class docs. Here is a 
>> summary of the changes:
>> 
>> * Moved the explanations of attenuation and direction up to `LightBase` 
>> since different light types share characteristics. `LightBase` now contains 
>> a summary of its subtypes and all the explanations of the 
>> properties/characteristics of the lights divided into sections: Color, 
>> Scope, Direction, Attenuation.
>> * Each light type links to the relevant section in `LightBase` when it 
>> mentioned the properties it has.
>> * Added examples of real-world applications for each light type.
>> * Consolidated the writing style for all the subclasses.
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Changed description of ambient light

LGTM. I like your updated AmbientLight description.

-

Marked as reviewed by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/717


Re: RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming

2022-02-04 Thread yosbits
On Fri, 4 Feb 2022 11:24:48 GMT, Alexander Matveev  wrote:

> - Added support for fragmented MP4 with HEVC/H.265, H.264 and AAC. 
>  - Added support for elementary AAC streams without any container for audio 
> only streams.
>  - Added "aacparse" plugin from GStreamer. Required on Linux, since decoder 
> cannot handle AAC elementary streams directly. DirectShow decoder works 
> without it.
>  - DirectShow H.264 decoder on Windows and H.265/H.264 decoder on Linux will 
> be reloaded when fMP4 stream changes resolution. Dynamic format change did 
> not worked for these streams on Windows and Linux.

modules/javafx.media/src/main/java/com/sun/media/jfxmedia/locator/HLSConnectionHolder.java

I noticed that in this change, ArrayList is rewritten as ArrayList<>.
The BitSet rewrite needs some thought, so it might be a good idea to make it a 
separate issue.
In the file I pointed out, it is used in many more places than the one I 
pointed out.

-

PR: https://git.openjdk.java.net/jfx/pull/726


Re: [jfx18] RFR: 8279345: Realign class docs of LightBase and subclasses [v5]

2022-02-04 Thread Nir Lisker
On Thu, 27 Jan 2022 19:01:32 GMT, Kevin Rushforth  wrote:

>> I think the description should focus on the meaning of the respective term 
>> in the lighting equation, and not on a non-technical analogy. In this case, 
>> the analogy is a bit misleading on several aspects, including the fact that 
>> ambient lighting is not dependent on an area being enclosed. Here's a 
>> suggestion:
>> 
>> Ambient lights add a constant term to the amount of light reflected by each 
>> point on
>> the surface of an object, thereby increasing the brightness of the object 
>> uniformly and
>> independently of the orientation of its surfaces. It is often used to 
>> represent the base
>> amount of illumination in a scene.
>
> I think the key aspects of an Ambient light are that: the light seems to come 
> from all directions (and thus has no position or direction), and that it 
> illuminates objects independently of the position or orientation of the 
> object.
> 
> I don't have a problem with also giving a real-world analogy if one can be 
> found that makes it easier to understand without causing confusion.

I tried to unify some of the points that were brought up. See if you would like 
more changes.

-

PR: https://git.openjdk.java.net/jfx/pull/717


Re: [jfx18] RFR: 8279345: Realign class docs of LightBase and subclasses [v5]

2022-02-04 Thread Nir Lisker
> Now that the standard concrete light types were added, there is an 
> opportunity to rearrange and rewrite some of the class docs. Here is a 
> summary of the changes:
> 
> * Moved the explanations of attenuation and direction up to `LightBase` since 
> different light types share characteristics. `LightBase` now contains a 
> summary of its subtypes and all the explanations of the 
> properties/characteristics of the lights divided into sections: Color, Scope, 
> Direction, Attenuation.
> * Each light type links to the relevant section in `LightBase` when it 
> mentioned the properties it has.
> * Added examples of real-world applications for each light type.
> * Consolidated the writing style for all the subclasses.

Nir Lisker has updated the pull request incrementally with one additional 
commit since the last revision:

  Changed description of ambient light

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/717/files
  - new: https://git.openjdk.java.net/jfx/pull/717/files/cae874d6..987a6165

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=717&range=04
 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=717&range=03-04

  Stats: 6 lines in 1 file changed: 2 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jfx/pull/717.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/717/head:pull/717

PR: https://git.openjdk.java.net/jfx/pull/717


Re: RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming

2022-02-04 Thread Kevin Rushforth
On Fri, 4 Feb 2022 17:19:36 GMT, yosbits  wrote:

> Why not use BitSet instead of ArrayList?

Can you be more specific? The only use of `ArrayList` that I see in the patch 
is in code that uses a few already-existing lists. Changing that to some other 
data structure would be an unrelated change that seems out of scope for this 
fix.

-

PR: https://git.openjdk.java.net/jfx/pull/726


Re: RFR: 8273336: Clicking a selected cell from a group of selected cells in a TableView clears the selected items list but remains selected

2022-02-04 Thread Michael Strauß
On Fri, 7 Jan 2022 19:36:45 GMT, Jose Pereda  wrote:

> This PR adds a predicate to TableView and TreeTableView selection models 
> order to remove rows from the selection only when there are no selected cells 
> in that given row, when cell selection is enabled.
> 
> Two tests have been added as well, that fail without this PR and pass with it.

modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java 
line 172:

> 170: .map(TablePositionBase::getRow)
> 171: .filter(removeRowFilter)
> 172: .distinct()

Maybe `distinct` should be applied before `filter`. This can cut down the 
number of times the predicate is invoked (which iterates over all selected 
cells, so it may be a performance issue for large selections).

-

PR: https://git.openjdk.java.net/jfx/pull/709


Re: RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming

2022-02-04 Thread yosbits
On Fri, 4 Feb 2022 11:24:48 GMT, Alexander Matveev  wrote:

> - Added support for fragmented MP4 with HEVC/H.265, H.264 and AAC. 
>  - Added support for elementary AAC streams without any container for audio 
> only streams.
>  - Added "aacparse" plugin from GStreamer. Required on Linux, since decoder 
> cannot handle AAC elementary streams directly. DirectShow decoder works 
> without it.
>  - DirectShow H.264 decoder on Windows and H.265/H.264 decoder on Linux will 
> be reloaded when fMP4 stream changes resolution. Dynamic format change did 
> not worked for these streams on Windows and Linux.

Why not use BitSet instead of ArrayList?

-

PR: https://git.openjdk.java.net/jfx/pull/726


Integrated: 8278980: Update WebKit to 613.1

2022-02-04 Thread Ambarish Rapte
On Wed, 2 Feb 2022 07:34:55 GMT, Ambarish Rapte  wrote:

> 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](https://bugs.openjdk.java.net/browse/JDK-8280421)

This pull request has now been integrated.

Changeset: 6f28d912
Author:Ambarish Rapte 
URL:   
https://git.openjdk.java.net/jfx/commit/6f28d912024495278c4c35ab054bc2aab480b3e4
Stats: 412962 lines in 6737 files changed: 231442 ins; 135635 del; 45885 mod

8278980: Update WebKit to 613.1

Co-authored-by: Ajit Ghaisas 
Co-authored-by: Jay Bhaskar 
Co-authored-by: Kevin Rushforth 
Reviewed-by: kcr, jvos, aghaisas

-

PR: https://git.openjdk.java.net/jfx/pull/723


RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming

2022-02-04 Thread Alexander Matveev
- Added support for fragmented MP4 with HEVC/H.265, H.264 and AAC. 
 - Added support for elementary AAC streams without any container for audio 
only streams.
 - Added "aacparse" plugin from GStreamer. Required on Linux, since decoder 
cannot handle AAC elementary streams directly. DirectShow decoder works without 
it.
 - DirectShow H.264 decoder on Windows and H.265/H.264 decoder on Linux will be 
reloaded when fMP4 stream changes resolution. Dynamic format change did not 
worked for these streams on Windows and Linux.

-

Commit messages:
 - 8277309: Add support for H.265/HEVC to HTTP Live Streaming

Changes: https://git.openjdk.java.net/jfx/pull/726/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=726&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8277309
  Stats: 2637 lines in 27 files changed: 2343 ins; 126 del; 168 mod
  Patch: https://git.openjdk.java.net/jfx/pull/726.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/726/head:pull/726

PR: https://git.openjdk.java.net/jfx/pull/726


Re: RFR: 8278980: Update WebKit to 613.1 [v2]

2022-02-04 Thread Ajit Ghaisas
On Wed, 2 Feb 2022 15:29:51 GMT, Ambarish Rapte  wrote:

>> 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](https://bugs.openjdk.java.net/browse/JDK-8280421)
>
> Ambarish Rapte has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Restore commits that were mistakenly reverted.
>   
>   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

Marked as reviewed by aghaisas (Reviewer).

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

-

PR: https://git.openjdk.java.net/jfx/pull/723


Re: RFR: 8278980: Update WebKit to 613.1 [v2]

2022-02-04 Thread Johan Vos
On Wed, 2 Feb 2022 15:29:51 GMT, Ambarish Rapte  wrote:

>> 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](https://bugs.openjdk.java.net/browse/JDK-8280421)
>
> Ambarish Rapte has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Restore commits that were mistakenly reverted.
>   
>   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

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.

-

Marked as reviewed by jvos (Reviewer).

PR: https://git.openjdk.java.net/jfx/pull/723