chromiumembedded / cef

Chromium Embedded Framework (CEF). A simple framework for embedding Chromium-based browsers in other applications.
https://bitbucket.org/chromiumembedded/cef/
Other
3.32k stars 464 forks source link

Proposal: Improve C API design #2999

Open magreenblatt opened 4 years ago

magreenblatt commented 4 years ago

Original report by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


Who is ideologue (API architect) of integration CEF with external apps that use libcef.dll? I am the qualified IT engineer with over up 10 years’ experience, and such ineffective and not unified API like used in CEF I do not seen yet.

1. Objects wrappers for passing to / accepting from external apps use closures on instance:

struct CefWrapper {
  struct CefBase {
    size_t size;
    // inline embed array of callbacks (virtual invokes)
    result_t (stdcall *) (CefBase *, …) callback_1;
    // . . .
    result_t (stdcall *) (CefBase *, …) callback_M;
  } base;
  // inline embed array of callbacks (virtual invokes)
  result_t (stdcall *) (CefWrapper *, …) callback_1;
  // . . .
  result_t (stdcall *) (CefWrapper *, …) callback_N;
} * wrapper;

The initialization of such wrapper requires filling all slots of inline embed array (VMT) for each instance (initialize array each time when create new instance wrapper), which is very expensive for CPU time and linking RVA.

The standard concept of using virtual methods table are follows:

struct Instance {
  // pointer to static array of callbacks (virtual invokes)
  result_t (stdcall *) (Instance *, …) vmt[M + N + …];
  // instance data fields below
} * instance;

The initialization of instance requires only single linking Instance::vmt member with const static array of callbacks (virtual invokes), contents of which may be read-only in addition (placed in CODE segment or in segment with R/O data).

The using standard concept of VMT is preferred, because the custom apps that will use libcef.dll may operate with instances of abstract classes or with interfaces (IInterface, IUnknown) depending of program's language features.

Also not all programs language supports a records / structs finalization and customizable operators for copying, like C++, thus the implementation of CefRefPtr features allows only via interfaces.

2. Several Strings API are used in CEF:

The shared library for using in custom apps must use single unified String type with static API, like below:

StringA * AllocStrA(AnsiChar * content, unsigned length);
StringW * AllocStrW(WideChar * content, unsigned length);

void FreeStrA(StringA * handle);
void FreeStrW(StringW * handle);

StringA * ReplaceStrA(StringA * handle, AnsiChar * content, unsigned length);
StringW * ReplaceStrW(StringW * handle, WideChar * content, unsigned length);

When String object must contain some information about text length and here reference count, this data prepends before a handle of value, usually.

#define StrLength(handle) (handle ? reinterpret_cast<unsigned *>(handle)[-1] : 0)
#define StrRefCount(handle) (handle ? reinterpret_cast<int *>(handle)[-2] : -1)

3. The calling convention must be unified for any kind of API of shared library: callbacks, virtual methods, static imports. Usually stdcall is used, no any language specifics (cdecl, pascal).

magreenblatt commented 4 years ago

Thanks for the feedback. The current API model has been in place for 11 years and is used by many language wrappers, but there is always room for improvement. What API design guide or existing library are you referencing with your suggestions?

magreenblatt commented 4 years ago

The calling convention must be unified for any kind of API of shared library

__stdcall is used on Windows via CEF_CALLBACK. Are you looking at the C API specifically? I guess it’s possible that calling conversions are mixed (definition of CEF_EXPORT vs CEF_CALLBACK on different platforms), but I’m not aware of this being a serious issue.

Is there something currently blocking you from using the existing C API via libcef or C++ API via libcef_dll_wrapper?

magreenblatt commented 4 years ago

For clarity, let’s look at a concrete example from cef_base_capi.h and cef_app_capi.h. The contents are currently:

typedef struct _cef_base_ref_counted_t {
  size_t size;

  void(CEF_CALLBACK* add_ref)(struct _cef_base_ref_counted_t* self);
  int(CEF_CALLBACK* release)(struct _cef_base_ref_counted_t* self);
} cef_base_ref_counted_t;

