Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

gaaclarke
Copy link
Member

Description

Related Issues

Tests

I added the following tests:

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Reviewer Checklist

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

@@ -19,6 +19,7 @@ NS_ASSUME_NONNULL_BEGIN
* @warning This class is a work-in-progress and may change.
* @see https://github.com/flutter/flutter/issues/72009
*/
FLUTTER_EXPORT
Copy link
Member

Choose a reason for hiding this comment

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

Why do we add __attribute__((visibility("default"))) to classes in our public headers? I wouldn't have thought that would be necessary but I don't know the history of this, is FLUTTER_EXPORT defined to something else in some cases?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, that's what it's defined as. When we compile the library we have the default visibility set to private, it's a linker flag. The reason for it is mostly around c++ code. By being more restrictive on what symbols are public different optimizations can happen in the loader, on top of having a little bit more control over how people use your API.

Copy link
Member

Choose a reason for hiding this comment

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

we have the default visibility set to private

Ah, gotcha.

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

LGTM

@gaaclarke gaaclarke merged commit effb529 into flutter:master Jan 14, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 14, 2021
cbracken pushed a commit to flutter/flutter that referenced this pull request Jan 14, 2021
* d7b25f9 [web] Reland: Enable the new rich paragraph implementation (flutter/engine#23659)

* 507a887 fix ax unique id flake (flutter/engine#23518)

* e5a4c7c Roll Fuchsia Linux SDK from BUsKF6z4t... to a7ezEWPM5... (flutter/engine#23668)

* 7d1e4e3 Roll Skia from 0e4a29af9db2 to af9b58e287b5 (10 revisions) (flutter/engine#23670)

* 635f64a Roll Skia from af9b58e287b5 to f435ada424df (1 revision) (flutter/engine#23671)

* 6957611 Roll Fuchsia Mac SDK from BsUY1yjWh... to BoBy7Eobf... (flutter/engine#23672)

* 37131f1 Use non-deprecated SkImageFilter factory functions (flutter/engine#23649)

* 38f08ff Roll Fuchsia Linux SDK from a7ezEWPM5... to erwxDS3kf... (flutter/engine#23676)

* a56183c Roll Skia from f435ada424df to 8f78d5528438 (7 revisions) (flutter/engine#23680)

* 9d75996 Revert "[web] Enable the new rich paragraph implementation (#23162)" (flutter/engine#23687)

* b030bc8 Roll Dart SDK from 724d9e5e7d71 to 3629a798353d (1 revision) (flutter/engine#23681)

* a8a7534 Support Mice in iPadOS 13.4+ (flutter/engine#23636)

* 3f691f0 [web] Fix semantic node order for webkit (flutter/engine#23601)

* 70d08c6 Roll Skia from 8f78d5528438 to 890b2b406a60 (9 revisions) (flutter/engine#23692)

* 5459565 Roll Fuchsia Mac SDK from BoBy7Eobf... to DVYrV15dq... (flutter/engine#23693)

* 3fd4c63 [web] Fix letter spacing for rich paragraphs (flutter/engine#23683)

* effb529 Added missing export for the flutter engine group. (flutter/engine#23689)
gspencergoog pushed a commit to gspencergoog/engine that referenced this pull request Jan 20, 2021
hjfreyer pushed a commit to hjfreyer/engine that referenced this pull request Mar 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants