c61b68aecf
We were passing offsets and sizes as integers. Use floats instead. In some parts of the code, sizes and offsets are in CSS pixels, in other parts they are in device pixels, and in some other parts they are in points. There are reasons for this, although it's currently a bit more convoluted than it has to be. Converting between them was done carefully with integer arithmetic and some special rounding code. This has worked mostly fine, but is fragile. I'm working on a CL that straightens out the conversions, to use CSS pixels instead of points in the Blink APIs (since that's what Blink uses internally). This would however mean that, if we were to keep on using integers, rounding errors that used to occur when printing HTML with Blink would be fixed, but, at the same time, we'd introduce new rounding errors when printing with a plug-in (when opening a PDF and printing it), since that part of the code wants things in points. So use floats to avoid this. This also allows for removal of PrintParamsWithFloatingSize. Although floats have precision issues for large integer values, this shouldn't be a problem here, since all the values changed are about page sizes, or offsets into a page (margins, unprintable area, etc.). Floats have 23 bits for the integer part, so as long as we stay (way) below a million pixels / points / whatever, we're good. It would easily become a problem if we start using floats for offsets into documents, though, as documents can become very tall. This CL isn't expected to make much of a behavior difference on its own. We'll still round down sizes to the nearest integer when entering Blink HTML layout, since we cannot reliably print fractional page sizes anyway. Furthermore, the way LocalFrame::ResizePageRectsKeepingRatio() is used to magically convert from points to pixels is inaccurate, and still causes the symptoms described in crbug.com/1444579 But it should now be more straight-forward to fix such issues without introducing new ones. Change-Id: I5fc5afeb14e5470faf970c9f7c94d0fad243ce3d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4604506 Reviewed-by: danakj <danakj@chromium.org> Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/main@{#1160870}
About //components
This directory is meant to house features or subsystems that are used in more than one part of the Chromium codebase.
Use cases:
- Features that are shared by Chrome on iOS (
//ios/chrome) and Chrome on other platforms (//chrome).- Note:
//iosdoesn't depend on//chrome.
- Note:
- Features that are shared between multiple embedders of content. For example,
//chromeand//android_webview. - Features that are shared between Blink and the browser process.
- Note: It is also possible to place code shared between Blink and the
browser process into
//third_party/blink/common. The distinction comes down to (a) whether Blink is the owner of the code in question or a consumer of it and (b) whether the code in question is shared by Chrome on iOS as well. If the code is conceptually its own cross-process feature with Blink as a consumer, then//componentscan make sense. If it's conceptually Blink code, then//third_party/blink/commonlikely makes more sense. (In the so-far hypothetical case where it's conceptually Blink code that is shared by iOS, raise the question on chromium-dev@, where the right folks will see it).
- Note: It is also possible to place code shared between Blink and the
browser process into
Note that the above list is meant to be exhaustive. A component should not be
added just to separate it from other code in the same layer that is the only
consumer; that can be done with strict DEPS or GN visibility rules.
Before adding a new component
- Is there an existing component that you can leverage instead of introducing
a new component?
- Can you restructure an existing component to logically encompass the proposed new code?
- As a general rule, we prefer fewer top level components. So, consider whether adding sub-features within an existing component is more appropriate for your use case.
- Historically, dependency issues were simply addressed by adding new components. But, you can (and it is preferred to) solve that by restructing an existing component and its dependencies where possible.
Guidelines for adding a new component
- You will be added to an
OWNERSfile under//components/{your component}and be responsible for maintaining your addition.- You must specify at least two OWNERS for any new component.
- A
//components/OWNERmust approve of the location of your code. - The CL (either commit message or comment) must explicitly specify what use case(s) justify the new component.
- Code must be needed in at least 2 places in Chrome that don't have a "higher
layered" directory that could facilitate sharing (e.g.
//content/common,//chrome/utility, etc.). - The CL adding a new component should be substantial enough so that //components/OWNERS can see its basic intended structure and usage before approving the addition (e.g., it should not just be an empty shell).
- You must add a
DIR_METADATAfile under//components/{your component}with an appropriately specified bug-component.
Dependencies of a component
Components cannot depend on the higher layers of the Chromium codebase:
//android_webview//chrome//chromecast//headless//ios/chrome//content/shell
Components can depend on the lower layers of the Chromium codebase:
//base//gpu//mojo//net//printing//ui
Components can depend on each other. This must be made explicit in the
DEPS file of the component.
Components can depend on //content/public, //ipc, and
//third_party/blink/public. This must be made explicit in the DEPS file of
the component. If such a component is used by Chrome for iOS (which does not
use content or IPC), the component will have to be in the form of a layered
component.
//chrome, //ios/chrome, //content and //ios/web can depend on
individual components. The dependency might have to be made explicit in the
DEPS file of the higher layer (e.g. in //content/browser/DEPS). Circular
dependencies are not allowed: if //content depends on a component, then that
component cannot depend on //content/public, directly or indirectly.
Structure of a component
As mentioned above, components that depend on //content/public, //ipc, or
third_party/blink/public might have to be in the form of a layered
component.
Components that have bits of code that need to live in different processes (e.g. some code in the browser process, some in the renderer process, etc.) should separate the code into different subdirectories. Hence for a component named 'foo' you might end up with a structure like the following (assuming that foo is not used by iOS and thus does not need to be a layered component):
components/foo-BUILD.gn,DEPS,DIR_METADATA,OWNERS,README.mdcomponents/foo/browser- code that needs the browser processcomponents/foo/common- for e.g. Mojo interfaces and suchcomponents/foo/renderer- code that needs renderer process
These subdirectories should have DEPS files with the relevant restrictions in
place, i.e. only components/foo/browser should be allowed to #include from
content/public/browser. Note that third_party/blink/public is a
renderer process directory except for third_party/blink/public/common which
can be used by all processes.
Note that there may also be an android subdir, with a Java source code
structure underneath it where the package name is org.chromium.components.foo,
and with subdirs after 'foo' to illustrate process, e.g. 'browser' or
'renderer':
components/foo/android/{OWNERS,DEPS}components/foo/android/java/src/org/chromium/components/foo/browser/components/foo/android/javatests/src/org/chromium/components/foo/browser/
Code in a component should be placed in a namespace corresponding to the name of
the component; e.g. for a component living in //components/foo, code in that
component should be in the foo:: namespace.