typedef struct _cef_app_t {
  cef_base_ref_counted_t base;

  void(CEF_CALLBACK* on_before_command_line_processing)(
      struct _cef_app_t* self,
      const cef_string_t* process_type,
      struct _cef_command_line_t* command_line);

  void(CEF_CALLBACK* on_register_custom_schemes)(
      struct _cef_app_t* self,
      struct _cef_scheme_registrar_t* registrar);

  struct _cef_resource_bundle_handler_t*(
      CEF_CALLBACK* get_resource_bundle_handler)(struct _cef_app_t* self);
} cef_app_t;

CEF_EXPORT int cef_execute_process(const struct _cef_main_args_t* args,
                                   cef_app_t* application,
                                   void* windows_sandbox_info);

How would this be rewritten in your suggested new format?

magreenblatt commented 4 years ago

See also https://www.magpcss.org/ceforum/viewtopic.php?f=10&t=10818 (number 11) which is related, and discusses a better way to define interfaces implemented by libcef. Each C++ class method would become a separate exported function that accepts an opaque handle as the first argument. For example:

typedef void* cef_browser_handle_t;
cef_browser_handle_t cef_browser_create(...);
void cef_browser_reload(cef_browser_handle_t handle, ...);
cef_browser_addref(cef_browser_handle_t handle);
cef_browser_release(cef_browser_handle_t handle);

One advantage to this approach is that backwards/forwards binary compatibility could be maintained by exporting new/additional functions with different names, instead of changing the signature of existing exported functions.

There are a number of different vtable-based strategies that could be used for interfaces implemented by the client (like cef_app_t above), which is why I’m interested in a concrete example of what your specific strategy would look like. We can then discuss specific pros/cons versus other strategies.

magreenblatt commented 4 years ago

The shared library for using in custom apps must use single unified String type with static API

If I’m understanding correctly, then I don’t agree with this point. Internally, CEF/Chromium has utf8 and utf16 strings, “wide” is defined differently on Windows (16-bit) vs Linux (32-bit), and Unicode conversions of large strings can be expensive. For most CEF consumers, they don’t want to care about utf8 vs utf16 and just convert implicitly to std::string (which is utf8). Ideally, we should export the underlying CEF/Chromium type (utf8 or utf16) without Unicode conversion for maximum efficiency, and then provide helpers for the client to implicitly convert to the desired type if necessary.

magreenblatt commented 4 years ago

user-free strings returning by handle

What alternative API are you proposing for this? Are you suggesting that we always return a StringA* (from your example) and the client then passes that string to FreeStrA? If so, we would no longer be able return static strings (which is currently done using a null dtor), and we would have extra/unnecessary copies.

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


CefApi.h

#define CEF_API __stdcall

CefBase.h

typedef struct CefBase {
  virtual void CEF_API AddRef() = 0;
  virtual bool CEF_API Release() = 0;
  virtual bool CEF_API HasOneRef() = 0;
  virtual bool CEF_API HasAtLeastOneRef() = 0;
} * ICefBase;

CefStrings.h

#ifdef UNICODE
typedef char16_t CefChar;
#else
typedef char CefChar;
#endif

typedef CefChar * CefString;

#define CefStrLength(handle) (handle ? reinterpret_cast<unsigned *>(handle)[-1] : 0)
#define CefStrRefCount(handle) (handle ? reinterpret_cast<int *>(handle)[-2] : -1)

extern "C" {
  CefString CEF_API CefAllocStr(const CefChar * content, unsigned length);
  bool CEF_API CefFreeStr(CefString handle);
  bool CEF_API CefReAllocStr(CefString & handle, const CefChar * content, unsigned length);
}

struct CefStringValue {
  const CefString Handle;
  CefStringValue(const CefChar * content, const unsigned length) :
    Handle(CefAllocStr(content, length)) {
  }
  ~CefStringValue() {
    if (Handle)
      CefFreeStr(Handle);
  }
  unsigned GetLength() const {
    return CefStrLength(Handle);
  }
  unsigned GetRefCount() const {
    return CefStrRefCount(Handle);
  }
  bool Replace(const CefChar * content, const unsigned length) {
    return CefReAllocStr(const_cast<CefString&>(Handle), content, length);
  }
};

CefTypes.h

struct CefMainArgs {
  // ...
};

CefCommandLine.h

typedef struct CefCommandLine : CefBase {
  // ...
} * ICefCommandLine;

CefScheme.h

typedef struct CefSchemeRegistrar : CefBase {
  // ...  
} * ICefSchemeRegistrar;

