From 5bf883a456cee04819ec2bf59c324016076fc7c4 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 10 Feb 2021 18:14:24 -0800 Subject: [PATCH] fix: move exposed binding and funcion callbacks to top level (#278) --- .../microsoft/playwright/BrowserContext.java | 6 +-- .../java/com/microsoft/playwright/Page.java | 30 ++---------- .../playwright/impl/BindingCall.java | 7 +-- .../playwright/impl/BrowserContextImpl.java | 12 ++--- .../microsoft/playwright/impl/PageImpl.java | 48 +++++++------------ .../playwright/options/BindingCallback.java | 31 ++++++++++++ .../playwright/options/FunctionCallback.java | 21 ++++++++ .../TestBrowserContextExposeFunction.java | 3 +- .../playwright/TestPageExposeFunction.java | 3 +- .../com/microsoft/playwright/TestWorkers.java | 4 +- .../playwright/tools/ApiGenerator.java | 24 ---------- .../com/microsoft/playwright/tools/Types.java | 8 ++-- 12 files changed, 96 insertions(+), 101 deletions(-) create mode 100644 playwright/src/main/java/com/microsoft/playwright/options/BindingCallback.java create mode 100644 playwright/src/main/java/com/microsoft/playwright/options/FunctionCallback.java diff --git a/playwright/src/main/java/com/microsoft/playwright/BrowserContext.java b/playwright/src/main/java/com/microsoft/playwright/BrowserContext.java index 3d001291..88721cea 100644 --- a/playwright/src/main/java/com/microsoft/playwright/BrowserContext.java +++ b/playwright/src/main/java/com/microsoft/playwright/BrowserContext.java @@ -144,7 +144,7 @@ public interface BrowserContext extends AutoCloseable { * @param urls Optional list of URLs. */ List cookies(List urls); - default void exposeBinding(String name, Page.Binding callback) { + default void exposeBinding(String name, BindingCallback callback) { exposeBinding(name, callback, null); } /** @@ -161,7 +161,7 @@ public interface BrowserContext extends AutoCloseable { * @param name Name of the function on the window object. * @param callback Callback function that will be called in the Playwright's context. */ - void exposeBinding(String name, Page.Binding callback, ExposeBindingOptions options); + void exposeBinding(String name, BindingCallback callback, ExposeBindingOptions options); /** * The method adds a function called {@code name} on the {@code window} object of every frame in every page in the context. When * called, the function executes {@code callback} and returns a [Promise] which resolves to the return value of {@code callback}. @@ -174,7 +174,7 @@ public interface BrowserContext extends AutoCloseable { * @param name Name of the function on the window object. * @param callback Callback function that will be called in the Playwright's context. */ - void exposeFunction(String name, Page.Function callback); + void exposeFunction(String name, FunctionCallback callback); default void grantPermissions(List permissions) { grantPermissions(permissions, null); } diff --git a/playwright/src/main/java/com/microsoft/playwright/Page.java b/playwright/src/main/java/com/microsoft/playwright/Page.java index 8e881df7..c76ca777 100644 --- a/playwright/src/main/java/com/microsoft/playwright/Page.java +++ b/playwright/src/main/java/com/microsoft/playwright/Page.java @@ -37,26 +37,6 @@ import java.util.regex.Pattern; *

To unsubscribe from events use the {@code removeListener} method: */ public interface Page extends AutoCloseable { - interface Function { - Object call(Object... args); - } - - interface Binding { - interface Source { - BrowserContext context(); - Page page(); - Frame frame(); - } - - Object call(Source source, Object... args); - } - - interface Error { - String message(); - String name(); - String stack(); - } - void onClose(Consumer handler); void offClose(Consumer handler); @@ -91,8 +71,8 @@ public interface Page extends AutoCloseable { void onLoad(Consumer handler); void offLoad(Consumer handler); - void onPageError(Consumer handler); - void offPageError(Consumer handler); + void onPageError(Consumer handler); + void offPageError(Consumer handler); void onPopup(Consumer handler); void offPopup(Consumer handler); @@ -1612,7 +1592,7 @@ public interface Page extends AutoCloseable { * @param arg Optional argument to pass to {@code expression}. */ JSHandle evaluateHandle(String expression, Object arg); - default void exposeBinding(String name, Binding callback) { + default void exposeBinding(String name, BindingCallback callback) { exposeBinding(name, callback, null); } /** @@ -1631,7 +1611,7 @@ public interface Page extends AutoCloseable { * @param name Name of the function on the window object. * @param callback Callback function that will be called in the Playwright's context. */ - void exposeBinding(String name, Binding callback, ExposeBindingOptions options); + void exposeBinding(String name, BindingCallback callback, ExposeBindingOptions options); /** * The method adds a function called {@code name} on the {@code window} object of every frame in the page. When called, the function * executes {@code callback} and returns a [Promise] which resolves to the return value of {@code callback}. @@ -1646,7 +1626,7 @@ public interface Page extends AutoCloseable { * @param name Name of the function on the window object * @param callback Callback function which will be called in Playwright's context. */ - void exposeFunction(String name, Function callback); + void exposeFunction(String name, FunctionCallback callback); default void fill(String selector, String value) { fill(selector, value, null); } diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/BindingCall.java b/playwright/src/main/java/com/microsoft/playwright/impl/BindingCall.java index 71e961b0..5ed13379 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/BindingCall.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/BindingCall.java @@ -22,6 +22,7 @@ import com.microsoft.playwright.BrowserContext; import com.microsoft.playwright.Frame; import com.microsoft.playwright.JSHandle; import com.microsoft.playwright.Page; +import com.microsoft.playwright.options.BindingCallback; import java.util.ArrayList; import java.util.List; @@ -29,7 +30,7 @@ import java.util.List; import static com.microsoft.playwright.impl.Serialization.*; class BindingCall extends ChannelOwner { - private static class SourceImpl implements Page.Binding.Source { + private static class SourceImpl implements BindingCallback.Source { private final Frame frame; public SourceImpl(Frame frame) { @@ -60,10 +61,10 @@ class BindingCall extends ChannelOwner { return initializer.get("name").getAsString(); } - void call(Page.Binding binding) { + void call(BindingCallback binding) { try { Frame frame = connection.getExistingObject(initializer.getAsJsonObject("frame").get("guid").getAsString()); - Page.Binding.Source source = new SourceImpl(frame); + BindingCallback.Source source = new SourceImpl(frame); List args = new ArrayList<>(); if (initializer.has("handle")) { JSHandle handle = connection.getExistingObject(initializer.getAsJsonObject("handle").get("guid").getAsString()); diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java index 3ee6c365..2684c968 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java @@ -38,7 +38,7 @@ class BrowserContextImpl extends ChannelOwner implements BrowserContext { final List pages = new ArrayList<>(); final Router routes = new Router(); private boolean isClosedOrClosing; - final Map bindings = new HashMap<>(); + final Map bindings = new HashMap<>(); PageImpl ownerPage; private final ListenerCollection listeners = new ListenerCollection<>(); final TimeoutSettings timeoutSettings = new TimeoutSettings(); @@ -167,11 +167,11 @@ class BrowserContextImpl extends ChannelOwner implements BrowserContext { } @Override - public void exposeBinding(String name, Page.Binding playwrightBinding, ExposeBindingOptions options) { + public void exposeBinding(String name, BindingCallback playwrightBinding, ExposeBindingOptions options) { withLogging("BrowserContext.exposeBinding", () -> exposeBindingImpl(name, playwrightBinding, options)); } - private void exposeBindingImpl(String name, Page.Binding playwrightBinding, ExposeBindingOptions options) { + private void exposeBindingImpl(String name, BindingCallback playwrightBinding, ExposeBindingOptions options) { if (bindings.containsKey(name)) { throw new PlaywrightException("Function \"" + name + "\" has been already registered"); } @@ -191,9 +191,9 @@ class BrowserContextImpl extends ChannelOwner implements BrowserContext { } @Override - public void exposeFunction(String name, Page.Function playwrightFunction) { + public void exposeFunction(String name, FunctionCallback playwrightFunction) { withLogging("BrowserContext.exposeFunction", - () -> exposeBindingImpl(name, (Page.Binding.Source source, Object... args) -> playwrightFunction.call(args), null)); + () -> exposeBindingImpl(name, (BindingCallback.Source source, Object... args) -> playwrightFunction.call(args), null)); } @Override @@ -380,7 +380,7 @@ class BrowserContextImpl extends ChannelOwner implements BrowserContext { pages.add(page); } else if ("bindingCall".equals(event)) { BindingCall bindingCall = connection.getExistingObject(params.getAsJsonObject("binding").get("guid").getAsString()); - Page.Binding binding = bindings.get(bindingCall.name()); + BindingCallback binding = bindings.get(bindingCall.name()); if (binding != null) { bindingCall.call(binding); } diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java index 42d3fa09..e5cd46fe 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java @@ -62,7 +62,7 @@ public class PageImpl extends ChannelOwner implements Page { } } }; - final Map bindings = new HashMap<>(); + final Map bindings = new HashMap<>(); BrowserContextImpl ownedContext; private boolean isClosed; final Set workers = new HashSet<>(); @@ -148,7 +148,7 @@ public class PageImpl extends ChannelOwner implements Page { } else if ("bindingCall".equals(event)) { String guid = params.getAsJsonObject("binding").get("guid").getAsString(); BindingCall bindingCall = connection.getExistingObject(guid); - Binding binding = bindings.get(bindingCall.name()); + BindingCallback binding = bindings.get(bindingCall.name()); if (binding == null) { binding = browserContext.bindings.get(bindingCall.name()); } @@ -213,7 +213,14 @@ public class PageImpl extends ChannelOwner implements Page { video().setRelativePath(params.get("relativePath").getAsString()); } else if ("pageError".equals(event)) { SerializedError error = gson().fromJson(params.getAsJsonObject("error"), SerializedError.class); - listeners.notify(EventType.PAGEERROR, new ErrorImpl(error)); + String errorStr = ""; + if (error.error != null) { + errorStr = error.error.name + ": " + error.error.message; + if (error.error.stack != null && !error.error.stack.isEmpty()) { + errorStr += "\n" + error.error.stack; + } + } + listeners.notify(EventType.PAGEERROR, errorStr); } else if ("crash".equals(event)) { listeners.notify(EventType.CRASH, this); } else if ("close".equals(event)) { @@ -352,12 +359,12 @@ public class PageImpl extends ChannelOwner implements Page { } @Override - public void onPageError(Consumer handler) { + public void onPageError(Consumer handler) { listeners.add(EventType.PAGEERROR, handler); } @Override - public void offPageError(Consumer handler) { + public void offPageError(Consumer handler) { listeners.remove(EventType.PAGEERROR, handler); } @@ -623,11 +630,11 @@ public class PageImpl extends ChannelOwner implements Page { } @Override - public void exposeBinding(String name, Binding playwrightBinding, ExposeBindingOptions options) { + public void exposeBinding(String name, BindingCallback playwrightBinding, ExposeBindingOptions options) { withLogging("Page.exposeBinding", () -> exposeBindingImpl(name, playwrightBinding, options)); } - private void exposeBindingImpl(String name, Binding playwrightBinding, ExposeBindingOptions options) { + private void exposeBindingImpl(String name, BindingCallback playwrightBinding, ExposeBindingOptions options) { if (bindings.containsKey(name)) { throw new PlaywrightException("Function \"" + name + "\" has been already registered"); } @@ -645,9 +652,9 @@ public class PageImpl extends ChannelOwner implements Page { } @Override - public void exposeFunction(String name, Function playwrightFunction) { + public void exposeFunction(String name, FunctionCallback playwrightFunction) { withLogging("Page.exposeFunction", - () -> exposeBindingImpl(name, (Binding.Source source, Object... args) -> playwrightFunction.call(args), null)); + () -> exposeBindingImpl(name, (BindingCallback.Source source, Object... args) -> playwrightFunction.call(args), null)); } @Override @@ -1146,29 +1153,6 @@ public class PageImpl extends ChannelOwner implements Page { listeners.notify(EventType.FRAMENAVIGATED, frame); } - private static class ErrorImpl implements Error { - private final SerializedError error; - - ErrorImpl(SerializedError error) { - this.error = error; - } - - @Override - public String message() { - return error.error.message; - } - - @Override - public String name() { - return error.error.name; - } - - @Override - public String stack() { - return error.error.stack; - } - } - private class WaitableFrameDetach extends WaitableEvent { WaitableFrameDetach(Frame frameArg) { super(PageImpl.this.listeners, EventType.FRAMEDETACHED, detachedFrame -> frameArg.equals(detachedFrame)); diff --git a/playwright/src/main/java/com/microsoft/playwright/options/BindingCallback.java b/playwright/src/main/java/com/microsoft/playwright/options/BindingCallback.java new file mode 100644 index 00000000..bd7f6583 --- /dev/null +++ b/playwright/src/main/java/com/microsoft/playwright/options/BindingCallback.java @@ -0,0 +1,31 @@ +/* + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.microsoft.playwright.options; + +import com.microsoft.playwright.BrowserContext; +import com.microsoft.playwright.Frame; +import com.microsoft.playwright.Page; + +public interface BindingCallback { + interface Source { + BrowserContext context(); + Page page(); + Frame frame(); + } + + Object call(Source source, Object... args); +} diff --git a/playwright/src/main/java/com/microsoft/playwright/options/FunctionCallback.java b/playwright/src/main/java/com/microsoft/playwright/options/FunctionCallback.java new file mode 100644 index 00000000..f9f6f627 --- /dev/null +++ b/playwright/src/main/java/com/microsoft/playwright/options/FunctionCallback.java @@ -0,0 +1,21 @@ +/* + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.microsoft.playwright.options; + +public interface FunctionCallback { + Object call(Object... args); +} diff --git a/playwright/src/test/java/com/microsoft/playwright/TestBrowserContextExposeFunction.java b/playwright/src/test/java/com/microsoft/playwright/TestBrowserContextExposeFunction.java index 72c5d842..ce9dcd22 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestBrowserContextExposeFunction.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestBrowserContextExposeFunction.java @@ -16,6 +16,7 @@ package com.microsoft.playwright; +import com.microsoft.playwright.options.BindingCallback; import org.junit.jupiter.api.Test; import java.util.ArrayList; @@ -29,7 +30,7 @@ public class TestBrowserContextExposeFunction extends TestBase { @Test void exposeBindingShouldWork() { - Page.Binding.Source[] bindingSource = {null}; + BindingCallback.Source[] bindingSource = {null}; context.exposeBinding("add", (source, args) -> { bindingSource[0] = source; return (Integer) args[0] + (Integer) args[1]; diff --git a/playwright/src/test/java/com/microsoft/playwright/TestPageExposeFunction.java b/playwright/src/test/java/com/microsoft/playwright/TestPageExposeFunction.java index f7bd0211..0e06a349 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestPageExposeFunction.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestPageExposeFunction.java @@ -16,6 +16,7 @@ package com.microsoft.playwright; +import com.microsoft.playwright.options.BindingCallback; import org.junit.jupiter.api.Test; import java.util.Map; @@ -31,7 +32,7 @@ public class TestPageExposeFunction extends TestBase { void exposeBindingShouldWork() { BrowserContext context = browser.newContext(); Page page = context.newPage(); - Page.Binding.Source[] bindingSource = { null }; + BindingCallback.Source[] bindingSource = { null }; page.exposeBinding("add", (source, args) -> { bindingSource[0] = source; return (Integer) args[0] + (Integer) args[1]; diff --git a/playwright/src/test/java/com/microsoft/playwright/TestWorkers.java b/playwright/src/test/java/com/microsoft/playwright/TestWorkers.java index c8bbb5f0..9d43078c 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestWorkers.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestWorkers.java @@ -81,7 +81,7 @@ public class TestWorkers extends TestBase { @Test void shouldReportErrors() { - Page.Error[] errorLog = {null}; + String[] errorLog = {null}; page.onPageError(e -> errorLog[0] = e); page.evaluate("() => new Worker(URL.createObjectURL(new Blob([`\n" + " setTimeout(() => {\n" + @@ -95,7 +95,7 @@ public class TestWorkers extends TestBase { page.waitForTimeout(100); assertTrue(Duration.between(start, Instant.now()).getSeconds() < 30, "Timed out"); } - assertTrue(errorLog[0].message().contains("this is my error")); + assertTrue(errorLog[0].contains("this is my error")); } @Test diff --git a/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java b/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java index 73e23b9a..8acff042 100644 --- a/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java +++ b/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java @@ -954,30 +954,6 @@ class Interface extends TypeDefinition { private void writeSharedTypes(List output, String offset) { switch (jsonName) { - case "Page": { - output.add(offset + "interface Function {"); - output.add(offset + " Object call(Object... args);"); - output.add(offset + "}"); - output.add(""); - - output.add(offset + "interface Binding {"); - output.add(offset + " interface Source {"); - output.add(offset + " BrowserContext context();"); - output.add(offset + " Page page();"); - output.add(offset + " Frame frame();"); - output.add(offset + " }"); - output.add(""); - output.add(offset + " Object call(Source source, Object... args);"); - output.add(offset + "}"); - output.add(""); - output.add(offset + "interface Error {"); - output.add(offset + " String message();"); - output.add(offset + " String name();"); - output.add(offset + " String stack();"); - output.add(offset + "}"); - output.add(""); - break; - } case "ElementHandle": { output.add(offset + "class SelectOption {"); output.add(offset + " public String value;"); diff --git a/tools/api-generator/src/main/java/com/microsoft/playwright/tools/Types.java b/tools/api-generator/src/main/java/com/microsoft/playwright/tools/Types.java index 5b6eec03..646d2b88 100644 --- a/tools/api-generator/src/main/java/com/microsoft/playwright/tools/Types.java +++ b/tools/api-generator/src/main/java/com/microsoft/playwright/tools/Types.java @@ -46,10 +46,10 @@ class Types { Types() { // Viewport size. - add("BrowserContext.exposeBinding.callback", "function", "Page.Binding"); - add("BrowserContext.exposeFunction.callback", "function", "Page.Function"); - add("Page.exposeBinding.callback", "function", "Binding"); - add("Page.exposeFunction.callback", "function", "Function"); + add("BrowserContext.exposeBinding.callback", "function", "BindingCallback"); + add("BrowserContext.exposeFunction.callback", "function", "FunctionCallback"); + add("Page.exposeBinding.callback", "function", "BindingCallback"); + add("Page.exposeFunction.callback", "function", "FunctionCallback"); add("BrowserContext.addInitScript.script", "Object|function|string", "String"); add("Page.addInitScript.script", "Object|function|string", "String");