CefResourceBundleHandler.h

typedef struct CefResourceBundleHandler : CefBase {
  // ...
} * ICefResourceBundleHandler;

CefApp.h

typedef struct CefApp : CefBase {
  virtual void CEF_API OnBeforeCommandLineProcessing(CefString process, ICefCommandLine command) = 0;
  virtual void CEF_API OnRegisterCustomSchemes(ICefSchemeRegistrar registrar) = 0;
  virtual ICefResourceBundleHandler CEF_API GetResourceBundleHandler() = 0;
}* ICefApp;

extern "C" bool CEF_API CefExecuteProcess(const CefMainArgs * args, ICefApp app = nullptr, void * sandbox = nullptr);

In struct cef_string_t the calling for dtor() does not defined calling convention, so __cdecl is used by default, but this member is callback!

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


The CefBase class contains redundant API (HasOneRef, HasAtLeastOneRef). The best way is make this class compatible to standard IUnknown, like below.

typedef struct CefObject {
  // param id may be any identifier (not GUID certain)
  // return S_OK (0) on success, E_XXX (< 0) in failure
  virtual int CEF_API QueryAs(const char * id, CefObject*& target) = 0;
  // return actual reference count
  virtual long CEF_API AddRef() = 0;
  // return 0 when instance released or the count of references used by other owners
  virtual long CEF_API Release() = 0;
} * ICefObject;

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


The some CEF objects contain the API is_same(), is_equal() that accept input argument same type object. When we pass some instance to that methods, the add_ref() must be applied to passed instance before. It is bad practice, because by standard all input referenced instances are used as constants, so such requirement is not used in caller side.

magreenblatt commented 4 years ago

The struct examples that you posted are C++, not C. Please answer the questions in this comment and this comment.

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


Expanding all CEF API to static export with typed handles for each instance type will require a huge DLL export section, so will follow a long time when libcef.dll dynamically linking to master process.

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


The no anybody who use pure C now without classes with virtual method. But my example may be implemented in pure C for emulation VMT like below.

struct CefBaseForPureC {
  const struct Vmt {
    void (CEF_API * AddRef) (CefBaseForPureC * self);
    bool (CEF_API * Release) (CefBaseForPureC * self);
    bool (CEF_API * HasOneRef) (CefBaseForPureC * self);
    bool (CEF_API * HasAtLeastOneRef) (CefBaseForPureC * self);
  } * vmt;
};

struct CefAppForPureC {
  const union {
    CefBaseForPureC::Vmt * Base;
    struct {
      CefBaseForPureC::Vmt Base;
      void (CEF_API * OnBeforeCommandLineProcessing) (CefAppForPureC * self, CefString process, ICefCommandLine command);
      void (CEF_API * OnRegisterCustomSchemes) (CefAppForPureC * self, ICefSchemeRegistrar registrar);
      ICefResourceBundleHandler (CEF_API * GetResourceBundleHandler) (CefAppForPureC * self);
    } * Self;    
  } vmt;
};

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


Do You know anything about OleStr or Delphi (Pascal) strings? In that cases the string is pointer to first char of here, buf all service data (chars count, refs count) are stored in same heap block before this pointer (before text content).

magreenblatt commented 4 years ago

The no anybody who use pure C now without classes with virtual method.

That’s not true. We currently have users of the C API without C++.

But my example may be implemented in pure C for emulation VMT like below.

Thanks for the example. I agree that something like this would probably be fine.

In that cases the string is pointer to first char of here, buf all service data (chars count, refs count) are stored in same heap block before this pointer (before text content).

What advantage do you see in this approach? You would still need to allocate new memory for static strings to hold header + char[].

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


What advantage do you see in this approach? You would still need to allocate new memory for static strings to hold header + char[].

The most popular modern programming languages use String type object that holds Unicode charset text with hidden prior StrPrefix struct but String instance points to first character in text buffer, like below.

#ifdef UNICODE
typedef wchar_t CefChar;
#else
typedef char_t CefChar;
#endif

#pragma pack(push, 1)
struct CefStrInfo {
  int RefCount;
  unsigned TextLength;
};
#pragma pack(pop)

template<unsigned Length> struct StaticString {
  CefStrInfo Prefix;
  CefChar Value[Length];
  StaticString() :
    Prefix { -1, Length } {
  }
};

struct CefString {
  const CefChar * Text;
};

StaticString<5> _str;
const auto str = CefString(_str.Value);

void CEF_API SomeApi(CefString text);

When CEF strings will have format like shown upper the passing static values will require formal type casting only instead of allocation on stack the wrapper variable of cef_string_t type that is redundant because dtor() not used in this case.

magreenblatt commented 4 years ago

Original comment by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).


Current cef_string_t capable to transfer existing char data via API layer without making copies at client side.

As for: “Expanding all CEF API to static export with typed handles for each instance type will require a huge DLL export section, so will follow a long time when libcef.dll dynamically linking to master process.”. Exports will not be so huge. Importing it is something for what dynamic linker exist in the end. Long time is not a value of measurement, so hard to take argument. Finally, you free to load library dynamically, and query exports manually (so it will inject nothing in caller).

You doesnt take into account another qualities: ability to create more stable ABI.

We can hide this exports via own set of runtime API with symbolic resolution, and this will have even more potential, but there is was never goal for what. Just never implemented idea. But, i feel what you will say what this “runtime” will be slow, because everything can be flattened into vtables.

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


Current cef_string_t capable to transfer existing char data via API layer without making copies at client side.

The type cef_string_t is redundant for passing static text (the dtor() member not using) !!!

Use

struct cef_text_t {
  cef_char_t * str;
  size_t length;
};

and when it's need use

struct cef_string_t {
  cef_text_t text;
  void (CEF_CALLBACK * dtor) (cef_char_t * str);
};

magreenblatt commented 4 years ago

Original comment by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).


No-one say what there it is static text. I'm use this for marshalling .NET strings onto CEF side without additional intermediate copy.

magreenblatt commented 4 years ago

Keep in mind that most strings originate from the client or from Chromium (not CEF) and will not already have your proposed header. With the current cef_string_t design those strings can be passed over the API boundary without requiring a copy. With your new proposed design every string that does not already have the header will need to be copied, and that includes large static strings. I would prefer to move in the opposite direction and reduce even further the number of copies required. For example, by supporting move-able string types where ownership can be passed to Chromium without requiring a copy. That would require the string to be allocated with a CEF function (so that it can be deleted in libcef), but it would not be compatible with requiring a special CEF-specific destructor that Chromium doesn’t know how to call.

The type cef_string_t is redundant for passing static text (the dtor() member not using)

Indeed. An unused dtor pointer is still preferable to always requiring a copy.

#define StrRefCount(handle)

Refcounting strings can be a performance advantage in certain use cases (like an HTML parser), but it’s not an optimization that we need in CEF. There aren’t many performance sensitive API paths in CEF, and the majority of strings passed to/from CEF are unique. Also, once we pass a string into Chromium it would have no knowledge of a special CEF-specific destructor.

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


On comment Dmitry Azaraev

Marshalling for .NET String type into unmanaged cef_string_t struct do via managed stack to allocate variable of type cef_string_t that holds handle of origin string object into member cef_string_t::str and System.String/Length as IntPtr cef_string_t::length member.

When CEF library will use standard string types for Windows libraries (BStr, LPCStr, LPWStr) then string marshalling may be implemented language compiler instead of reinventing wheel.

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


On comment Marshall Greenblatt

Use BStr type for string marshalling on Windows. For other OS (Nix, Mac) the implementation of BStr may provide via third party libs that emulate BStr functional.

When you positioning LibCef product as shared library for using in applications written at different languages (not C/C++ only) you should use one of defaults types for target OS strings: BStr, PCStr, PWStr for strings marshalling, then language compiler may provide native support for target strings types.

Do not reinvent a wheel !!!

magreenblatt commented 4 years ago

@{557058:5f4b8024-dc2c-4057-a4ce-06df6e3b9670} While your suggestions may be great for the average new project, they’re not necessarily appropriate for CEF. We have to carefully consider the cost/benefit trade-offs of different implementations in the context of current CEF users and Chromium requirements.

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


On comment Dmitry Azaraev

We can hide this exports via own set of runtime API with symbolic resolution, and this will have even more potential, but there is was never goal for what. Just never implemented idea. But, i feel what you will say what this “runtime” will be slow, because everything can be flattened into vtables.

The alternative implementation of export all API by typed handles instead of VMT or RefPtr instances may be implemented as single exported invoke that return handle to struct of all API, like below.

typedef struct CefCommandLine * HCefCommandLine;
typedef struct RefPtr * HRefPtr;

typedef struct {
  struct {
    HCefCommandLine (CEF_CALLBACK * Create) ();
    HCefCommandLine (CEF_CALLBACK * GetGlobal) ();
    bool (CEF_CALLBACK * IsValid) (HCefCommandLine);
    bool (CEF_CALLBACK * IsReadOnly) (HCefCommandLine);
    // ...
  } CommandLine;
  struct {
    void (CEF_CALLBACK * AddRef) (HRefPtr);
    // ...
  } RefPtr;
  // ...
} const * HCefApi;

CEF_EXPORT HCefApi CefApi();

magreenblatt commented 4 years ago

Original comment by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).


I'm doesnt reinventing wheel. C++ string_view or .NET Span plays and used exactly same role as cef_string_t play: provide zero-cost access to part or full contiguous data. And there is no compilers who can substitute incompatibe types, even when this is possible thereotically. And marshalling to everything other type is copy in practice. You fight for one IntPtr on the stack, but your proposition turn effectively from zero-cost to heap string transfer on client side.

In the end this is no big matter, but defering this action onto CEF side allow CEF be more flexible in strategy which it might choose.

As for BStr and other windows-specific types: they doesnt make any things simplier with interop with different languages: no languages support this shit.

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


To Dmitry Azaraev

The .NET specific type Span does not implemented by other languages (no languages support this shit). See my comment for shared library requirements.

What about embedded cef_string_t members in cef_settings_t, cef_browser_settings_t, …? The each string member requires place of 3 CPU-Words size (with unused callback dtor() member).

magreenblatt commented 4 years ago

To summarize, I think the detail that we’ve agreed upon is the use of a vtable pointer structure for the C API, like in this comment.

@{557058:5f4b8024-dc2c-4057-a4ce-06df6e3b9670} Will you implement this change in the translator tool and submit a PR?

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


What do you mean for PR (wikipedia.org)? Is the Pull Request (PR)?

magreenblatt commented 4 years ago

What do you mean for PR

PR = Pull Request

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


What about types cef_string_list_t, string_map_t, string_multimap_t? Will they implemented as ref-counted instances with vtable interface or still yeit as handles?

magreenblatt commented 4 years ago

What about types cef_string_list_t, string_map_t, string_multimap_t? Will they implemented as ref-counted instances with vtable interface or still yeit as handles?

Let’s leave those types unchanged for now.

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


Will you implement this change in the translator tool and submit a PR?

I did not use CEF translator tool never, thus I do not know how to implement this change in the translator tool.

magreenblatt commented 4 years ago

The CEF translator tool is a set of python scripts that generates the C API source and headers. You can find details in translator.README.txt (it’s a bit outdated, but still mostly accurate).

magreenblatt commented 4 years ago

Original comment by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).


@{557058:5f4b8024-dc2c-4057-a4ce-06df6e3b9670} I told you, what C++ nowadays has string_view which play similar role as cef_string_t do. Relatively conservative .NET got in last years not only Span data type but also direct support for interior GC pointers on stack, to achieve… same thing what cef_string_t do. You can see main difference: all of them represent pointer and length rather than be self-contained variable-length data type like .NET string or your’s proposed string. Yes, in many languages length-prefixed strings are used, and they are good when needed to be stored on heap. But all string consumers which doesn’t rely on their identity or doesn’t take ownership - always prefer span/view/whatever. And i’m should say what cef_string_t was modeled far earlier than cases listed above was actually implemented and massively propagandized. So, after more than 10 years history, i’m say what it was brilliant solution and keeps their status on this. This fits in API greatly.

As for dtor() - it is not really a big price, and doesn’t strictly required, but moving to something else may be painful in practice, because you have 3 ropes without 2 ends at hands: 1) how this is used internally in CEF, 2) how it is used in libcefwrapper (dll boundary code) and 3) how it is used by CEF clients. (I’m personally forget dtor details long time ago... i’m not use it today, but if CEF will accept UTF8 strings → then i’m might reconsider this.)

magreenblatt commented 4 years ago

I’m personally forget dtor details long time ago... i’m not use it today, but if CEF will accept UTF8 strings → then i’m might reconsider this.

Note that you can change the default string data type for CEF by modifying the #define in cef_string.h and recompiling.

magreenblatt commented 4 years ago

Original comment by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).


Note that you can change the default string data type for CEF by modifying the #define in cef_string.h and recompiling

Thanks. I’m think what i’m currently already do it or should read it and should have some form of assertion if it is not UTF16. :slight_smile: And we currently even include it in API hash... By forgetting generally I meant what once I realized strategy how to use it on client side in most efficient way for me - i’m stopped to think about it at all. But at current state changing UTF16 to UTF8 will make things more complex rather than easier. Sometimes it is profitable to have ability call some method within UTF8 form (e.g. have dual interface, basically around V8-eval-like because basically only V8 internally may utilize both types of source).

magreenblatt commented 4 years ago

I would like to eventually be explicit about the string type (e.g. use cef_string_utf8_t or cef_string_utf16_t directly instead of the cef_string_t define). The selected type would then match the underlying Chromium/CEF string type and thereby avoid unnecessary Unicode conversions. But I haven’t done that yet because it would be a time-consuming project to review every method and choose the correct string type, and also require a lot of client code changes.

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


It is unconvincing and applies to C/C++ features and CEF API architecture.

What about next sample?

struct CefStringList {
  CefStringList() :
    handle(cef_string_list_alloc()) {
  }
  CefStringList operator +(const std::wstring& value) const {
    cef_string_t marshal;
    auto wrapper = &marshal;
    const auto length = value.length();
    if (length) {
      // Expand value into cef_string
      // Write data to stack (memory that does not in CPU chip physically, so system bus sync is required).
      // The stack segment does not mapped into CPU cache (only code segment in prefetch)
      wrapper->str = const_cast<wchar_t*>(value.data());
      wrapper->length = length;
      wrapper->dtor = nullptr; // The REDUNDANT member for const InputString param
    }
    else {
      // Null ptr to cef_string ???
      wrapper = nullptr;
      // Or should pass empty cef_string ???
      // Write data to stack (memory that does not in CPU chip physically, so system bus sync is required).
      // The stack segment does not mapped into CPU cache (only code segment in prefetch)
      wrapper->str = nullptr;
      wrapper->length = 0;
      wrapper->dtor = nullptr; // The REDUNDANT member for const InputString param
    }
    cef_string_list_append(handle, wrapper);
    return *this;
  }
private:
  cef_string_list_t handle;
};

When cef_string_t param by ref is optional what to use NULL or pointer to empty placeholder? When classic String objects is used (as pointer to instance in heap) then behavior is trivial.

magreenblatt commented 4 years ago

Original comment by Dmitry Azaraev (Bitbucket: dmitry-azaraev, GitHub: dmitry-azaraev).


I would like to eventually be explicit about the string type (e.g. use cef_string_utf8_t or cef_string_utf16_t directly instead of the cef_string_t define). The selected type would then match the underlying Chromium/CEF string type and thereby avoid unnecessary Unicode conversions. But I haven’t done that yet because it would be a time-consuming project to review every method and choose the correct string type, and also require a lot of client code changes.

In practice i’m positive on this. Even .NET about to expose standard Utf8String type (as side type). You even might consider to create dual interfaces. :slight_smile:

magreenblatt commented 4 years ago

When cef_string_t param by ref is optional what to use NULL or pointer to empty placeholder?

They should behave the same, and insert an empty CefString in the std::vector. You can see the current implementation in string_list_impl.cc.

Making better C++ wrappers for cef_string_list_t, etc, is issue #222.

magreenblatt commented 4 years ago

Original comment by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


My tests for passing nullptr to input param cef_string_t* show the follows:
cef_string_list_append(handle, nullptr) – OK
cef_string_map_append(handle, nullptr, nullptr) – OK
cef_string_multimap_append(handle, nullptr, nullptr) – OK
cef_dictionary_value_t::set_int(target, nullptr, 0) – FAIL (Exception in libcef.dll)

magreenblatt commented 4 years ago

Original changes by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


magreenblatt commented 4 years ago

Original changes by Михаил Юрьевич Брызгалов (Bitbucket: Michael_Bryzgalov).


magreenblatt commented 3 years ago