diff options
| author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2021-05-20 09:47:09 +0200 |
|---|---|---|
| committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2021-06-07 11:15:42 +0000 |
| commit | 189d4fd8fad9e3c776873be51938cd31a42b6177 (patch) | |
| tree | 6497caeff5e383937996768766ab3bb2081a40b2 /chromium/docs | |
| parent | 8bc75099d364490b22f43a7ce366b366c08f4164 (diff) | |
| download | qtwebengine-chromium-189d4fd8fad9e3c776873be51938cd31a42b6177.tar.gz | |
BASELINE: Update Chromium to 90.0.4430.221
Change-Id: Iff4d9d18d2fcf1a576f3b1f453010f744a232920
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Diffstat (limited to 'chromium/docs')
119 files changed, 3621 insertions, 1026 deletions
diff --git a/chromium/docs/README.md b/chromium/docs/README.md index 133d37e36cc..e6fb53dc667 100644 --- a/chromium/docs/README.md +++ b/chromium/docs/README.md @@ -48,8 +48,8 @@ used when committed. (on a Linux host) * [Cast for Android Build Instructions](android_cast_build_instructions.md) - Cast for Android (on a Linux host) -* [Fuchsia Build Instructions](fuchsia_build_instructions.md) - Fuchsia target - (on a Linux host) +* [Fuchsia Build Instructions](fuchsia/build_instructions.md) - + Fuchsia target (on a Linux host) * [iOS Build Instructions](ios/build_instructions.md) - iOS target (on a MacOS host) * [Chrome OS Build Instructions](chromeos_build_instructions.md) - Chrome OS @@ -123,6 +123,7 @@ used when committed. and handle thread safety in Chrome. * [Callback<> and Bind()](callback.md) - All about Callbacks, Closures, and Bind(). +* [Chromium Views UI](ui/index.md) - Working with the desktop UI framework. * [Views Platform Styling](ui/views/platform_style.md) - How views are styled to fit in different native platforms * [Tab Helpers](tab_helpers.md) - Using WebContents/WebContentsObserver to add @@ -254,7 +255,7 @@ used when committed. * [Windows Split DLLs](windows_split_dll.md) - Splitting `chrome.dll` into multiple dlls to work around toolchain limitations on Windows. * [Windows Native Window Occlusion Tracking](windows_native_window_occlusion_tracking.md) -* [Windows PWA Integration](windows_pwa_integration.md) - Integration with +* [Windows PWA Integration](windows_pwa_integration.md) - Integration with Progressive Web Apps on Windows * [Windows Shortcut and Taskbar Handling](windows_shortcut_and_taskbar_handling.md) * [Windows Virtual Desktop Integration](windows_virtual_desktop_handling.md) @@ -304,10 +305,13 @@ used when committed. * [Kiosk mode and public sessions](enterprise/kiosk_public_session.md) * [Debugging UI in OOBE/login/lock](login/ui_debugging.md) * [Chrome Logging on Chrome OS](chrome_os_logging.md) +* [Debugging tips](testing/chromeos_debugging_tips.md) ### Misc WebUI-Specific Docs * [Creating WebUI Interfaces in components/](webui_in_components.md) How to create a new WebUI component in the `components/` directory. +* [Trusted Types on WebUI](trusted_types_on_webui.md) Tips for coding in WebUI + with Trusted Types in mind. ### Media * [Audio Focus Handling](media/audio_focus.md) - How multiple MediaSession diff --git a/chromium/docs/accessibility/chromevox.md b/chromium/docs/accessibility/chromevox.md index 05f69285a08..1e9574cc047 100644 --- a/chromium/docs/accessibility/chromevox.md +++ b/chromium/docs/accessibility/chromevox.md @@ -64,6 +64,41 @@ ChromeVox options page with Search+Shift+o, o; then, substitute the “options.html” path with “background.html”, and then open up the inspector. +### Debugging ChromeOS + +To debug ChromeVox in ChromeOS, you need to add the command-line flag to the +config file in device under test(DUT) instead of starting chrome from command +line. + +``` +(dut) $ echo " --remote-debugging-port=9222 " >> /etc/chrome_dev.conf +(dut) $ restart ui +``` + +This is also written in +[Simple Chrome Workflow Doc](https://chromium.googlesource.com/chromiumos/docs/+/HEAD/simple_chrome_workflow.md#command_line-flags-and-environment-variables). + +You need to ssh from your development device into your DUT forwarding port 9222 +to open ChromeVox extension background page in your dev device, for example +``` +ssh my_crbook -L 3333:localhost:9222 +``` + +Then open the forwarded port in the development device, http://localhost:3333 in +the example. + +You may need to remove rootfs verification to write to `/etc/chrome_dev.conf`. + +``` +(dut) $ crossystem dev_boot_signed_only=0 +(dut) $ sudo /usr/share/vboot/bin/make_dev_ssd.sh --remove_rootfs_verification +(dut) $ reboot +``` + +See +[Chromium OS Doc](https://chromium.googlesource.com/chromiumos/docs/+/HEAD/developer_mode.md#disable-verity) +for more information about removing rootfs verification. + ### Running tests Build the browser_tests target. To run lots of tests in parallel, run it like diff --git a/chromium/docs/accessibility/chromevox_on_desktop_linux.md b/chromium/docs/accessibility/chromevox_on_desktop_linux.md index 07ef8344cbc..828b963ce42 100644 --- a/chromium/docs/accessibility/chromevox_on_desktop_linux.md +++ b/chromium/docs/accessibility/chromevox_on_desktop_linux.md @@ -85,9 +85,14 @@ gsutil ls gs://chromeos-localmirror/distfiles/espeak* Pick the latest version and ``` -gsutil cp gs://chromeos-localmirror/distfiles/espeak-ng-20180801.tar.gz /usr/share/chromeos-assets/speech_synthesis/espeak-ng/ -tar xvf /usr/share/chromeos-assets/speech_synthesis/espeak-ng/espeak-ng-20180801.tar.gz -rm /usr/share/chromeos-assets/speech_synthesis/espeak-ng/espeak-ng-20180801.tar.gz +VERSION=1.49.3.7 +TMPDIR=$(mktemp -d) +gsutil cp gs://chromeos-localmirror/distfiles/espeak-ng-$VERSION.tar.gz $TMPDIR +tar -C $TMPDIR -xvf $TMPDIR/espeak-ng-$VERSION.tar.gz +sudo mkdir -p /usr/share/chromeos-assets/speech_synthesis/espeak-ng/ +sudo chown -R $(whoami) /usr/share/chromeos-assets/ +cp -r $TMPDIR/espeak-ng/chrome-extension/* /usr/share/chromeos-assets/speech_synthesis/espeak-ng +rm -rf $TMPDIR ``` **Be sure to check permissions of /usr/share/chromeos-assets, some users report diff --git a/chromium/docs/accessibility/select_to_speak.md b/chromium/docs/accessibility/select_to_speak.md index b6b6e46505e..44641f6f55d 100644 --- a/chromium/docs/accessibility/select_to_speak.md +++ b/chromium/docs/accessibility/select_to_speak.md @@ -49,6 +49,8 @@ chrome/browser/resources/chromeos/accessibility/select_to_speak/ - The status tray button, ash/system/accessibility/select_to_speak_tray.h +- Floating panel, system/accessibility/select_to_speak_menu_bubble_controller.h + In addition, there are settings for STS in chrome/browser/resources/settings/a11y_page/manage_a11y_page.* @@ -129,6 +131,9 @@ all the root’s children to find ones that overlap with the selected rect. Walking back down through the children occurs in NodeUtils.findAllMatching, and results in a list of AutomationNodes that can be sent for speech. +If the rect size is below a certain threshold, all nodes within overlapped +block parent are selected. + ##### With search + s select_to_speak.js requests focus information from the Automation API. The @@ -187,6 +192,98 @@ status change, and listens to onSelectToSpeakStateChangeRequested to know when a user wants to change state. The STS extension is the source of truth for STS state. +### Navigation features + +When `select-to-speak-navigation-control` flag is enabled, STS will display a +floating control panel when activated. The control panel hosts controls for +pause/resume, updating reading speed, navigating by sentence or paragraph, +and deactivating STS. + +#### Floating control panel + +The panel is implemented as a native ASH component +[select_to_speak_menu_bubble_controller.h](https://source.chromium.org/chromium/chromium/src/+/master:ash/system/accessibility/select_to_speak_menu_bubble_controller.h). +Similar to focus rings, the STS component extension communicates with the panel +via the `chrome.accessibilityPrivate` API. The +`chrome.accessibilityPrivate.updateSelectToSpeakPanel` API controls the +visibility and button states, and panel actions are communicated back to the +extension by adding a listener to +`chrome.accessibilityPrivate.onSelectToSpeakPanelAction`. + +When the panel is displayed, STS will no longer dismiss itself when TTS +playback is complete. The user must quit STS either from the panel or +the tray button. + +##### Keyboard shortcuts + +When the panel is displayed, it is initially focused and captures keypresses to +implement keyboard shortcuts: + +* Space - activates currently focused button, which is 'Pause/Resume' + initially. +* Left Arrow - Navigate to previous sentence (for RTL languages, this is Right + Arrow) +* Right Arrow - Navigate to next sentence (for RTL languages, this is Left + Arrow) +* Up Arrow - Navigate to previous paragraph +* Down Arrow - Navigate to next paragraph + +If the panel loses focus, keyboard shortcuts will no longer work. User can press +Search+S keyboard shortcut (with no text selection) to restore focus to the +panel. + +##### Disallowed nodes + +The panel is not shown when STS is activated on nodes where navigation features +do not add value, such as in system UI or top-level windows. + +* System UI nodes - any nodes that have a `root` with role `desktop` +* Root nodes that are children of the root `desktop` node + +#### Pause/Resume + +Since `chrome.tts.pause` and `chrome.tts.resume` are not consistently +implemented across all TTS engines, STS implements pause/resume functionality +using the `chrome.tts.stop` and `chrome.tts.speak` APIs. While TTS is playing, +STS keeps track of the current word offset, and when TTS is resumed, it will +call `speak` with text trimmed to the start of the last spoken word. + +Resuming TTS behaves differently depending on the context: + +* If TTS was paused within the user-selected text, resuming will play until + the end of the selected text. +* If TTS stopped when it reached the end of the selected text, but before the + end of the paragraph, resuming will continue from that point to the end of + the paragraph. +* If TTS stopped when it reached the end of a paragraph, resuming will speak + the next paragraph. + +#### Paragraph navigation + +Users can navigate to adjacent paragraphs from the current block parent when +Select-to-speak is active. A 'paragraph' is any block element as defined by +[ParagraphUtils.isBlock](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/resources/chromeos/accessibility/select_to_speak/paragraph_utils.js) +and the navigation occurs in DOM-order. + +#### Sentence navigation + +Paragraphs are split into sentences based on the `sentenceStarts` property of +an AutomationNode. Users can skip to previous and next sentences using similar +technique as pause/resume (`stop` then `speak` with trimmed text). See +[sentence_utils.js](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/resources/chromeos/accessibility/select_to_speak/sentence_utils.js) +for logic on breaking node groups into sentences. + +#### Reading speed + +Users can slow down or speed up TTS speaking rate using the floating control +panel. The rate the user selects in the panel is multiplied by the system +default TTS rate. So if the user selects 1.2x reading speed in the panel and +has a system default of 2.0x, the effective TTS rate will be 2.4x. + +When users adjust reading speed, `chrome.tts.stop` is called, and +`chrome.tts.speak` is then called with text trimmed to the current word +position, passing in the new effective TTS rate as an option. + ### Special case: Google Drive apps Google Drive apps require a few work-arounds to work correctly with STS. @@ -218,3 +315,5 @@ for more details on design as well as UMA. - Per word highlighting, [go/chrome-sts-sentences-and-words](go/chrome-sts-sentences-and-words) and [go/chromeos-sts-highlight](go/chromeos-sts-highlight) + +- Navigation features, [go/enhanced-sts-dd](go/enhanced-sts-dd)
\ No newline at end of file diff --git a/chromium/docs/accessibility/switch_access.md b/chromium/docs/accessibility/switch_access.md index 8b1fe092ca6..3d27858fd30 100644 --- a/chromium/docs/accessibility/switch_access.md +++ b/chromium/docs/accessibility/switch_access.md @@ -31,11 +31,12 @@ after a set period of time (the **scanning speed**). The user can identify which element is currently focused because it is surrounded by a **focus ring**, which is two concentric rounded rectangles of -contrasting colors (currently fixed at light blue and black) that surround the +contrasting colors (currently fixed at light and dark blue) that surround the element. Sometimes, a second focus ring will appear onscreen, which has dashed -rather than solid lines. This dashed focus ring provides a preview of which node -will be focused, if the user selects the current node. It also provides a hint -that the current node is a navigational node, and is not directly actionable. +line for the inner rectangle. This dashed focus ring provides a preview of which +node will be focused next, if the user selects the current node. It also +provides a hint that the current node is a navigational node, and is not +directly actionable. There are two types of navigational nodes: nodes that group other nodes together, and the **back button**. The back button allows a user to exit the @@ -136,9 +137,9 @@ The Switch Access extension does the following, at a high level: - If it is a select command, the available actions are determined. -If there is only one, it is performed (Note that for navigational nodes, this -means entering or exiting a group). If more than one action is available, the -menu is opened and focus jumps into the menu. + - If there is only one action, it is performed (Note that for navigational + nodes, this means entering or exiting a group). If more than one action is + available, the menu is opened and focus jumps into the menu. 3. Listens for focus events, and moves Switch Access focus to follow system focus. diff --git a/chromium/docs/accessibility/tests.md b/chromium/docs/accessibility/tests.md index 6b00f0ecd47..ecd6489aed5 100644 --- a/chromium/docs/accessibility/tests.md +++ b/chromium/docs/accessibility/tests.md @@ -130,7 +130,7 @@ codebase. Here are a few other locations to check: * [chrome/android/javatests/src/org/chromium/chrome/browser/accessibility](https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromium/chrome/browser/accessibility/) * [chrome/browser/accessibility](https://cs.chromium.org/chromium/src/chrome/browser/accessibility/) -* [chrome/browser/chromeos/accessibility/](https://cs.chromium.org/chromium/src/chrome/browser/chromeos/accessibility/) +* [chrome/browser/ash/accessibility/](https://cs.chromium.org/chromium/src/chrome/browser/ash/accessibility/) * [ui/chromeos](https://cs.chromium.org/chromium/src/ui/chromeos/) * [ui/views/accessibility](https://cs.chromium.org/chromium/src/ui/views/accessibility/) diff --git a/chromium/docs/android_dynamic_feature_modules.md b/chromium/docs/android_dynamic_feature_modules.md index 3e5dd23d732..8bd10eb14a6 100644 --- a/chromium/docs/android_dynamic_feature_modules.md +++ b/chromium/docs/android_dynamic_feature_modules.md @@ -14,9 +14,6 @@ DFMs have the following limitations: * **WebView:** We don't support DFMs for WebView. If your feature is used by WebView you cannot put it into a DFM. -* **Android K:** DFMs are based on split APKs, a feature introduced in Android - L. Therefore, we don't support DFMs on Android K. As a workaround - you can add your feature to the Android K APK build. See below for details. ## Getting started @@ -29,6 +26,13 @@ instance of `foo`/`Foo`/`FOO` with `your_feature_name`/`YourFeatureName`/ `YOUR_FEATURE_NAME`. *** +*** note +**Note:** Chrome's bundles use the [android:isolatedSplits](https://developer.android.com/reference/android/R.attr#isolatedSplits) +attribute. For more details and advice on when to create a DFM, see +[go/isolated-splits-dev-guide](http://go/isolated-splits-dev-guide) +**(Googlers only)**. +*** + ### Reference DFM In addition to this guide, the @@ -580,8 +584,8 @@ follows: lang="am" type="android" /> <!-- List output file for all other supported languages. See - //chrome/android/java/strings/android_chrome_strings.grd for the full - list. --> + //chrome/browser/ui/android/strings/android_chrome_strings.grd for the + full list. --> ... </outputs> <translations> @@ -589,7 +593,7 @@ follows: <!-- Here, too, list XTB files for all other supported languages. --> ... </translations> - <release allow_pseudo="false" seq="1"> + <release seq="1"> <messages fallback_to_english="true"> <message name="IDS_BAR_IMPL_TEXT" desc="Magical string."> impl diff --git a/chromium/docs/android_native_libraries.md b/chromium/docs/android_native_libraries.md index 50096d381ac..8a5062d0545 100644 --- a/chromium/docs/android_native_libraries.md +++ b/chromium/docs/android_native_libraries.md @@ -21,7 +21,7 @@ Chrome on Android. * Android Q (TrichromeChrome.aab + TrichromeLibrary.apk): * Trichrome uses the exact same native library as Monochrome: `libmonochrome.so`. - * `libmonochrome.so` is stored in the shared library (TrichromeLibrary.apk) + * `libmonochrome.so` is stored in the shared APK (TrichromeLibrary.apk) so that it can be shared with TrichromeWebView. * It is loaded by `libchromium_android_linker.so` using `android_dlopen_ext()` to enable RELRO sharing. @@ -186,7 +186,7 @@ Builds on | Variant | Chrome | Library | Webview * For Android N-P: * The OS maintains a RELRO file on disk with the contents of the GNU_RELRO segment. * All Android apps that contain a WebView load `libmonochrome.so` at the same virtual address and apply RELRO sharing against the memory-mapped RELRO file. - * Chrome uses `MonochromeLibraryPreloader` to call into the same WebView library loading code. + * Chrome uses `WebViewLibraryPreloader` to call into the same WebView library loading code. * When Monochrome is the WebView provider, `libmonochrome.so` is loaded with the system's cached RELRO's applied. * `System.loadLibrary()` is called afterwards. * When Monochrome is the WebView provider, this only calls JNI_OnLoad, since the library is already loaded. Otherwise, this loads the library and no RELRO sharing occurs. diff --git a/chromium/docs/asan.md b/chromium/docs/asan.md index 4d5b2a580fb..db215fc2f5d 100644 --- a/chromium/docs/asan.md +++ b/chromium/docs/asan.md @@ -32,6 +32,12 @@ tests including browser\_tests and content\_browsertests). You can grab fresh Chrome binaries built with ASan [here](https://commondatastorage.googleapis.com/chromium-browser-asan/index.html). +The lists of ASan binaries are _very_ long, but you can filter down to more +specific releases by specifying a prefix like +[linux-debug/asan-linux-debug-83](https://commondatastorage.googleapis.com/chromium-browser-asan/index.html?prefix=linux-debug/asan-linux-debug-83). +This is useful for finding a build for a specific revision, since filenames are of +the form `asan-<platform>-<buildtype>-<revision>` (but not every revision has an +archived ASan build). ## Build tests with ASan diff --git a/chromium/docs/branch_sheriff.md b/chromium/docs/branch_sheriff.md index 62b1d8a3886..2b21073e18d 100644 --- a/chromium/docs/branch_sheriff.md +++ b/chromium/docs/branch_sheriff.md @@ -69,16 +69,23 @@ tests. ### Flaky tests -You should largely ignore flaky tests for the time being unless you have -specific reason to believe that a flake was introduced by a cherry-pick to the -branch in question. If a test is flaky on both trunk *and* a release branch, -the trunk sheriffs should investigate it. +Flaky tests that are disabled on trunk should also be disabled on any branches +with frequent failures of that test. If a trunk CL lands with no change other +than to disable one or more tests ([example](https://crrev.com/c/2507299)) and +it has an associated bug and the release manager is cc'd on the bug, you can and +should cherrypick it to the affected branch without requesting merge approval. + +On the other hand, if you believe that a flake was introduced by a cherry-pick +to the branch in question and is not flaky on trunk, you will need to create a +new CL to disable it only on the branch and go through the usual merge request +process. ### Landing changes -When you need to land a change to a branch, you'll need to go through the same -merge approval process as other cherry-picks. You should feel free to ping the -relevant release TPM as listed on [chromiumdash][chromiumdash-schedule]. +When you need to land a change to a branch, you'll need to go through [the same +merge approval process](./process/merge_request.md) as other cherry-picks (see +exception for flaky tests above). You should feel free to ping the relevant +release TPM as listed on [chromiumdash][chromiumdash-schedule]. ## Tools diff --git a/chromium/docs/building_old_revisions.md b/chromium/docs/building_old_revisions.md index 293247f763b..669e0cfff6b 100644 --- a/chromium/docs/building_old_revisions.md +++ b/chromium/docs/building_old_revisions.md @@ -47,6 +47,15 @@ required: $ gclient sync -D --force --reset ``` +Note that if you are attempting to build an old revision that is on a branch, +you will need to use: + +```shell +$ gclient sync -D --force --reset --with_branch_heads +``` + +instead. + **Warning: `gclient sync` may overwrite the URL of your `origin` remote** if it encounters problems. You'll notice this when Git starts thinking everything is "untracked" or "deleted". If this happens, fix and fetch the remote before diff --git a/chromium/docs/callback.md b/chromium/docs/callback.md index 412242c3ad5..1b51645a80c 100644 --- a/chromium/docs/callback.md +++ b/chromium/docs/callback.md @@ -232,6 +232,49 @@ OnceClosure task = other_task_runner->PostTask(FROM_HERE, std::move(task)); ``` +### Splitting a OnceCallback in two + +If a callback is only run once, but two references need to be held to the +callback, using a `base::OnceCallback` can be clearer than a +`base::RepeatingCallback`, from an intent and semantics point of view. +`base::SplitOnceCallback()` takes a `base::OnceCallback` and returns a pair of +callbacks with the same signature. When either of the returned callback is run, +the original callback is invoked. Running the leftover callback will result in a +crash. +This can be useful when passing a `base::OnceCallback` to a function that may or +may not take ownership of the callback. E.g, when an object creation could fail: + +```cpp +std::unique_ptr<FooTask> CreateFooTask(base::OnceClosure task) { + std::pair<base::OnceClosure,base::OnceClosure> split + = base::SplitOnceCallback(std::move(task)); + + std::unique_ptr<FooTask> foo = TryCreateFooTask(std::move(split.first)); + if (foo) + return foo; + + return CreateFallbackFooTask(std::move(split.second)); +} +``` + +While it is best to use a single callback to report success/failure, some APIs +already take multiple callbacks. `base::SplitOnceCallback()` can be used to +split a completion callback and help in such a case: + +```cpp +using StatusCallback = base::OnceCallback<void(FooStatus)>; +void DoOperation(StatusCallback done_cb) { + std::pair<StatusCallback, StatusCallback> split + = base::SplitOnceCallback(std::move(done_cb)); + + InnerWork(BindOnce(std::move(split.first), STATUS_OK), + BindOnce(std::move(split.second), STATUS_ABORTED)); +} + +void InnerWork(base::OnceClosure work_done_cb, + base::OnceClosure work_aborted_cb); +``` + ## Quick reference for basic stuff ### Binding A Bare Function @@ -329,9 +372,9 @@ void DoSomething(const base::RepeatingCallback<double(double)>& callback) { If running a callback could result in its own destruction (e.g., if the callback recipient deletes the object the callback is a member of), the callback should -be moved before it can be safely invoked. (Note that this is only an issue for -RepeatingCallbacks, because a OnceCallback always has to be moved for -execution.) +be moved or copied onto the stack before it can be safely invoked. (Note that +this is only an issue for RepeatingCallbacks, because a OnceCallback always has +to be moved for execution.) ```cpp void Foo::RunCallback() { diff --git a/chromium/docs/chrome_untrusted.md b/chromium/docs/chrome_untrusted.md index 0ffa408fa2c..d4240e06ae7 100644 --- a/chromium/docs/chrome_untrusted.md +++ b/chromium/docs/chrome_untrusted.md @@ -62,22 +62,66 @@ We currently use `postMessage()` to expose certain APIs to `chrome-untrusted://` We are hoping to move to Mojo to improve auditability of these APIs and to make the security review required. ## Can chrome-untrusted:// be the main document or does it need to be embedded in a `chrome://` page? -Yes, `chrome-untrusted://` can be the main document, although the most common case is for `chrome://` to embed a `chrome-untrusted://` page. +Yes, `chrome-untrusted://` can be the main document, although the most common case is for a `chrome://` page to embed a `chrome-untrusted://` subframe. That said, the `chrome-untrusted://` scheme is an implementation detail of the WebUI and should never be shown to users. This should be factored into account when deciding whether or not to use `chrome-untrusted://` as the main document. ## How do I use chrome-untrusted://? -(This will be out of date soon) -### Create a URLDataSource and register it. +### Create a standalone chrome-untrusted:// WebUI -This will make its resources loadable in Chrome. +1. Create a class overriding `ui::WebUIConfig` and another one overriding `ui::UntrustedWebUIController` + +`WebUIConfig` contains properties for the `chrome-untrusted://` page i.e. the host and scheme. In the future, this might also contain other properties like permissions or resources. + +`UntrustedWebUIController` register the resources for the page. + +```cpp +const char kUntrustedExampleHost[] = "untrusted-example"; +const char kUntrustedExampleURL[] = "chrome-untrusted://untrusted-example"; + +class UntrustedExampleUIConfig : public ui::WebUIConfig { + public: + UntrustedExampleUIConfig() + // Set scheme and host. + : WebUIConfig(content::kChromeUIUntrustedScheme, kUntrustedExampleHost) {} + ~UntrustedExampleUIConfig() override = default; + + std::unique_ptr<content::WebUIController> CreateWebUIController( + content::WebUI* web_ui) override { + return std::make_unique<UntrustedExampleUI>(web_ui); + } +}; + +class UntrustedExampleUI : public ui::UntrustedWebUIController { + public: + UntrustedExampleUI::UntrustedExampleUI(content::WebUI* web_ui) + : ui::UntrustedWebUIController(web_ui) { + + // Create a URLDataSource and add resources. + auto* untrusted_source = + content::WebUIDataSource::Create(kUntrustedExampleURL); + untrusted_source->AddResourcePath(...); + + // Register the URLDataSource + auto* browser_context = web_ui->GetWebContents()->GetBrowserContext(); + content::WebUIDataSource::Add(browser_context, untrusted_source); + } + + UntrustedExampleUI(const UntrustedExampleUI&) = delete; + UntrustedExampleUI& operator=(const UntrustedExampleUI&) = delete; + + UntrustedExampleUI::~UntrustedExampleUI() = default; +}; ``` -auto* data_source = - WebUIDataSource::Create(GURL(“chrome-untrusted://example/”)); -// Add resources i.e. data_source->SetRequestFilter -WebUIDataSource::Add(browser_context, data_source); + +2. Register the WebUIConfig + +Add the `WebUIConfig` to the list of WebUIConfigs in `[ChromeUntrustedWebUIControllerFactory](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/webui/chrome_untrusted_web_ui_controller_factory.cc)`. + +```cpp +register_config(std::make_unique<chromeos::UntrustedExampleUIConfig>()); ``` ### Embed chrome-untrusted:// in chrome:// WebUIs @@ -85,18 +129,18 @@ WebUIDataSource::Add(browser_context, data_source); Developers can embed `chrome-untrusted://` iframes in `chrome://` pages. [Example CL](https://chromium-review.googlesource.com/c/chromium/src/+/2037186). The general steps are: -1. Create and register a `URLDataSource` that serves necessary resources for the `chrome-untrusted://` page. +1. Create a WebUIConfig and UntrustedWebUIController to register the resources for the `chrome-untrusted://` page. 2. Allow the `chrome://` WebUI to embed the corresponding `chrome-untrusted://` WebUI. -``` +```cpp untrusted_data_source->AddFrameAncestor(kWebUIPageURL) ``` 3. Make `chrome-untrusted://` requestable by the main `chrome://` WebUI. -``` +```cpp web_ui->AddRequestableScheme(content::kChromeUIUntrustedScheme) ``` 4. Allow the `chrome://` WebUI to embed chrome-untrusted://. -``` +```cpp trusted_data_source->OverrideContentSecurityPolicy( - “frame-src ” + kChromeUntrustedPageURL); + “frame-src ” + kUntrustedExampleURL); ``` 5. Add communication mechanism to `chrome-untrusted://` frames. For example, `iframe.postMessage()` and `window.onmessage`. diff --git a/chromium/docs/chromeos_build_instructions.md b/chromium/docs/chromeos_build_instructions.md index a82f4c7980e..af8aed05003 100644 --- a/chromium/docs/chromeos_build_instructions.md +++ b/chromium/docs/chromeos_build_instructions.md @@ -10,6 +10,8 @@ build configurations: workstation. - Otherwise, Chrome's full integration can be covered by building for a real Chrome OS device or VM using [Simple Chrome](#Chromium-OS-Device-Simple-Chrome). +- Use `is_chromeos_device` in GN and `BUILDFLAG(IS_CHROMEOS_DEVICE)` in C++ code + to differentiate between these two modes. [TOC] @@ -95,7 +97,7 @@ Some useful flags: * `--enable-features=Feature1,OtherFeature2`: Enable specified features. Features are often listed in chrome://flags, or in source files such as [chrome_features.cc](https://source.chromium.org/chromium/chromium/src/+/master:chrome/common/chrome_features.cc) - or [chromeos_features.cc](https://source.chromium.org/chromium/chromium/src/+/master:chromeos/constants/chromeos_features.cc). + or [ash_features.cc](https://source.chromium.org/chromium/chromium/src/+/master:ash/constants/ash_features.cc). Note that changing values in chrome://flags does not work for linux-chromeos, and this flag must be used. * `--enable-ui-devtools[=9223]`: Allow debugging of the system UI through diff --git a/chromium/docs/chromium_browser_vs_google_chrome.md b/chromium/docs/chromium_browser_vs_google_chrome.md index 70d3f5ec12b..ff20f16f9f8 100644 --- a/chromium/docs/chromium_browser_vs_google_chrome.md +++ b/chromium/docs/chromium_browser_vs_google_chrome.md @@ -16,8 +16,9 @@ builds **on Linux**. Please include symbolized backtraces in bug reports if you don't have crash reporting turned on. * User metrics only if turned on -* Video and Audio codecs (may vary by distro) - * AAC, H.264, MP3, Opus, Theora, Vorbis, VP8, VP9, and WAV +* Video and Audio codecs (may vary by distribution) + * **H.264**, AV1, VP8, and VP9 video codecs. + * **AAC**, MP3, Opus, Theora, Vorbis, FLAC, and WAV audio codecs. * Sandboxed PPAPI (non-free) Flash plugin included in release * Code is tested by Chrome developers * Sandbox is always on @@ -33,8 +34,9 @@ builds **on Linux**. * Does not ever [report crashes](linux/crash_dumping.md). Please include symbolized backtraces in bug reports. * User metrics are never reported. -* Video and Audio codecs (may vary by distro) - * Opus, Theora, Vorbis, VP8, VP9, and WAV by default +* Video and Audio codecs (may vary by distribution) + * AV1, VP8, and VP9 video codecs. + * MP3, Opus, Theora, Vorbis, FLAC, and WAV audio codecs. * Supports NPAPI (unsandboxed) Flash plugins, including the one from Adobe in Chrome 34 and below * Code may be modified by distributions diff --git a/chromium/docs/cipd.md b/chromium/docs/cipd.md deleted file mode 100644 index 14298411b1e..00000000000 --- a/chromium/docs/cipd.md +++ /dev/null @@ -1,265 +0,0 @@ -# CIPD for chromium dependencies - -[TOC] - -## What is CIPD? -* CIPD stands for "Chrome Infrastructure Package Deployment". -* Its code and docs [live within the luci-go project][CIPD]. -* Chromium uses CIPD to avoid checking large binary files into git, which git - does not handle well. -* gclient supports CIPD packages in the same way as git repositories. They are - specified in [DEPS] and updated via `gclient sync`. -* You can [browse Chromium's CIPD repository][browse] online. - -[CIPD]: https://chromium.googlesource.com/infra/luci/luci-go/+/master/cipd/README.md -[DEPS]: /DEPS -[browse]: https://chrome-infra-packages.appspot.com/p/chromium - -## Adding a new CIPD dependency - -### 1. Set up a new directory for your dependency - -You'll first want somewhere in the repository in which your dependency will -live. For third-party dependencies, this should typically be a subdirectory -of `//third_party`. You'll need to add the same set of things to that -directory that you'd add for a non-CIPD dependency -- OWNERS, README.chromium, -etc. - -For example, if you want to add a package named `sample_cipd_dep`, you might -create the following: - -``` - third_party/ - sample_cipd_dep/ - LICENSE - OWNERS - README.chromium -``` - -For more on third-party dependencies, see [adding_to_third_party.md]. - -[adding_to_third_party.md]: /docs/adding_to_third_party.md - -### 2. Acquire whatever you want to package - -Build it, download it, whatever. Once you've done that, lay it out -in your local checkout the way you want it to be laid out in a typical -checkout. - -Staying with the example from above, if you want to add a package named -`sample_cipd_dep` that consists of two JARs, `foo.jar` and `bar.jar`, you might -lay them out like so: - -``` - third_party/ - sample_cipd_dep/ - ... - lib/ - bar.jar - foo.jar -``` - -### 3. Create a cipd.yaml file - -CIPD knows how to create your package based on a .yaml file you provide to it. -This file can either be generated by a GN template, or manually. - -#### 3a. Generating cipd.yaml via GN Template: - -The `cipd_package_definition` template in [build/cipd/cipd.gni] can be used to -create the yaml definition as part of Chromium's normal build process. Declare -a target like: -``` -cipd_package_definition("my_cipd_package") { - package = "path/to/cipd/package" - description = "Prebuilt test binary." - install_mode = "copy" - deps = [ "//path/to:test_binary_target" ] - sources = [ "//path/to:test_binary_file" ] -} -``` - -After generating build files, you can then run -`ninja -C out/Default my_cipd_package`, which creates the .yaml file under -`out/Default/my_cipd_package/package.yaml`. - -[build/cipd/cipd.gni]: https://source.chromium.org/chromium/chromium/src/+/master:build/cipd/cipd.gni - -#### 3b. Generating cipd.yaml by hand: - -You can also write the .yaml file by hand. The file should take the following -form: - -``` -# Comments are allowed. - -# The package name is required. Third-party chromium dependencies should -# unsurprisingly all be prefixed with chromium/third_party/. -package: chromium/third_party/sample_cipd_dep - -# The description is optional and is solely for the reader's benefit. It -# isn't used in creating the CIPD package. -description: A sample CIPD dependency. - -# The root is optional and, if unspecified, defaults to ".". It specifies the -# root directory of the files and directories specified below in "data". -# -# You won't typically need to specify this explicitly. -root: "." - -# The install mode is optional. If provided, it specifies how CIPD should -# install a package: "copy", which will copy the contents of the package -# to the installation directory; and "symlink", which will create symlinks -# to the contents of the package in the CIPD root inside the installation -# directory. -# -# You won't typically need to specify this explicitly. -install_mode: "symlink" - -# The data is required and described what should be included in the CIPD -# package. -data: - # Data can include directories, files, or a version file. - - - dir: "directory_name" - - # Directories can include an optional "exclude" list of regexes. - # Files or directories within the given directory that match any of - # the provided regexes will not be included in the CIPD package. - exclude: - - .*\.pyc - - exclude_me - - keep_this/but_not_this - - - file: keep_this_file.bin - - # If included, CIPD will dump package version information to this path - # at package installation. - - version_file: CIPD_VERSION.json -``` - -For example, for `sample_cipd_dep`, we might write the following .yaml file: - -``` -package: chromium/third_party/sample_cipd_dep -description: A sample CIPD dependency. -data: - - file: bar.jar - - file: foo.jar -``` - -For more information about the package definition spec, see [the code]. - -> **Note:** To make the package private (Googler-only), prefix the package name -> with `chrome_internal`. For example: -> ``` -> package: chrome_internal/third_party/sample_cipd_dep -> ``` - -[the code]: https://chromium.googlesource.com/infra/luci/luci-go/+/master/cipd/client/cipd/builder/pkgdef.go - -### 4. Create your CIPD package - -To actually create your package, you'll need: - - - the `cipd.yaml` file (described above) - - [permission](#permissions-in-cipd). - -Once you have those, you can create your package like so: - -``` -# Assuming that the third-party dependency in question is at version 1.2.3 -# and this is the first chromium revision of that version. -$ cipd auth-login # One-time auth. -$ cipd create --pkg-def cipd.yaml -... -[P114210 10:14:17.215 client.go:931 I] cipd: instance -chromium/third_party/sample_cipd_dep:TX7HeY1_1JLwFVx-xiETOpT8YK4W5CbyO26SpmaMA0IC was -successfully registered -``` - -Take note of the instance ID printed in the log -(`TX7HeY1_1JLwFVx-xiETOpT8YK4W5CbyO26SpmaMA0IC` in the example above). -You'll be adding it to DEPS momentarily. - -### 5. Add your CIPD package to DEPS - -You can add your package to `DEPS` by adding an entry of the following form to -the `deps` dict: - -``` -deps = { - # ... - - # This is the installation directory. - 'src/third_party/sample_cipd_dep': { - - # In this example, we're only installing one package in this location, - # but installing multiple package in a location is supported. - 'packages': [ - { - 'package': 'chromium/third_party/sample_cipd_dep', - 'version': 'TX7HeY1_1JLwFVx-xiETOpT8YK4W5CbyO26SpmaMA0IC', - }, - ], - - # As with git-based DEPS entries, 'condition' is optional. - 'condition': 'checkout_android', - 'dep_type': 'cipd', - }, - - # ... -} -``` - -This will result in CIPD package `chromium/third_party/sample_cipd_dep` at -`TX7HeY1_1JLwFVx-xiETOpT8YK4W5CbyO26SpmaMA0IC` being installed in -`src/third_party/sample_cipd_dep` (relative to the gclient root directory). - -## Updating a CIPD dependency - -To modify a CIPD dependency, follow steps 2, 3, and 4 above, then modify the -version listed in DEPS. - -## Miscellaneous - -### Permissions in CIPD - -You can check a package's ACLs with `cipd acl-list`: - -``` -$ cipd acl-list chromium/third_party/sample_cipd_dep -... -``` - -Permissions in CIPD are handled hierarchically. You can check entries higher -in the package hierarcy with `cipd acl-list`, too: - -``` -$ cipd acl-list chromium -... -``` - -By default, [cria/project-chromium-cipd-owners][cria] own all CIPD packages -under `chromium/`. If you're adding a package, talk to one of them. - -To obtain write access to a new package, ask an owner to run: - -``` -$ cipd acl-edit chromium/third_party/sample_cipd_dep -owner user:email@address.com -``` - -[cria]: https://chrome-infra-auth.appspot.com/auth/groups/project-chromium-cipd-owners - -## Troubleshooting - - - **A file maintained by CIPD is missing, and gclient sync doesn't recreate it.** - -CIPD currently caches installation state. Modifying packages managed by CIPD -will invalidate this cache in a way that CIPD doesn't detect - i.e., CIPD will -assume that anything it installed is still installed, even if you deleted it. -To clear the cache and force a full reinstallation, delete your -`$GCLIENT_ROOT/.cipd` directory. - -Note that there is a [bug](https://crbug.com/794764) on file to add a mode to CIPD -that is not so trusting of its own cache. diff --git a/chromium/docs/cipd_and_3pp.md b/chromium/docs/cipd_and_3pp.md new file mode 100644 index 00000000000..8a3b0decae8 --- /dev/null +++ b/chromium/docs/cipd_and_3pp.md @@ -0,0 +1,399 @@ +# CIPD and 3pp for chromium dependencies + +[TOC] + +## What is CIPD? +* CIPD stands for "Chrome Infrastructure Package Deployment". +* Its code and docs [live within the luci-go project][CIPD]. +* Chromium uses CIPD to avoid checking large binary files into git, which git + does not handle well. +* gclient supports CIPD packages in the same way as git repositories. They are + specified in [DEPS] and updated via `gclient sync`. +* You can [browse Chromium's CIPD repository][browse] online. + +[CIPD]: https://chromium.googlesource.com/infra/luci/luci-go/+/master/cipd/README.md +[DEPS]: /DEPS +[browse]: https://chrome-infra-packages.appspot.com/p/chromium + +## What is 3pp? +* 3pp stands for "Third Party Packages" which allows uniform cross-compiliation, + version tracking and archival for third-party software packages for + distribution via CIPD. +* The code and docs [live within the recipe module support_3pp][support_3pp]. +* By specifying a 3pp package, you can define how to build certain artifacts and + where to upload to CIPD. Then our packagers will do the rest for you. + +[support_3pp]: https://chromium.googlesource.com/infra/infra/+/master/recipes/README.recipes.md#recipe_modules-support_3pp + +## Why use CIPD & 3pp?: + +* CIPD is our solution to storing large binary blobs directly in git, which git + is not good at. +* Building these packages with 3pp is beneficial because: + * It makes how each binary file was created clear and verifiable. + * It avoids the need for maintaining a list of ACLs for uploading to CIPD. + +## Adding a new 3pp package + +### 1. Set up a new directory for your dependency + +You'll first want somewhere in the repository in which your dependency will +live. For third-party dependencies, this should typically be a subdirectory +of `//third_party`. You'll need to add the same set of things to that +directory that you'd add for a non-CIPD dependency -- OWNERS, README.chromium, +etc. + +For example, if you want to add a package named `sample_cipd_dep`, you might +create the following: + +``` + third_party/ + sample_cipd_dep/ + LICENSE + OWNERS + README.chromium +``` + +For more on third-party dependencies, see [adding_to_third_party.md]. + +[adding_to_third_party.md]: /docs/adding_to_third_party.md + +### 2. Set up the 3pp subdirectory + +The 3pp subdirectory will store all the 3pp related files, including a 3pp spec +(`3pp.pb`), as well as scripts, patches and/or tools to build the software +from source. It should be placed directly under the package directory. + +Staying with the example from above, the `sample_cipd_dep` directory may be +like the following. + +> Note that among the files in 3pp subdirectory, the `3pp.pb` is always +> required. The rest are optional, depending on how the `3pp.pb` is specified. + +``` + third_party/ + sample_cipd_dep/ + LICENSE + OWNERS + README.chromium + 3pp/ + 3pp.pb # REQUIRED + bootstrap.py + fetch.py + install.sh + install_win.sh + patches/ + 0001-foo.patch +``` + +#### 2.1 The file `3pp.pb` (Required) + +`3pp.pb` is a text proto specified by the **[`spec.proto`]** schema. It is broken up +into two main sections: +* `create`: allows you to specify how the package software gets created, and + allows specifying differences in how it's fetched/built/tested on a + per-target basis. See [here][doc_create] for more details. +* `upload`: contains some details on how the final result gets uploaded to CIPD. + See [here][doc_upload] for more details. + +[`spec.proto`]: https://chromium.googlesource.com/infra/infra/+/master/recipes/recipe_modules/support_3pp/spec.proto +[doc_create]: https://chromium.googlesource.com/infra/infra/+/master/recipes/README.recipes.md#creation-stages +[doc_upload]: https://chromium.googlesource.com/infra/infra/+/master/recipes/README.recipes.md#upload + +Staying with the example from above, the file `sample_cipd_dep/3pp/3pp.pb` may +be like the following: + +``` +create { + source { + url { + download_url: "https://some_url_link/foo.zip" + version: "1.0.0" + extension: ".zip" + } + patch_version: "cr0" + unpack_archive: true + } +} + +upload { + pkg_prefix: "tools" + universal: true +} +``` + +While the above example could meet most of the use case, `3pp.pb` is capable of +handling more complicated use case like the following: + +``` +# create section that is shared by linux-.* and mac-.* platforms +create { + platform_re: "linux-.*|mac-.*" + source { + git { + repo: "<one_git_repo>" + tag_pattern: "v%s", + + # Fixed to 3.8.x for now. + version_restriction: { op: LT val: "3.9a0"} + } + patch_dir: "patches" + } + build { + # Can also leave as blank since the script name defaults to "install.sh" + install: "install.sh" + } +} + +# create section that is specific to linux-.* platforms +create { + platform_re: "linux-.*" + build { + dep: "<dep_foo>" + dep: "<dep_bar>" + + tool: "<tool_foo>" + } +} + +# create section that is specific to linux-arm.* and linux-mips.* platforms +create { + platform_re: "linux-arm.*|linux-mips.*" + build { + tool: "<tool_bar>" + } +} + +# create section that is specific to windows-* +create { + platform_re: "windows-.*" + source { script { name: "fetch.py" } } + build { + install: "install_win.sh" + } +} + +upload { pkg_prefix: "tools" } +``` + +#### 2.2 The file `install.sh` (Optional) + +When the `build` message is specified in `3pp.pb`, the file specified in +"build.install" (Default to "install.sh") will be run to transform the source +into the built product. + +Staying with the example from above, the file `sample_cipd_dep/3pp/install.sh` +may be like the following. + +> Note that during the build stage, the 3pp directory and all its dependent 3pp +> directories (i.e. the `tool` and `dep` from the `build` message in `3pp.pb`) +> will be [copied to a different directory]. So commands in `install.sh` should +> not refer to files that are outside of these directories. + +[copied to a different directory]: https://chromium.googlesource.com/infra/infra/+/53fd7d1eda2010009ed00fdc1a7b59fe5034ae0c/recipes/recipe_modules/support_3pp/source.py#246 + +``` +#!/bin/bash +# Copyright 2021 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +set -e +set -x +set -o pipefail + +# An auto-created directory whose content will ultimately be uploaded to CIPD. +# So the commands below should output the built product to this directory. +PREFIX="$1" + +# Commands to transform the source into the built product and move it to $PREFIX +./configure +make install + +SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )" +cp -a bin_foo "$SCRIPT_DIR/bootstrap.py" "$PREFIX" +``` + +#### 2.3 The file `fetch.py` (Optional) + +When specifying the `source` in 3pp.pb, it is possible to use a custom catch-all +script to probe for the latest version and obtain the latest sources. A simple +example can be like the following: + +``` +#!/usr/bin/env python +# Copyright 2021 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +from __future__ import print_function + +import argparse +import json +import os +import urllib + + +def do_latest(): + print(urllib.urlopen('some_url/master/VERSION').read().strip()) + + +def get_download_url(version, platform): + target_os, target_arch = platform.split('-') + ext = '.zip' if target_os == 'windows' else '.tar.gz' + partial_manifest = { + 'url': ['download_url1', 'download_url2'], + 'ext': ext, + } + print(json.dumps(partial_manifest)) + + +def main(): + ap = argparse.ArgumentParser() + sub = ap.add_subparsers() + + latest = sub.add_parser("latest") + latest.set_defaults(func=lambda _opts: do_latest()) + + download = sub.add_parser("get_url") + download.set_defaults( + func=lambda _opts: get_download_url( + os.environ['_3PP_VERSION'], os.environ['_3PP_PLATFORM'] + ) + ) + + opts = ap.parse_args() + opts.func(opts) + + +if __name__ == '__main__': + main() +``` + +### 3. Add "3pp" subdirectory to `chromium/src` repo + +#### 3pp CQ builders (Presubmit) + +The following are the optional CQ builders to run the presubmit check for CLs +that have the directory "3pp" in the patchset. + +* [3pp-linux-amd64-packager](https://ci.chromium.org/p/chromium/builders/try/3pp-linux-amd64-packager): + For builds on the linux-amd64 platform or universal builds (e.g. to be used by + all platforms) + +#### 3pp CI builders (Postsubmit) + +Once the CLs pass the CQ and get landed, the following CI builders will +periodically build all the 3pp packages that match the given platforms and +upload any new results to CIPD. + +* [3pp-linux-amd64-packager](https://ci.chromium.org/p/chromium/builders/ci/3pp-linux-amd64-packager): + For builds on the linux-amd64 platform or universal builds (e.g. to be used by + all platforms) + +### 4. Add your CIPD package to DEPS + +Once your CIPD package is created by the 3pp CI builders, you can add it to +`DEPS` by adding an entry of the following form to the `deps` dict: + +``` +deps = { + # ... + + # This is the installation directory. + 'src/third_party/sample_cipd_dep': { + + # In this example, we're only installing one package in this location, + # but installing multiple package in a location is supported. + 'packages': [ + { + 'package': 'chromium/third_party/sample_cipd_dep', + 'version': 'TX7HeY1_1JLwFVx-xiETOpT8YK4W5CbyO26SpmaMA0IC', + }, + ], + + # As with git-based DEPS entries, 'condition' is optional. + 'condition': 'checkout_android', + 'dep_type': 'cipd', + }, + + # ... +} +``` + +This will result in CIPD package `chromium/third_party/sample_cipd_dep` at +`TX7HeY1_1JLwFVx-xiETOpT8YK4W5CbyO26SpmaMA0IC` being installed in +`src/third_party/sample_cipd_dep` (relative to the gclient root directory). + +## Updating a CIPD dependency + +To modify a CIPD dependency, follow steps 2 and 3 above, then modify the +version listed in DEPS. + +## Miscellaneous + +### Create a cipd.yaml file in the old way + +While it is strongly suggested to use 3pp infrastructure, there are existing flows +that create a cipd.yaml file by a GN template or a script, and upload it to CIPD +by builders with custom recipes. + +Examples are: +* [android-androidx-packager](https://ci.chromium.org/p/chromium/builders/ci/android-androidx-packager) +* [android-sdk-packager](https://ci.chromium.org/p/chromium/builders/ci/android-sdk-packager) + +#### Generating cipd.yaml via GN Template: +The `cipd_package_definition` template in [build/cipd/cipd.gni] can be used to +create the yaml definition as part of Chromium's normal build process. Declare +a target like: +``` +cipd_package_definition("my_cipd_package") { + package = "path/to/cipd/package" + description = "Prebuilt test binary." + install_mode = "copy" + deps = [ "//path/to:test_binary_target" ] + sources = [ "//path/to:test_binary_file" ] +} +``` +[build/cipd/cipd.gni]: https://source.chromium.org/chromium/chromium/src/+/master:build/cipd/cipd.gni + +### Permissions in CIPD + +You can check a package's ACLs with `cipd acl-list`: + +``` +$ cipd acl-list chromium/third_party/sample_cipd_dep +... +``` + +Permissions in CIPD are handled hierarchically. You can check entries higher +in the package hierarchy with `cipd acl-list`, too: + +``` +$ cipd acl-list chromium +... +``` + +By default, [cria/project-chromium-cipd-owners][cria] own all CIPD packages +under `chromium/`. If you're adding a package, talk to one of them. + +To obtain write access to a new package, ask an owner to run: + +``` +$ cipd acl-edit chromium/third_party/sample_cipd_dep -owner user:email@address.com +``` + +[cria]: https://chrome-infra-auth.appspot.com/auth/groups/project-chromium-cipd-owners + +## Troubleshooting + + - **A file maintained by CIPD is missing, and gclient sync doesn't recreate it.** + +CIPD currently caches installation state. Modifying packages managed by CIPD +will invalidate this cache in a way that CIPD doesn't detect - i.e., CIPD will +assume that anything it installed is still installed, even if you deleted it. +To clear the cache and force a full reinstallation, delete your +`$GCLIENT_ROOT/.cipd` directory. + +Note that there is a [bug](https://crbug.com/1176408) on file where +`gclient sync` does not reset CIPD entries that are changed locally. diff --git a/chromium/docs/clang.md b/chromium/docs/clang.md index 259560c172d..ce7cca8152e 100644 --- a/chromium/docs/clang.md +++ b/chromium/docs/clang.md @@ -88,3 +88,15 @@ Chromium tries to be buildable with its currently pinned clang, and with clang trunk. Set `llvm_force_head_revision = true` in your args.gn if the clang you're trying to build with is closer to clang trunk than to Chromium's pinned clang (which `tools/clang/scripts/update.py --print-revision` prints). + +## Related documents + +* [Toolchain support](toolchain_support.md) gives an overview of clang + rolls, and documents when to revert clang rolls and how to file good + toolchain bugs. + +* [Updating clang](updating_clang.md) documents the mechanics of updating clang, + and which files are included in the default clang package. + +* [Clang Sheriffing](clang_sheriffing.md) contains instructions for how to debug + compiler bugs, for clang sheriffs. diff --git a/chromium/docs/clang_sheriffing.md b/chromium/docs/clang_sheriffing.md index 4cf52f372b5..ad9b905b8d0 100644 --- a/chromium/docs/clang_sheriffing.md +++ b/chromium/docs/clang_sheriffing.md @@ -14,19 +14,19 @@ determining if any compile or test failures are due to an upstream compiler change, filing bugs upstream, and often reverting bad changes in LLVM. This document describes some of the processes and techniques for doing that. -Some may find the [sheriff-o-matic] -(https://sheriff-o-matic.appspot.com/chromium.clang) view of the waterfall -easier to work with. +Some may find the +[sheriff-o-matic](https://sheriff-o-matic.appspot.com/chromium.clang) +view of the waterfall easier to work with. -To keep others informed, [file a bug] -(https://bugs.chromium.org/p/chromium/issues/entry) . +To keep others informed, [file a +bug](https://bugs.chromium.org/p/chromium/issues/entry). earlier rather than later for build breaks likely caused by changes in clang or the rest fo the toolchain. Make sure to set the component field to `Tools > LLVM`, which will include the entire Chrome toolchain (Lexan) team. At the beginning of your sheriff rotation, it may be -useful to [search for recent bot breaks] -(https://bugs.chromium.org/p/chromium/issues/list?q=component%3ATools%3ELLVM&can=2&sort=-modified). +useful to [search for recent bot +breaks](https://bugs.chromium.org/p/chromium/issues/list?q=component%3ATools%3ELLVM&can=2&sort=-modified). We prefer searching like this to having sheriffs compose status email at the end of their week. @@ -91,8 +91,13 @@ and what to do about them: This is probably the most common bug. The standard procedure is to do these things: -1. Use `got_clang_revision` property from first red and last green build to find - upstream regression range +1. Open the `gclient runhooks` stdout log from the first red build. Near the + top of that log you can find the range of upstream llvm revisions. For + example: + + From https://github.com/llvm/llvm-project + f917356f9ce..292e898c16d master -> origin/master + 1. File a crbug documenting the crash. Include the range, and any other bots displaying the same symptoms. 1. All clang crashes on the Chromium bots are automatically uploaded to @@ -201,7 +206,7 @@ To use `ld.lld`'s `--reproduce` flag, follow these steps: 1. Copy the link command that ninja prints, `cd out/gn`, paste it, and manually append `-Wl,--reproduce,repro.tar`. With `lld-link`, instead append - `/linkrepro:repro.tar`. (`ld.lld` is invoked through the `clang` driver, so + `/reproduce:repro.tar`. (`ld.lld` is invoked through the `clang` driver, so it needs `-Wl` to pass the flag through to the linker. `lld-link` is called directly, so the flag needs no prefix.) diff --git a/chromium/docs/clangd.md b/chromium/docs/clangd.md index 861ae299495..4a776920342 100644 --- a/chromium/docs/clangd.md +++ b/chromium/docs/clangd.md @@ -6,6 +6,19 @@ It brings IDE features (e.g. diagnostics, code completion, code navigations) to your editor. +## Quick Start + +* **Googlers**: clangd weekly is available by default on glinux + (`/usr/bin/clangd`) +* Make sure generated ninja files are up-to-date +* Optional: build chrome normally to get generated headers +* Generate compilation database (note: it's not regenerated automatically): +``` +tools/clang/scripts/generate_compdb.py -p out/<build> > compile_commands.json +``` +* Indexing is enabled by default (since clangd 9) +* Use clangd in your favourite editor + ## Getting clangd See [instructions](https://clang.llvm.org/extra/clangd/Installation.html#installing-clangd). @@ -65,19 +78,23 @@ ninja -C out/Release chrome 4. Use clangd in your favourite editor, see detailed [instructions]( https://clang.llvm.org/extra/clangd/Installation.html#getting-started-with-clangd). -## Index +## Background Indexing -By default, clangd only knows the files you are currently editing. To provide -project-wide code navigations (e.g. find references), clangd neesds a -project-wide index. +clangd builds an incremental index of your project (all files listed in the +compilation database). The index improves code navigation features +(go-to-definition, find-references) and code completion. -You can pass an **experimental** `--background-index` command line argument to -clangd, clangd will incrementally build an index of Chromium in the background. -Note: the first index time may take hours (for reference, it took 2~3 hours on -a 48-core, 64GB machine). +* clangd only uses idle cores to build the index, you can limit the total amount + of cores by passing the *-j=\<number\>* flag; +* the index is saved to the `.clangd/index` in the project root; index shards + for common headers e.g. STL will be stored in *$HOME/.clangd/index*; +* background indexing can be disabled by the `--background-index=false` flag; + Note that, disabling background-index will limit clangd’s knowledge about your + codebase to files you are currently editing. -A full index of Chromium (including v8, blink) takes ~550 MB disk space and ~2.7 -GB memory in clangd. +Note: the first index time may take hours (for reference, it took 2~3 hours on +a 48-core, 64GB machine). A full index of Chromium (including v8, blink) takes +~550 MB disk space and ~2.7 GB memory in clangd. ## Questions diff --git a/chromium/docs/code_reviews.md b/chromium/docs/code_reviews.md index ecfe7495897..de612276055 100644 --- a/chromium/docs/code_reviews.md +++ b/chromium/docs/code_reviews.md @@ -15,7 +15,7 @@ touching. Any committer can review code, but an owner must provide a review for each directory you are touching. If you have doubts, look at the git blame for the file and the `OWNERS` files (see below). -To indicate a positive review, the reviewer provides a "Code-Review +1" in +To indicate a positive review, the reviewer provides a `Code-Review +1` in Gerrit, also known as an LGTM ("Looks Good To Me"). A score of "-1" indicates the change should not be submitted as-is. @@ -166,102 +166,43 @@ per-file *_messages*.h=set noparent per-file *_messages*.h=file://ipc/SECURITY_OWNERS ``` -## TBR ("To Be Reviewed") +### Owners-Override -"TBR" is our mechanism for post-commit review. It should be used rarely and -only in cases where a normal review is unnecessary, as described under -"When to TBR", below. +Setting the `Owners-Override +1` label will bypass OWNERS enforcement. Active +sheriffs, Large Scale Changes and Global Approvers (see below) reviewers have +this capability. -TBR does not mean "no review." A reviewer TBR-ed on a change should still -review the change. If there are comments after landing, the author is obligated -to address them in a followup patch. +## Mechanical changes -Do not use TBR just because a change is urgent or the reviewer is being slow. -Contact the reviewer directly or find somebody else to review your change. +### Large Scale Changes +You can use the [Large Scale Changes](process/lsc/large_scale_changes.md) +process to get approval to bypass OWNERS enforcement for large changes like +refactoring, architectural changes, or other repetitive code changes across the +whole codebase. This is used for work that span many dozen CLs. -### How to TBR +### Global Approvals +For one-off CLs API owners of base, blink, build, content and url can +Owners-Override +1 a change to their APIs to avoid waiting for rubberstamp +1s +from affected directories' owners. This should only be used for mechanical +updates to the affected directories. -To send a change TBR, annotate the description and send email like normal. -Otherwise the reviewer won't know to review the patch. +## Documentation updates - * Add the reviewer's email address in the code review tool's reviewer field - like normal. +Documentation updates require code review. We may revisit this decision in the +future. - * Add a line "Tbr: <reviewer's email>" to the bottom of the change list - description. e.g. `Tbr: reviewer1@chromium.org,reviewer2@chromium.org` +## Automated code-review - * Type a message so that the owners in the TBR list can understand who is - responsible for reviewing what, as part of their post-commit review - responsibility. e.g. - ``` - TBRing reviewers: - reviewer1: Please review changes to foo/ - reviewer2: Please review changes to bar/ - ``` +For verifiably safe changes like translation files, clean reverts, and clean +cherry-picks, we have automation that will vote +1 on the `Bot-Commit` label +allowing the CL to be submitted without human code-review. Add `Rubber Stamper` +(rubber-stamper@appspot.gserviceaccount.com) to your CL as a reviewer to +activate this automation. It will scan the CL after about 1 minute and reply +with its verdict. `Bot-Commit` votes are not sticky between patchsets and so +only add the bot once the CL is finalized. -### When to TBR +When combined with the [`Owners-Override`](#owners_override) power discussed +above, sheriffs can effectively revert and reland on their own. -#### Reverts and relands - -The most common use of TBR is to revert patches that broke the build. Clean -reverts of recent patches may be submitted TBR. However, TBR should not be used -if the revert required non-trivial conflict resolution, or if the patch being -reverted is older than a few days. - -A developer relanding a patch can TBR the OWNERS for changes which are identical -to the original (reverted) patch. If the reland patch contains any new changes -(such as bug fixes) on top of the original, those changes should go through the -normal review process. - -When creating a reland patch, you should first upload an up-to-date patchset -with the exact content of the original (reverted) patch, and then upload the -patchset to be relanded. This is important for the reviewers to understand what -the fix for relanding was. - -#### Mechanical changes - -You can use TBR with certain mechanical changes that affect many callers in -different directories. For example, adding a parameter to a common function in -`//base`, with callers in `//chrome/browser/foo`, `//net/bar`, and many other -directories. If the updates to the callers is mechanical, you can: - - 1. Get a normal owner of the lower-level code you're changing (in this - example, the function in `//base`) to do a proper review of those changes. - - 2. Get _somebody_ to review the downstream changes made to the callers as a - result of the `//base` change. This is often the same person from the - previous step but could be somebody else. - - 3. TBR the owners of the calling code, after the API change is LGTM'ed. - -This process ensures that all code is reviewed prior to checkin and that the -concept of the change is reviewed by a qualified person, without having to ping -many owners with little say in the trivial side-effects they incur. - -**Note:** The above policy is only viable for strictly mechanical changes. For -large-scale scripted changes you should: - - 1. Have an owner of the core change review the script. - - 2. Use `git cl split` to shard the large change into many small CLs with a - clear description of what each reviewer is expected to verify - ([example](https://chromium-review.googlesource.com/1191225)). - -#### Documentation updates - -You can TBR documentation updates. Documentation means markdown files, text -documents, and high-level comments in code. At finer levels of detail, comments -in source files become more like code and should be reviewed normally (not -using TBR). Non-TBR-able stuff includes things like function contracts and most -comments inside functions. - - * Use good judgement. If you're changing something very important, tricky, - or something you may not be very familiar with, ask for the code review - up-front. - - * Don't TBR changes to policy documents like the style guide or this document. - - * Don't mix unrelated documentation updates with code changes. - - * Be sure to actually send out the email for the code review. If you get one, - please actually read the changes. +Changes not supported by Rubber Stamper still need a +1 from another +committer. diff --git a/chromium/docs/commit_checklist.md b/chromium/docs/commit_checklist.md index 6ddeef93046..fc801410d91 100644 --- a/chromium/docs/commit_checklist.md +++ b/chromium/docs/commit_checklist.md @@ -190,7 +190,14 @@ your CL touches. For your CL to land, you need an approval from an owner for each file you've changed, unless you are an owner of some files, in which case you don't need separate owner approval for those files. -## 17. Implement feedback from your reviewers +## 17. Start Your Review + +Click on the `Start Review` button to begin the actual review process. Until +you press this button, nobody will look at your change. Once pressed, you'll +have the opportunity to include an additional message in the notification sent +to your reviewers. + +## 18. Implement feedback from your reviewers Then go through this commit checklist again. Reply to all comments from the reviewers on Gerrit and mark all resolved issues as resolved (clicking `Done` or @@ -205,7 +212,7 @@ to the next step automatically after approval. This feature is useful if your reviewer is in a different time zone and you want to land the CL sooner. Setting this flag also puts the onus on your reviewer to land the CL. -## 18. Land your CL +## 19. Land your CL Once you have obtained a Looks Good To Me (LGTM), which is reflected by a Code-Review+1 in Gerrit, from at least one owner for each file, then you have @@ -222,7 +229,7 @@ unnoticed during the code review process. Consider monitoring the the Sheriff or anyone else brings any failures to your attention, revert the CL first and ask questions later. Gerrit can automatically generate revert CLs. -## 19. Cleanup +## 20. Cleanup After your CL is landed, you can use `git rebase-update` or `git cl archive` to clean up your local branches. These commands will automatically delete merged diff --git a/chromium/docs/design/sandbox.md b/chromium/docs/design/sandbox.md index d3a62823b76..ef7e9d79677 100644 --- a/chromium/docs/design/sandbox.md +++ b/chromium/docs/design/sandbox.md @@ -364,8 +364,8 @@ policies on the target process for enforcing security characteristics. #### CET Shadow Stack: -* Only in Insider Builds of Windows 10 yet. -* It's being evaluated and not enabled for any processes. See +* Available in Windows 10 2004 December Update. +* Is not enabled in the renderer. See [ticket](https://bugs.chromium.org/p/chromium/issues/detail?id=1136224), [MSDN](https://docs.microsoft.com/en-us/cpp/build/reference/cetcompat?view=vs-2019). diff --git a/chromium/docs/enterprise/description_guidelines.md b/chromium/docs/enterprise/description_guidelines.md index 9dd5890bbe3..6e21953c5f1 100644 --- a/chromium/docs/enterprise/description_guidelines.md +++ b/chromium/docs/enterprise/description_guidelines.md @@ -3,12 +3,17 @@ how various product names and the like should be referenced. * Chrome: `<ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph>` * Chrome OS: `<ph name="PRODUCT_OS_NAME">$2<ex>Google Chrome OS</ex></ph>` -* Chrome Browser Cloud Management: `<ph name="CHROME_BROWSER_CLOUSE_MANAGEMENT_NAME">Chrome Browser Cloud Management</ph>` +* Chrome Browser Cloud Management: `<ph name="CHROME_BROWSER_CLOUD_MANAGEMENT_NAME">Chrome Browser Cloud Management</ph>` +* Chrome Cleanup: `<ph name="CHROME_CLEANUP_NAME">Chrome Cleanup</ph>` * Linux: `<ph name="LINUX_OS_NAME">Linux</ph>` * Internet Explorer: `<ph name="IE_PRODUCT_NAME">Internet® Explorer®</ph>` * Google Admin console: `<ph name="GOOGLE_ADMIN_CONSOLE_PRODUCT_NAME">Google Admin console</ph>` * Google Cast: `<ph name="PRODUCT_NAME">Google Cast</ph>` +* Google Cloud Print: `<ph name="CLOUD_PRINT_NAME">Google Cloud Print</ph>` * Google Drive: `<ph name="GOOGLE_DRIVE_NAME">Google Drive</ph>` +* Lacros: `<ph name="LACROS_NAME">Lacros</ph>` +* Android: `<ph name="ANDROID_NAME">Android</ph>` * macOS: `<ph name="MAC_OS_NAME">macOS</ph>` +* iOS: `<ph name="IOS_NAME">iOS</ph>` * Windows: `<ph name="MS_WIN_NAME">Microsoft® Windows®</ph>` * Microsoft ActiveDirectory: `<ph name="MS_AD_NAME">Microsoft® Active Directory®</ph>` diff --git a/chromium/docs/enterprise/policies.md b/chromium/docs/enterprise/policies.md index b774df633c0..abeed738c6c 100644 --- a/chromium/docs/enterprise/policies.md +++ b/chromium/docs/enterprise/policies.md @@ -65,8 +65,8 @@ which users can sign in on the device. Implementation-wise, these policies can have complex structure, and are usually accessed via -[DeviceSettingsProvider](https://cs.chromium.org/chromium/src/chrome/browser/chromeos/settings/device_settings_provider.h) -or its wrapper [CrosSettings](https://cs.chromium.org/chromium/src/chrome/browser/chromeos/settings/cros_settings.h). +[DeviceSettingsProvider](https://cs.chromium.org/chromium/src/chrome/browser/ash/settings/device_settings_provider.h) +or its wrapper [CrosSettings](https://cs.chromium.org/chromium/src/chrome/browser/ash/settings/cros_settings.h). ## User policies diff --git a/chromium/docs/fuchsia_build_instructions.md b/chromium/docs/fuchsia/build_instructions.md index ba27e01372f..0b37b748b59 100644 --- a/chromium/docs/fuchsia_build_instructions.md +++ b/chromium/docs/fuchsia/build_instructions.md @@ -201,54 +201,10 @@ $ exit ### Running test suites -Building test suites generate a launcher script to run them on an emulator -or a physical device. These scripts are generated at `out/fuchsia/bin`. For -instance,to run the `base_unittests` target, launch: +There are three types of tests available to run on Fuchsia: -```shell -$ out/fuchsia/bin/run_base_unittests -``` - -Common gtest arguments such as `--gtest_filter=...` are supported by the run -script. The launcher script also symbolizes backtraces. - -The test suite, by default, will run on QEMU. AEMU can be used for running -tests that interact with Fuchsia's window manager, Scenic. To change the device -that Fuchsia will run on, use `--device={aemu|qemu|device}`. - -To run a test suite on an *unprovisioned device* in a zedboot state, simply add -`-d` to the test runner script arguments. Subsequent runs of the test runner -script will be able to pick up the same device. - -To run a test suite on a device that is *already provisioned*, add the following -arguments to the test runner script: - -* `-d` to run the test suite on a device. -* `--fuchsia-out-dir=[/path/to/fuchsia/out/directory]` or - `--ssh-config=[/path/to/ssh_config]` to specify the SSH configuration used for - connecting to the target device. -* (Optional) `--host=[IP]` to specify the test device IP. Typically, this is the -value obtained from the command `fx netaddr --fuchsia`. -* (Optional) `--port=[port]` to specify the SSH port, if different from 22. - -### Troubleshooting a test - -To troubleshoot a specific test, consider a combination of the following -arguments to the test runner script: - -* `--gtest_filter="[TestSuite.TestName]"` to only run a specific test, or a - comma-separated list to run a set of tests. Wildcards can also be used to run - a set of tests or an entire test suite, e.g. `--gtest_filter="[TestSuite.*]"`. -* `--test-launcher-jobs=1` to only have one batch of tests running at a time. - This bypasses the test launcher buffering of test log output, making it - possible to access the log output from successful test runs. -* `--single-process-tests` to run all the tests in the same process. Unlike the - above option, this will run the tests directly in the test launcher process, - making it easier to attach a debugger. -* `system-log-file=[/path/to/syslog]` to specify the file to write system logs - to. Or `system-log-file=-` to write the system logs to stdout. By default, - Chromium logs are written to the system log on Fuchsia. This argument is known - to cause `IOError` python exceptions with a QEMU target. -* `--gtest_repeat=[number] --gtest_break_on_failure` to run a test or test suite - a certain number of times until it fails. This is useful to investigate flaky - tests. +1. [Gtests](gtests.md) +2. [GPU integration tests](gpu_testing.md) +3. [Blink tests](web_tests.md) + +Check the documentations to learn more about how to run these tests. diff --git a/chromium/docs/fuchsia/debug_instructions.md b/chromium/docs/fuchsia/debug_instructions.md new file mode 100644 index 00000000000..264332bbc5d --- /dev/null +++ b/chromium/docs/fuchsia/debug_instructions.md @@ -0,0 +1,49 @@ +# Debugging + +It is possible to debug Fuchsia binaries using `zxdb`. For the sake of this +example, we will be using `base_unittests` as the test suite we wish to execute: + +1. (From Chromium) Install your package(s) and its symbols onto the device. + + ```bash + $ out/fuchsia/bin/install_base_unittests + ``` + +2. (From Fuchsia source tree) Run the debugger. + + ```bash + $ fx debug + ``` + +3. Set up the debugger to attach to the process. + + ``` + [zxdb] attach base_unittests.cmx + ``` + +4. Configure breakpoint(s). + + ``` + [zxdb] break base::GetDefaultJob + ``` + +5. (In another terminal, from Fuchsia source tree) Run the test package. + + ```bash + $ fx shell run fuchsia-pkg://fuchsia.com/base_unittests#meta/base_unittests.cmx + ``` + +6. At this point, you should hit a breakpoint in `zxdb`. + + ``` + [zxdb] f + ▶ 0 base::GetDefaultJob() • default_job.cc:18 + 1 base::$anon::LaunchChildTestProcessWithOptions(…) • test_launcher.cc:335 + 2 base::$anon::DoLaunchChildTestProcess(…) • test_launcher.cc:528 + 3 base::TestLauncher::LaunchChildGTestProcess(…) • test_launcher.cc:877 + ... + ``` + +7. Enjoy debugging! Steps 2 through 6 will also work for things like services + which aren't run directly from the command line, such as WebEngine. + Run `help` inside ZXDB to see what debugger commands are available.
\ No newline at end of file diff --git a/chromium/docs/fuchsia/gpu_testing.md b/chromium/docs/fuchsia/gpu_testing.md new file mode 100644 index 00000000000..a5bcbf3fc6d --- /dev/null +++ b/chromium/docs/fuchsia/gpu_testing.md @@ -0,0 +1,45 @@ +# Running GPU integration tests on Fuchsia. + +General instruction on running and debugging GPU integration tests can be +found [here](../gpu/gpu_testing.md). + +Fuchsia uses [web_engine_shell](../../fuchsia/engine/test/README.md) to run GPU +integration tests. For the sake of this example, we will be using `gpu_process` +as the test suite we wish to execute. Build the target +`fuchsia_telemetry_gpu_integration_test` and run the appropriate commands: + +## Hermetic emulation + +The test script brings up an emulator, runs the tests on it, and shuts the +emulator down when finished. + +```bash +$ content/test/gpu/run_gpu_integration_test_fuchsia.py gpu_process +--browser=web-engine-shell --out-dir=/path/to/outdir +``` + +## Run on an physical device + +```bash +$ content/test/gpu/run_gpu_integration_test_fuchsia.py gpu_process +--browser=web-engine-shell --out-dir=/path/to/outdir -d +``` + +## Run on a device paved with Fuchsia built from source + +```bash +$ content/test/gpu/run_gpu_integration_test_fuchsia.py gpu_process +--browser=web-engine-shell --out-dir=/path/to/outdir -d +--fuchsia-out-dir=/path/to/fuchsia/outdir +``` + +## Run on a device the host is connected to remotely via ssh + +Note the `--ssh-config` flag, which should point to the config file used to set +up the connection between the host and the remote device. + +```bash +$ content/test/gpu/run_gpu_integration_test_fuchsia.py gpu_process +--browser=web-engine-shell --out-dir=/path/to/outdir -d --host=localhost +--ssh-config=/path/to/ssh/config +```
\ No newline at end of file diff --git a/chromium/docs/fuchsia/gtests.md b/chromium/docs/fuchsia/gtests.md new file mode 100644 index 00000000000..aacb9823ae8 --- /dev/null +++ b/chromium/docs/fuchsia/gtests.md @@ -0,0 +1,97 @@ +# Deploying and running gtests on Fuchsia. + +Fuchsia gtest binaries are deployed and executed via scripts that are +automatically generated by the `fuchsia_package_runner()` GN target. +The binaries can deploy to either emulators started by the runner script, +an existing emulator instance, or a physical device. To build a gtest binary, +check this [documentation](build_instructions.md). + +For the sake of this example, we will be using `base_unittests` as the package +we wish to install and/or execute. + +## Hermetic emulation + +The test script brings up an emulator, runs the tests on it, and +shuts the emulator down when finished. +```bash +$ out/fuchsia/bin/run_base_unittests +``` + +## Run on an physical device + +Note the `-d` flag, which is an alias for `--device`. + +```bash +$ out/fuchsia/bin/run_base_unittests -d +``` + +## Run on a device paved with Fuchsia built from source + +Make sure that the CPU architecture of your Chromium output directory matches +the architecture of the Fuchsia output directory (x64==x64, arm64==arm64, etc.). + +```bash +$ out/fuchsia/bin/run_base_unittests -d +--fuchsia-out-dir=/path/to/fuchsia/outdir +``` + +If you are frequently deploying to Fuchsia built from source, you might want to +add the following entry to your `args.gn`: + +``` +default_fuchsia_build_dir_for_installation = "/path/to/fuchsia/outdir" +``` + +With this flag in place, the `--fuchsia-out-dir` flag will automatically be +used whenever you `run_` or `install_` Fuchsia packages, making your command +lines much shorter: + +```bash +$ out/fuchsia/bin/run_base_unittests -d +``` + +## Install on a device running Fuchsia built from source + +```bash +$ out/fuchsia/bin/install_base_unittests +--fuchsia-out-dir=/path/to/fuchsia/outdir +``` + +You will need to run the package manually on your device. In this case, run the +following from your Fuchsia directory: + +``` +$ fx shell run fuchsia-pkg://fuchsia.com/base_unittests#meta/base_unittests.cmx +``` + +## Run on a device the host is connected to remotely via ssh + +Note the `--ssh-config` flag, which should point to the config file used to set +up the connection between the host and the remote device. + +```bash +$ out/fuchsia/bin/run_base_unittests -d +--host=localhost --ssh-config=/path/to/ssh/config +``` + +## Troubleshooting a test + +To troubleshoot a specific test, consider a combination of the following +arguments to the test runner script: + +* `--gtest_filter="[TestSuite.TestName]"` to only run a specific test, or a + comma-separated list to run a set of tests. Wildcards can also be used to run + a set of tests or an entire test suite, e.g. `--gtest_filter="[TestSuite.*]"`. +* `--test-launcher-jobs=1` to only have one batch of tests running at a time. + This bypasses the test launcher buffering of test log output, making it + possible to access the log output from successful test runs. +* `--single-process-tests` to run all the tests in the same process. Unlike the + above option, this will run the tests directly in the test launcher process, + making it easier to attach a debugger. +* `system-log-file=[/path/to/syslog]` to specify the file to write system logs + to. Or `system-log-file=-` to write the system logs to stdout. By default, + Chromium logs are written to the system log on Fuchsia. This argument is known + to cause `IOError` python exceptions with a QEMU target. +* `--gtest_repeat=[number] --gtest_break_on_failure` to run a test or test suite + a certain number of times until it fails. This is useful to investigate flaky + tests.
\ No newline at end of file diff --git a/chromium/docs/fuchsia_sdk_updates.md b/chromium/docs/fuchsia/sdk_updates.md index 6fb55301807..6fb55301807 100644 --- a/chromium/docs/fuchsia_sdk_updates.md +++ b/chromium/docs/fuchsia/sdk_updates.md diff --git a/chromium/docs/fuchsia/web_tests.md b/chromium/docs/fuchsia/web_tests.md new file mode 100644 index 00000000000..6c573b21acb --- /dev/null +++ b/chromium/docs/fuchsia/web_tests.md @@ -0,0 +1,28 @@ +# Deploying content_shell and running web_tests on Fuchsia. + +General instruction on running and debugging web_tests can be found +[here](../testing/web_tests.md). + +Currently, only +[a small subset of web tests](../../third_party/blink/web_tests/SmokeTests) +can be run on Fuchsia. Build the target `blink_tests` first before running any +of the commands below: + +## Hermetic emulation + +The test script brings up an emulator, runs the tests on it, and shuts the +emulator down when finished. +```bash +$ third_party/blink/tools/run_web_tests.py -t <output-dir> --platform=fuchsia +``` + +## Run on an physical device. + +Note the `--fuchsia-host-ip` flag, which is the ip address of the test host that +the Fuchsia device uses to establish a connection. + +```bash +$ third_party/blink/tools/run_web_tests.py -t <output-dir> --platform=fuchsia +--device=device --fuchsia-target-cpu=<device-cpu-arch> +--fuchsia-out-dir=/path/to/fuchsia/outdir --fuchsia-host-ip=test.host.ip.address +```
\ No newline at end of file diff --git a/chromium/docs/fuchsia_gardening.md b/chromium/docs/fuchsia_gardening.md deleted file mode 100644 index 63c11245f04..00000000000 --- a/chromium/docs/fuchsia_gardening.md +++ /dev/null @@ -1,40 +0,0 @@ -# Cr-Fuchsia Gardening - -## Gardener Responsibilities - -In priority order, the responsibilities of the Fuchsia Gardener are as follows (though see notes below!): - -1. **Chromium waterfall:** Keep [the Fuchsia bots on the Chromium waterfall](https://ci.chromium.org/p/chromium/g/chromium.linux/console) green. - 1. Join the #chromium IRC channel on Freenode. - 1. Not all waterfall bots have a corresponding try-bot configuration. - 1. E.g. the Cast Fuchsia bots are not run on the CQ by default. - 1. E.g. the CQ builds have DCHECKs enabled, whereas our Cast waterfall bots don't. -1. **Chromium try-bots:** Ensure that Fuchsia bots are not causing CQ flake. - 1. Join the #chromium IRC channel on Freenode. - 1. Watch for try-flakes emails & investigate any tests/suites reported flakey. - 1. Watch for IRC mentions of flakiness on the Fuchsia bots. -1. **Fuchsia SDK rolls:** Keep [the Fuchsia SDK auto-roller](https://autoroll.skia.org/r/fuchsia-sdk-chromium-autoroll) working. - 1. Watch for emailed status updates from auto-roller CLs. - 1. If a roll CL fails, check the failed bot to confirm SDK-related fail vs other flake. - 1. If it was an actual SDK-related failure, note the latest auto-roller patch-Id, and stop the auto-roller. - 1. Create a local branch e.g. with "`git checkout -b sdkRoll origin`". - 1. Pull-down the auto-roll CL with "`git cl patch <patch-Id> && gclient sync`". - 1. Clear the CL metadata with "`git cl issue 0`". - 1. Make any necessary modifications for compatibility with the new SDK. - 1. Commit the changes and run "`git cl upload`" to upload a new CL. - 1. Edit the CL description, which will include the git commit description from the auto-roller CL, making it easy to provide a consistent description. - 1. Note that if the auto-roller is blocked for a long time then it may be easier to fix the most-recent failed roll, and roll from there, to avoid having to deal with several different causes of breakage in a single roll! -1. **FYI waterfall:** Keep our bots on the [FYI waterfall](https://ci.chromium.org/p/chromium/g/chromium.fyi/console) green. - 1. FYI bots don't block the CQ, but still provide early-warning of regressions. - 1. They're also our staging-ground for bringing complex test suites to the CQ/waterfall. - -While the Gardener takes primary responsibility for each of these areas during their rotation, that does not mean that they must do all the work - if another teammate happens to have started fixing the SDK roll, un-breaking the bots, or sending you CLs to fix our Debug builder (look, ma! No try-bot!), then lucky you, your Gardening job is done. :) - -## Optional Gardener Tasks - -The waterfall is green, the try-bots reliable, SDK is rolling and pigs soar majestically in the sky. Fear not, gentle Gardener, you still have a valuable role to play! - -Googlers: The current gardener can be found at [go/cr-fuchsia-cop](http://go/cr-fuchsia-cop). - -* Look for tests that have been filtered under Fuchsia, and diagnose them. - * File a new bug, or upate the filter to refer to existing bugs, as appropriate. diff --git a/chromium/docs/get_the_code.md b/chromium/docs/get_the_code.md index 0b4e1acaa92..06a666389be 100644 --- a/chromium/docs/get_the_code.md +++ b/chromium/docs/get_the_code.md @@ -12,7 +12,7 @@ you might want to build: * [Android](android_build_instructions.md) * [Android Cast](android_cast_build_instructions.md) * [Chrome OS](chromeos_build_instructions.md) -* [Fuchsia](fuchsia_build_instructions.md) +* [Fuchsia](fuchsia/build_instructions.md) * [iOS](ios/build_instructions.md) * [Linux](linux/build_instructions.md) * [Linux Cast](linux/cast_build_instructions.md) diff --git a/chromium/docs/gpu/gpu_testing_bot_details.md b/chromium/docs/gpu/gpu_testing_bot_details.md index a5a4eff16cf..fe3240c5957 100644 --- a/chromium/docs/gpu/gpu_testing_bot_details.md +++ b/chromium/docs/gpu/gpu_testing_bot_details.md @@ -468,6 +468,23 @@ Builder]. add a manually-triggered trybot at the same time that the CI bot is added. This is described in [How to add a new manually-triggered trybot]. +While the above instructions assume that an existing parent builder will be +be used, a new one can be set up by performing a modified version of the steps: + +1. Make a [`tools/build`][tools/build] CL that adds the config for *only* the + new builder and land it. +1. Make and land Chromium CL that makes the above changes in addition to the + following: + 1. Add the new builder to the necessary `//infra/config` files in the same + way as the tester. + 1. Add the new builder to [`src/tools/mb/mb_config.pyl`][mb_config.pyl]. +1. Make a [`tools/build`][tools/build] CL that adds the config for *only* the + new tester and land it. + +Attempting to set up the builder/tester pair without first landing the +[`tools/build`][tools/build] CL for the new builder will result in things +breaking as seen in [this bug][misconfigured builder bug]. + [How to add a new manually-triggered trybot]: https://chromium.googlesource.com/chromium/src/+/master/docs/gpu/gpu_testing_bot_details.md#How-to-add-a-new-manually_triggered-trybot [ci.star]: https://chromium.googlesource.com/chromium/src/+/master/infra/config/subprojects/ci.star @@ -477,6 +494,7 @@ Builder]. [luci-scheduler.cfg]: https://chromium.googlesource.com/chromium/src/+/master/infra/config/generated/luci-scheduler.cfg [luci-milo.cfg]: https://chromium.googlesource.com/chromium/src/+/master/infra/config/generated/luci-milo.cfg [GPU FYI Win Builder]: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/GPU%20FYI%20Win%20Builder +[misconfigured builder bug]: https://bugs.chromium.org/p/chromium/issues/detail?id=1163657 ### How to start running tests on a new GPU type on an existing try bot diff --git a/chromium/docs/gpu/pixel_wrangling.md b/chromium/docs/gpu/pixel_wrangling.md index 400475f0f4f..befb4a61aa7 100644 --- a/chromium/docs/gpu/pixel_wrangling.md +++ b/chromium/docs/gpu/pixel_wrangling.md @@ -271,6 +271,10 @@ shift, and a calendar appointment. how important it is to eliminate flakiness rather than hiding it. 1. For failures of rendering_representative_perf_tests please refer to its [instructions on updating expectations][rendering_representative_perf_tests]. + 1. It's always better to have the CL reviewed properly, but for urgent + suppressions when no reviewer is available, it is possible to rubber + stamp the CL via adding `rubber-stamper@appspot.gserviceaccount.com` as + your reviewer, in addition to the regular reviewer. 1. For the remaining Gtest-style tests, use the [`DISABLED_` modifier][gtest-DISABLED] to suppress any failures if necessary. diff --git a/chromium/docs/gpu/webgl_bug_triage.md b/chromium/docs/gpu/webgl_bug_triage.md index fff3f34dd76..226e8a48dda 100644 --- a/chromium/docs/gpu/webgl_bug_triage.md +++ b/chromium/docs/gpu/webgl_bug_triage.md @@ -33,6 +33,8 @@ introduced. The specifics of the rotation follow: * Please also monitor these candidates for closing as WontFix: + * [Untriaged bugs labeled Hotlist-Recharge-Cold](https://bugs.chromium.org/p/chromium/issues/list?colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified&x=m&y=releaseblock&cells=ids&q=is%3Aopen%20component%3ABlink%3EWebGL%20status%3AUntriaged%20label%3Ahotlist-recharge-cold&can=2) + * [Open bugs needing feedback of some sort, not updated in the last 30 days](https://bugs.chromium.org/p/chromium/issues/list?can=2&q=is%3Aopen+component%3ABlink%3EWebGL+label%3ANeeds+modified%3Ctoday-30&colspec=ID+Pri+M+Stars+ReleaseBlock+Component+Status+Owner+Summary+OS+Modified&x=m&y=releaseblock&cells=ids) @@ -84,6 +86,9 @@ bugs during that shift. Bugs that aren't handled during a given shift stay with the triager; they don't spill over to the next shift, unless there's agreement with the person next on the triage rotation. +Please create a saved query in Monorail for `component:Blink>WebGL` and select +"Notify Immediately" to get emails for every change to a WebGL bug. + This is intended to be a lightweight rotation that shouldn't take too much of the triager's time. For this reason it's scheduled independently of other shifts like [pixel wrangling](pixel_wrangling.md), and may overlap. If any conflicts do diff --git a/chromium/docs/how_to_add_your_feature_flag.md b/chromium/docs/how_to_add_your_feature_flag.md index d7d6caeaca7..07c7ef8da8d 100644 --- a/chromium/docs/how_to_add_your_feature_flag.md +++ b/chromium/docs/how_to_add_your_feature_flag.md @@ -22,7 +22,7 @@ to see [[2](https://chromium-review.googlesource.com/c/554510/8/content/public/common/content_features.h)] 2. how to use it [[1](https://chromium-review.googlesource.com/c/554510/8/content/common/service_worker/service_worker_utils.cc#153)] -3. how to wire your new base::Feature to a blink runtime feature[[1](https://chromium.googlesource.com/chromium/src/+/HEAD/content/child/InitializeBlinkFeatures.md)] +3. how to wire your new base::Feature to a blink runtime feature[[1](https://chromium.googlesource.com/chromium/src/+/master/docs/initialize_blink_features.md)] 4. how to use it in blink [[1](https://chromium-review.googlesource.com/c/554510/8/third_party/blnk/renderere/core/workers/worker_thread.cc)] @@ -50,7 +50,8 @@ autoninja -C out/Default unit_tests ./out/Default/bin/run_unit_tests --gtest_filter=AboutFlagsHistogramTest.CheckHistograms ``` -That test will ask you to add several entries to enums.xml. +That test will ask you to add several entries to enums.xml. After doing so, run `git cl format` +which will insert the entries in enums.xml in the correct order and run the tests again. You can refer to [this CL](https://chromium-review.googlesource.com/c/593707) as an example. Finally, run the following test. diff --git a/chromium/docs/ios/build_instructions.md b/chromium/docs/ios/build_instructions.md index 3b2fbeb1f44..7920f090944 100644 --- a/chromium/docs/ios/build_instructions.md +++ b/chromium/docs/ios/build_instructions.md @@ -13,7 +13,7 @@ Are you a Google employee? See ## System requirements * A 64-bit Mac running 10.12.6 or later. -* [Xcode](https://developer.apple.com/xcode) 11.4+. +* [Xcode](https://developer.apple.com/xcode) 12.0 or higher. * The current version of the JDK (required for the Closure compiler). ## Install `depot_tools` @@ -168,10 +168,13 @@ You then need to request provisioning profiles from Apple for your devices for the following bundle identifiers to build and run Chromium with these application extensions: -- `${prefix}.chrome.ios.herebedragons` -- `${prefix}.chrome.ios.herebedragons.ShareExtension` -- `${prefix}.chrome.ios.herebedragons.TodayExtension` -- `${prefix}.chrome.ios.herebedragons.SearchTodayExtension` +- `${prefix}.chrome.ios.dev` +- `${prefix}.chrome.ios.dev.ContentTodayExtension` +- `${prefix}.chrome.ios.dev.CredentialProviderExtension` +- `${prefix}.chrome.ios.dev.SearchTodayExtension` +- `${prefix}.chrome.ios.dev.ShareExtension` +- `${prefix}.chrome.ios.dev.TodayExtension` +- `${prefix}.chrome.ios.dev.WidgetKitExtension` All these certificates need to have the "App Groups" (`com.apple.security.application-groups`) capability enabled for @@ -185,6 +188,11 @@ to share files and configurations while the `group.${prefix}.common` is shared with Chromium and other applications from the same organisation and can be used to send commands to Chromium. +`${prefix}.chrome.ios.dev.CredentialProviderExtension` needs the AutoFill +Credential Provider Entitlement, which corresponds to the key +`com.apple.developer.authentication-services.autofill-credential-provider` +Please refer to Apple's documentation on how to set this up. + ### Mobile provisioning profiles for tests In addition to that, you need a different provisioning profile for each @@ -345,6 +353,46 @@ This will also allow you to see the content of some of Chromium types in the debugger like `base::string16`, ... If you want to use `lldb` directly, name the file `~/.lldbinit` instead of `~/.lldbinit-Xcode`. +Note: if you are using `ios/build/tools/setup-gn.py` to generate the Xcode +project, the script also generate an `.lldbinit` file next to the project and +configure Xcode to use that file instead of the global one. + +### Changing the version of Xcode + +To change the version of Xcode used to build Chromium on iOS, please follow +the steps below: + +1. Launch the new version of Xcode.app. + + This is required as Xcode may need to install some components into + the system before the new version can be used from the command-line. + +1. Reboot your computer. + + This is required as some of Xcode components are daemons that are not + automatically stopped when updating Xcode, and command-line tools will + fail if the daemon version is incompatible (usually `actool` fails). + +1. Run `gn gen`. + + This is required as the `ninja` files generated by `gn` encodes some + information about Xcode (notably the path to the SDK, ...) that will + change with each version. It is not possible to have `ninja` re-run + `gn gen` automatically when those changes unfortunately. + + If you have a downstream chekout, run `gclient runhooks` instead of + `gn gen` as it will ensure that `gn gen` will be run automatically + for all possible combination of target and configuration from within + Xcode. + +If you skip some of those steps, the build may occasionally succeed, but +it has been observed in the past that those steps are required in the +vast majority of the situation. Please save yourself some painful build +debugging and follow them. + +If you use `xcode-select` to switch between multiple version of Xcode, +you will have to follow the same steps. + ### Improving performance of `git status` #### Increase the vnode cache size diff --git a/chromium/docs/lacros.md b/chromium/docs/lacros.md index a4fd2dc4a4b..6784b4d033e 100644 --- a/chromium/docs/lacros.md +++ b/chromium/docs/lacros.md @@ -53,34 +53,51 @@ The ash-side implementation lives in Code can be conditionally compiled into lacros via BUILDFLAG(IS_CHROMEOS_LACROS). -Lacros bugs can be filed under component: OS>LaCrOs. - -## GN var and C++ macros for Lacros - -### Desired state - -- defined(OS_CHROMEOS) is true in C++ for both ash-chrome and lacros-chrome. -- BUILDFLAG(IS_CHROMEOS_ASH) in C++ is used for ash-chrome specific part. -- BUILDFLAG(IS_CHROMEOS_LACROS) in C++ is used for lacros-chrome specific part. -- GN variable is_chromeos is true for both ash-chrome and lacros-chrome. -- GN variable is_chromeos is equivalent to is_chromeos_ash || is_chromeos_lacros. -- GN variable is_chormeos_ash is used for ash-chrome specific part. -- GN variable is_chromeos_lacros is used for lacros-chrome specific part. - -### Current state - -OS_CHROMEOS is defined and is_chromeos is set only for ash-chrome at the moment. -We are currently migrating defined(OS_CHROMEOS) to BUILDFLAG(IS_CHROMEOS_ASH), -see [crbug.com/1052397](https://crbug.com/1052397). Until the migration is -complete, for parts used by both ash-chrome and lacros-chrome, use -`BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)` in C++ files and -`is_chromeos_ash || is_chromeos_lacros` in GN files. After the migration, the -macros and GN variables should be used according to the above desired state. - -Googlers: -- [go/lacros-porting](http://go/lacros-porting) has tips on which binary the code should live in. -- [go/lacros-macros](http://go/lacros-macros) describes the steps for the migration. -- [go/lacros-build-config](http://go/lacros-build-config) is the original design doc. +## Filing bugs + +Lacros bugs should be filed under OS=Lacros + +Bugs in the ash-chrome binary that only affect ash-chrome should be labeled OS=Chrome. + +Bugs in the lacros-chrome binary that only affect lacros-chrome should be labeled OS=Lacros. + +Bugs in the ash-chrome binary that affect lacros-chrome should be labeled with both OS=Chrome and OS=Lacros. +These should not block ash-chrome releases in the short term, but should block ash-chrome releases in the long term. + +Bug in the lacros-chrome binary that affects ash-chrome: should not be possible. If lacros-chrome causes bugs in ash-chrome, then there must be a corresponding bug in ash-chrome as well. +The lacros-chrome bug should be labeled OS=Lacros and the ash-chrome bug should be labeled OS=Chrome. + +Cross-platform browser bugs e.g. Blink bug should set both OS=Lacros and OS=Chrome in the short term, since we are supporting both ash and lacros as browsers in the short term. +Once Lacros launches, the plan to use Lacros vs Chrome will be finalized. + + +## GN var and C++ macros + +*Note* that this will take effect once target_os flip of lacros-chrome is landed +with patch [crrev.com/c/2644407](https://crrev.com/c/2644407). For a short while +please keep using `BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS` +and `is_chromeos_ash || is_chromeos_lacros` to target both binaries. + +Both lacros and ash are built with gn arg `target_os="chromeos"`. This means +that C++ macro defined(OS_CHROMEOS) and gn variable is_chromeos are set true for +both lacros and ash. + +### Targeting ash or lacros +To target lacros or ash separately, use BUILDFLAG(IS_CHROMEOS_LACROS), +BUILDFLAG(IS_CHROMEOS_ASH) in C++ files and is_chromeos_lacros and +is_chromeos_ash in gn files. + +Note that these are not defined globally and must be included manually. + +To use the buildflags in C++ files, add #include "build/chromeos_buildflags.h" +and then also add "//build:chromeos_buildflags" to deps of the target that is +using the update C++ files inside gn files. See e.g. crrev.com/c/2494186. + +To use the gn variables add `import("//build/config/chromeos/ui_mode.gni")`. + +Doc for googlers: +[go/lacros-porting](http://go/lacros-porting) has tips on determining which +binary (lacros or ash) a feature should live in. ## Testing diff --git a/chromium/docs/mac_build_instructions.md b/chromium/docs/mac_build_instructions.md index b6609bb7c80..0396294fa7e 100644 --- a/chromium/docs/mac_build_instructions.md +++ b/chromium/docs/mac_build_instructions.md @@ -12,16 +12,20 @@ Are you a Google employee? See ## System requirements -* A 64-bit Mac running 10.14+. -* [Xcode](https://developer.apple.com/xcode) 11.2+ -* The OS X 10.15.1 SDK. Run +* A 64-bit Intel Mac running 10.15.4+. (Building on Arm Macs is + [not yet supported](https://chromium.googlesource.com/chromium/src.git/+/master/docs/mac_arm64.md).) +* [Xcode](https://developer.apple.com/xcode/) 12.2+. This version of Xcode + comes with ... +* The macOS 11.0 SDK. Run ```shell $ ls `xcode-select -p`/Platforms/MacOSX.platform/Developer/SDKs ``` - to check whether you have it. Building with a newer SDK works too, but - the releases [currently use Xcode 11.2.1](https://source.chromium.org/search?q=MAC_BINARIES_LABEL&ss=chromium). + to check whether you have it. Building with a newer SDK usually works too + (please fix it if it doesn't), but the releases + [currently use Xcode 12.2](https://source.chromium.org/search?q=MAC_BINARIES_LABEL&ss=chromium) + and the macOS 11.0 SDK. ## Install `depot_tools` diff --git a/chromium/docs/mac_lld.md b/chromium/docs/mac_lld.md new file mode 100644 index 00000000000..85b919ed320 --- /dev/null +++ b/chromium/docs/mac_lld.md @@ -0,0 +1,126 @@ +# LLD for Mac builds + +*Disclaimer* We don't use LLD for Mac builds. Using it is not supported, and +if you try to use it, it likely won't work. Only keep reading if you want to +work on the build. + +## Background + +Chromium uses [LLD](https://lld.llvm.org/) as linker on all platforms, +except when targeting macOS or iOS. LLD is faster than other ELF linkers (ELF +is the executable file format used on most OSs, including Linux, Android, +Chrome OS, Fuchsia), and it's faster than other COFF linkers (the executable +file format on Windows). + +ld64, the standard Mach-O linker (the executable file format on iOS and macOS), +is on the other hand already fairly fast and works well, so there are fewer +advantages to using LLD here. (Having said that, LLD is currently 4x faster +at linking Chromium Framework than ld64 in symbol\_level=0 release builds, +despite ld64 being already fast. Maybe that's due to LLD not yet doing +critical things and it will get slower, but at the moment it's faster than +ld64.) + +LLD does have a few advantages unrelated to speed, however: + +- It's developed in the LLVM repository, and we ship it in our clang package + (except on macOS, where it's not in the default clang package but an opt-in + download instead). We can fix issues upstream and quickly deploy fixed + versions, instead of having to wait for Xcode releases (which is where ld64 + ships). + +- For the same reason, it has a much simpler LTO setup: Clang and LLD both link + in the same LLVM libraries that are built at the same revision, and compiler + and linker bitcodes are interopable for that reason. With ld64, the LTO code + has to be built as a plugin that's loaded by the linker. + +- LLD/Mach-O supports "LLVM-y" features that the ELF and COFF LLDs support as + well, such as thin archives, colored diagnostics, and response files + (ld64 supports this too as of Xcode 12, but we had to wait many years for it, + and it's currently [too crashy](https://crbug.com/1147968) to be usable). + +For that reason, it's possible to opt in to LLD for macOS builds (not for +iOS builds, and that's intentionally not in scope). + +A background note: There are two versions of LLD upstream: The newer ports +that are nowadays used for ELF and COFF, and an older design that's still the +default Mach-O LLD. There's however an effort underway to write a new Mach-O +LLD that's built on the same design as the ELF and COFF ports. Chromium Mac +builds uses the new Mach-O port of LLD ("`ld64.lld.darwinnew`"). + +Just like the LLD ELF port tries to be commandline-compatible with other ELF +linkers and the LLD COFF port tries to be commandline-compatible with the +Visual Studio linker link.exe, the LLD Mach-O port tries to be +commandline-compatible with ld64. This means LLD accepts different flags on +different platforms. + +## Current status and known issues + +A `symbol_level = 0` `is_debug = false` `use_lld = true` build produces +a mostly-working Chromium.app, but there are open issues and missing features: + +- LLD does not yet have any ARM support + - relocations are missing + ([in-progress patch](https://reviews.llvm.org/D88629)) + - ad-hoc code signing is missing +- LLD produces bad debug info, and LLD-linked binaries don't yet show C++ + source code in a debugger ([bug](https://llvm.org/PR48714)] +- LLD doesn't produce unwind info, so code relying on exceptions doesn't work + ([bug](https://llvm.org/PR48389)) +- We haven't tried actually running any other binaries, so chances are many + other tests fail +- LLD doesn't yet implement `-dead_strip`, leading to many linker warnings +- LLD doesn't yet implement deduplication (aka "ICF") +- LLD doesn't yet call graph profile sort +- LLD doesn't yet implement `-exported_symbol` or `-exported_symbols_list`, + leading to some linker warnings + +## Opting in + +1. First, obtain lld. Do either of: + + 1. run `src/tools/clang/scripts/update.py --package=lld_mac` to download a + prebuilt lld binary. + 2. build `lld` and `llvm-ar` locally and copy it to + `third_party/llvm-build/Relase+Asserts/bin`. Also run + `ln -s lld third_party/llvm-build/Release+Asserts/bin/ld64.lld.darwinnew`. + + You have to do this again every time `runhooks` updates the clang + package. + + The prebuilt might work less well than a more up-to-date, locally-built + version -- see the list of open issues above for details. If anything is + marked "fixed upstream", then the fix is upstream but not yet in the + prebuilt lld binary. + +2. Add `use_lld = true` to your args.gn + +3. Then just build normally. + +`use_lld = true` makes the build use thin archives. For that reason, `use_lld` +also switches from `libtool` to `llvm-ar`. + +## Creating stand-alone repros for bugs + +For simple cases, LLD's `--reproduce=foo.tar` flag / `LLD_REPRODUCE=foo.tar` +env var is sufficient. + +See "Note to self:" [here](https://bugs.llvm.org/show_bug.cgi?id=48657#c0) for +making a repro file that involved the full app and framework bundles. + +Locally, apply this patch before building chrome to make a repro that can +be used with both lld and ld (useful for making comparisons): + + diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn + index 5ea2f2130abb..ed871642cee9 100644 + --- a/build/config/compiler/BUILD.gn + +++ b/build/config/compiler/BUILD.gn + @@ -1780,7 +1780,7 @@ config("export_dynamic") { + config("thin_archive") { + # The macOS and iOS default linker ld64 does not support reading thin + # archives. + - if ((is_posix && !is_nacl && (!is_apple || use_lld)) || is_fuchsia) { + + if ((is_posix && !is_nacl && (!is_apple)) || is_fuchsia) { + arflags = [ "-T" ] + } else if (is_win && use_lld) { + arflags = [ "/llvmlibthin" ] + diff --git a/chromium/docs/media/autoplay.md b/chromium/docs/media/autoplay.md index 3d0594a7b93..04ee027be0d 100644 --- a/chromium/docs/media/autoplay.md +++ b/chromium/docs/media/autoplay.md @@ -1,9 +1,8 @@ # Autoplay of HTMLMediaElements -Autoplay is the concept of playing media elements without user gesture. On -desktop, autoplay is always allowed. On mobile, only muted video elements are -allowed to autoplay. The autoplay logic follows -the +Autoplay is the concept of playing media elements without user gesture. The +policy that defines when autoplay is allowed is defined [here](https://www.chromium.org/audio-video/autoplay). +The autoplay logic follows the [HTML spec](https://html.spec.whatwg.org/multipage/embedded-content.html#media-elements). There are two ways of initiating autoplay: diff --git a/chromium/docs/media/gpu/veatest_usage.md b/chromium/docs/media/gpu/veatest_usage.md deleted file mode 100644 index 464db23f412..00000000000 --- a/chromium/docs/media/gpu/veatest_usage.md +++ /dev/null @@ -1,77 +0,0 @@ -# Using the Video Encode Accelerator Unittests Manually - -The VEAtest (or `video_encode_accelerator_unittest`) is a set of unit tests that -embeds the Chrome video encoding stack without requiring the whole browser, -meaning they can work in a headless environment. It includes a variety of tests -to validate the encoding stack with h264, vp8 and vp9. - -Running this test manually can be very useful when bringing up a new codec, or -in order to make sure that new code does not break hardware encoding. This -document is a walk though the prerequisites for running this program, as well -as the most common options. - -## Prerequisites - -The required kernel drivers should be loaded, and there should exist a -`/dev/video-enc0` symbolic link pointing to the encoder device node (e.g. -`/dev/video-enc0` → `/dev/video0`). - -The unittests can be built by specifying the `video_encode_accelerator_unittest` -target to `ninja`. If you are building for an ARM board that is not yet -supported by the -[simplechrome](https://chromium.googlesource.com/chromiumos/docs/+/master/simple_chrome_workflow.md) -workflow, use `arm-generic` as the board. It should work across all ARM targets. - -For unlisted Intel boards, any other Intel target (preferably with the same -chipset) should be usable with libva. AMD targets can use `amd64-generic`. - -## Basic VEA usage - -The VEA test takes raw YUV files in I420 format as input and produces e.g. an -H.264 Annex-B byte stream. Sample raw YUV files can be found at the following -locations: - -* [1080 Crowd YUV](http://commondatastorage.googleapis.com/chromiumos-test-assets-public/crowd/crowd1080-96f60dd6ff87ba8b129301a0f36efc58.yuv) -* [320x180 Bear YUV](http://commondatastorage.googleapis.com/chromiumos-test-assets-public/bear/bear-320x180-c60a86c52ba93fa7c5ae4bb3156dfc2a.yuv) - -It is recommended to rename these files after downloading them to e.g. -`crowd1080.yuv` and `bear-320x180.yuv`. - -The VEA can then be tested as follows: - - ./video_encode_accelerator_unittest --single-process-tests --disable_flush --gtest_filter=SimpleEncode/VideoEncodeAcceleratorTest.TestSimpleEncode/0 --test_stream_data=bear-320x180.yuv:320:180:1:bear.mp4:100000:30 - -for the `bear` file, and - - ./video_encode_accelerator_unittest --single-process-tests --disable_flush --gtest_filter=SimpleEncode/VideoEncodeAcceleratorTest.TestSimpleEncode/0 --test_stream_data=crowd1080.yuv:1920:1080:1:crowd.mp4:4000000:30 - -for the larger `crowd` file. These commands will put the encoded output into -`bear.mp4` and `crowd.mp4` respectively. They can then be copied on the host and -played with `mplayer -fps 25`. - -## Test filtering options - -`./video_encode_accelerator_unittest --help` will list all valid options. - -The list of available tests can be retrieved using the `--gtest_list_tests` -option. - -By default, all tests are run, which can be a bit too much, especially when -bringing up a new codec. The `--gtest_filter` option can be used to specify a -pattern of test names to run. - -## Verbosity options - -The `--vmodule` options allows to specify a set of source files that should be -more verbose about what they are doing. For basic usage, a useful set of vmodule -options could be: - - --vmodule=*/media/gpu/*=4 - -## Source code - -The VEAtest's source code can be consulted here: [https://cs.chromium.org/chromium/src/media/gpu/video_encode_accelerator_unittest.cc](https://cs.chromium.org/chromium/src/media/gpu/video_encode_accelerator_unittest.cc). - -V4L2 support: [https://cs.chromium.org/chromium/src/media/gpu/v4l2/](https://cs.chromium.org/chromium/src/media/gpu/v4l2/). - -VAAPI support: [https://cs.chromium.org/chromium/src/media/gpu/vaapi/](https://cs.chromium.org/chromium/src/media/gpu/vaapi/). diff --git a/chromium/docs/media/gpu/video_encoder_test_usage.md b/chromium/docs/media/gpu/video_encoder_test_usage.md index 688d497cb5c..6a743bb07e4 100644 --- a/chromium/docs/media/gpu/video_encoder_test_usage.md +++ b/chromium/docs/media/gpu/video_encoder_test_usage.md @@ -8,20 +8,31 @@ don't require the full Chrome browser stack. They are built on top of the [GoogleTest](https://github.com/google/googletest/blob/master/README.md) framework. -__Note:__ Currently these tests are under development and functionality is still -limited. - [TOC] ## Running from Tast -Running these tests from Tast is not supported yet. +The Tast framework provides an easy way to run the video encoder tests from a +ChromeOS chroot. Test data is automatically deployed to the device being tested. +To run all video encoder tests use: + + tast run $HOST video.EncodeAccel.* + +Wildcards can be used to run specific sets of tests: +* Run all VP8 tests: `tast run $HOST video.EncodeAccel.vp8*` + +Check the +[tast video folder](https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/refs/heads/master/src/chromiumos/tast/local/bundles/cros/video/) +for a list of all available tests. +See the +[Tast quickstart guide](https://chromium.googlesource.com/chromiumos/platform/tast/+/HEAD/docs/quickstart.md) +for more information about the Tast framework. ## Running manually To run the video encoder tests manually the _video_encode_accelerator_tests_ target needs to be built and deployed to the device being tested. Running the video encoder tests can be done by executing: - ./video_encode_accelerator_tests [<video path>] [<video metadata path>] + ./video_encode_accelerator_tests [<video path>] e.g.: `./video_encode_accelerator_tests bear_320x192_40frames.yuv.webm` @@ -29,7 +40,7 @@ Running the video encoder performance tests can be done in a smilar way by building, deploying and executing the _video_encode_accelerator_perf_tests_ target. - ./video_encode_accelerator_perf_tests [<video path>] [<video metadata path>] + ./video_encode_accelerator_perf_tests [<video path>] e.g.: `./video_encode_accelerator_perf_tests bear_320x192_40frames.yuv.webm` @@ -48,13 +59,16 @@ simple json file that contains info about the video such as its pixel format, dimensions, framerate and number of frames. These can also be found in the _media/test/data_ folder (e.g. [_bear_320x192_40frames.yuv.webm.json_](https://cs.chromium.org/chromium/src/media/test/data/bear_320x192_40frames.yuv.webm.json)). -If no metadata file is specified _\<video path\>.json_ will be used. +The metadata file must be _\<video path\>.json_ in the same directory. ## Command line options Multiple command line arguments can be given to the command: --codec codec profile to encode, "h264 (baseline)", "h264main, "h264high", "vp8" and "vp9" + --output_folder overwrite the output folder used to store + performance metrics, if not specified results + will be stored in the current working directory. -v enable verbose mode, e.g. -v=2. --vmodule enable verbose mode for the specified module, @@ -65,9 +79,21 @@ Multiple command line arguments can be given to the command: Non-performance tests only: - --disable_validator disable validation of encoded bitstream. + --num_temporal_layers the number of temporal layers of the encoded + bitstream. Only used in --codec=vp9 currently. + --disable_validator disable validation of encoded bitstream. + --output_bitstream save the output bitstream in either H264 AnnexB + format (for H264) or IVF format (for vp8 and + vp9) to <output_folder>/<testname>. + --output_images in addition to saving the full encoded, + bitstream it's also possible to dump individual + frames to <output_folder>/<testname>, possible + values are \"all|corrupt\" + --output_format set the format of images saved to disk, + supported formats are \"png\" (default) and + \"yuv\". + --output_limit limit the number of images saved to disk. ## Source code See the video encoder tests [source code](https://cs.chromium.org/chromium/src/media/gpu/video_encode_accelerator_tests.cc). See the video encoder performance tests [source code](https://cs.chromium.org/chromium/src/media/gpu/video_encode_accelerator_perf_tests.cc). - diff --git a/chromium/docs/mojo_and_services.md b/chromium/docs/mojo_and_services.md index 49d0408d2a2..2b04aeeaf69 100644 --- a/chromium/docs/mojo_and_services.md +++ b/chromium/docs/mojo_and_services.md @@ -368,15 +368,14 @@ auto RunMathService(mojo::PendingReceiver<math::mojom::MathService> receiver) { return std::make_unique<math::MathService>(std::move(receiver)); } -mojo::ServiceFactory* GetMainThreadServiceFactory() { - // Existing factories... - static base::NoDestructor<mojo::ServiceFactory> factory { - RunFilePatcher, - RunUnzipper, - - // We add our own factory to this list - RunMathService, - //... +void RegisterMainThreadServices(mojo::ServiceFactory& services) { + // Existing services... + services.Add(RunFilePatcher); + services.Add(RunUnzipper); + + // We add our own factory to this list + services.Add(RunMathService); + //... ``` With this done, it is now possible for the browser process to launch new diff --git a/chromium/docs/ozone_overview.md b/chromium/docs/ozone_overview.md index 3c0546ea086..96a6176547e 100644 --- a/chromium/docs/ozone_overview.md +++ b/chromium/docs/ozone_overview.md @@ -306,6 +306,21 @@ use_glib=true use_gtk=true ``` +Running some test suites requires a Wayland server. If you're not +running one you can use a locally compiled version of Weston. This is +what the build bots do. Add this to your gn args: + +``` shell +use_bundled_weston = true +``` + +Then run the xvfb.py wrapper script and tell it to start Weston: + +``` shell +cd out/debug # or your out directory +../../testing/xvfb.py --use-weston --no-xvfb ./views_unittests --ozone-platform=wayland --enable-features=UseOzonePlatform +``` + Feel free to discuss with us on freenode.net, `#ozone-wayland` channel or on `ozone-dev`, or on `#ozone-wayland-x11` channel in [chromium slack](https://www.chromium.org/developers/slack). diff --git a/chromium/docs/patterns/domain-lens.md b/chromium/docs/patterns/domain-lens.md index c2bed7d9ee5..d9f85d4b505 100644 --- a/chromium/docs/patterns/domain-lens.md +++ b/chromium/docs/patterns/domain-lens.md @@ -85,8 +85,8 @@ using DownloadProgressCallback = class DownloadProgressProvider { public: - base::CallbackList::Subscription - RegisterDownloadProgressCallback(DownloadProgressCallback callback); + base::CallbackListSubscription RegisterDownloadProgressCallback( + DownloadProgressCallback callback); DownloadProgress GetCurrentProgress(); }; diff --git a/chromium/docs/patterns/passkey.md b/chromium/docs/patterns/passkey.md index bb7828053c5..5c0b3667e41 100644 --- a/chromium/docs/patterns/passkey.md +++ b/chromium/docs/patterns/passkey.md @@ -1,7 +1,7 @@ # The Passkey Pattern For the Chromium implementation of this pattern, see -[//base/util/type_safety/pass_key.h]. +[//base/types/pass_key.h]. The Passkey pattern is used when you need to expose a subset of a class's methods to another class in a more granular way than simply friending the other @@ -49,4 +49,4 @@ are used to pass in the Passkey object. It is encouraged to leave the `BarPasskey` parameter unnamed to reinforce that it carries no semantic information and is not actually used for anything. -[//base/util/type_safety/pass_key.h]: ../../base/util/type_safety/pass_key.h +[//base/types/pass_key.h]: ../../base/types/pass_key.h diff --git a/chromium/docs/pgo.md b/chromium/docs/pgo.md index a7eef3f9aca..3c7b800fb22 100644 --- a/chromium/docs/pgo.md +++ b/chromium/docs/pgo.md @@ -21,7 +21,6 @@ To produce an executable built with a custom PGO profile: * `vpython tools/perf/run_benchmark system_health.common_desktop --assert-gpu-compositing --run-abridged-story-set --browser=exact --browser-executable=out/path/to/chrome` * `vpython tools/perf/run_benchmark speedometer2 --assert-gpu-compositing --browser=exact --browser-executable=out/path/to/chrome` - * `vpython tools/perf/run_benchmark webrtc --assert-gpu-compositing --browser=exact --browser-executable=out/path/to/chrome` * This will produce `*.profraw` files in the current working directory * Merge the profiling data diff --git a/chromium/docs/process/lsc/large_scale_changes.md b/chromium/docs/process/lsc/large_scale_changes.md new file mode 100644 index 00000000000..8b0c40d1d78 --- /dev/null +++ b/chromium/docs/process/lsc/large_scale_changes.md @@ -0,0 +1,123 @@ +# Chrome Large Scale Changes + +[TOC] + +*** promo +**Note**: this is heavily inspired by a [Google-internal process (link for Googlers only)](https://goto.google.com/lsc) with Chrome-specific modifications. +*** + +## What is it? {#what-is-it} + +`chromium/src` has millions of lines of code. Sometimes, we want to change code that has been used for years and is referenced from thousands of places throughout the codebase. These sorts of changes are referred to as LSCs (Large Scale Changes) and generally require complex planning. + +This page and the referenced docs describe the policies and best practices that go into the planning, creating, and executing changes that impact many distinct packages. + +## What is it good for? {#what-is-it-good-for} + +Fixing old mistakes, crufty interfaces, and bad usage requires making many small changes across the codebase. Individually, these changes are tiny, but taken all together, they allow us to deprecate old interfaces and pay down our technical debt. The simpler we can make the code base, the more that developers can focus on their actual problems, instead of coping with artificial complexity because of accumulated cruft. + +Although there are many ways that these changes can be handled, most will be handled by an Eng Review reviewer and a domain reviewer. + +## What is it NOT good for? {#what-is-it-not-good-for} + +Given the cost of large scale changes (review cost often being a significant factor), we want to prioritize changes that have high value. + +For example, while cosmetic changes have some positive value, we want reviewers to spend their time on higher value changes. This means that changes which are primarily cosmetic are rarely worth doing unless they interact with other automation, such as fixing a spelling mistake that then makes other refactorings work more consistently. If in doubt, send us your proposal before spending too much time on it. + +## Who uses it? {#who-uses-it} + +Anyone making "large" changes. (Currently this is roughly defined as changes covering more than 10 distinct directories with OWNERS files.) We are aiming to enable a process where the processes and practices (using [`git cl split`](https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-cl.html)) for making sweeping changes are more broadly accessible enabling teams to more readily change the APIs they export while keeping a bound on how much time is spent coping with codebase churn. + +## Practical matters {#practical-matters} + +### Why do we have this process? {#why-do-we-have-this-process} + +At the highest level, we want to make sure anything going out in tens/hundreds/thousands of CLs is moving the codebase in the right direction and that it's worth the work it takes to get such changes tested, reviewed, and submitted. Eng Review will rarely say "This isn't impactful enough," although it may happen (especially in cases where there is little or no distinct benefit). The more common role of the review is to ensure that the necessary documentation is in place: you will be sending CLs that are seen by dozens or hundreds of engineers who may not have any knowledge about your change. Some CL description updates and documentation can go a long way to making the process smooth. + +In Chrome, we will grant the LSC Requestor the Gerrit Global Owners Approver power for the duration of their change, i.e. a member of Eng Review will approve the LSC Proposal which when then gives the LSC Requester the ability to bypass OWNERS in the interest of efficiency. (For external contributors, a Googler sponsor will act as the Global Owners Approver power holder. If you’re an external contributor, everything else is the same.) Each CL will still need a second human to review it to satisfy “two sets of eyes” code review policy requirements. This reduces the code-review burden on the rest of Chrome reviewers and increases the rate at which your change gets submitted. However, this also means that these kinds of LSCs have a high bar of being low-risk and non-controversial. + +As the LSC Requestor, the process of getting the LSC fully submitted looks like: + +1. Create all of the CLs in the set, using a hashtag to identify them. This can be done in small batches (using automation or manually), if desired. +2. Use a command-line tool to mark them all of them as Owners-Override +1 using Global Owners Approver power (or have your Googler sponsor do it for you) +3. Ask a Chromium peer to use the same command-line tool to mass Code-Review +1 the set (spot-checking the CLs for correctness) + +After the LSC is fully submitted, the LSC Requestor will lose their Global Owners Approver power. + +If global owners approval does not seem appropriate, we will ask you to send your changes for local approval by owners of the individual directories you're changing. + +### Multiple repositories {#multiple-repositories} + +To build Chrome, we have many other repos which are included via DEPS. Most of these are under the ownership of the Chrome org and this process (and have identical policies). But, some are not. For those that are under Chrome, this policy applies to all repos. V8 and Skia are not yet included in this process. + +As a LSC Requestor, you can split your CLs across these boundaries. + +### Historical examples of candidates for this new process {#historical-examples-of-candidates-for-this-new-process} + +* [Migrate from base::Bind() to base::BindOnce() or base::BindRepeating()](https://bugs.chromium.org/p/chromium/issues/detail?id=714018) +* [Restrict abilities of MessageLoop::current() (and ultimately remove it)](https://bugs.chromium.org/p/chromium/issues/detail?id=825327) + +## Best practices {#best-practices} + +### Docs for authors {#docs-for-authors} + +* [Chrome LSC Template](https://docs.google.com/document/d/10S8ESUvwhEOOBEKr-hn97y8eRTYczavizsUNv5Gvcg8/edit) - If you only look at one thing, this should be it. +* [Chrome LSC Workflow](lsc_workflow.md) - The high-level steps to get a new LSC approved. + +### FAQ for authors {#faq-for-authors} + +* **Do I have to use this process?** This depends on how large your change is—specifically, how many child CLs your large refactoring will split it into—and whether all sub-CLs affect the same OWNERS. + + | Number of child CLs | Action | + |---------------------|----------------------------------------------------------------------------------------------------------------------------| + | >= 30 | Follow the [Chrome LSC Workflow](lsc_workflow.md) (i.e. create an LSC doc, file a bug, wait for approval). | + | < 29 | No requirements | + + * If you would like to request global owners approval for safe or trivial changes of any size, please also follow this process and wait for the LSC committee to respond to your document. + * If all sub-CLs affect the same OWNERS, you do not need to follow the LSC process and instead can talk to an area owner for how to proceed with your change. +* **What do I have to do?** Follow the steps at [Chrome LSC Workflow](lsc_workflow.md). +* **How long will this process take?** You should expect an initial response from a cleanup approver within two days. +* **I'm in a hurry; Is there a fast track?** Eng Review may fast-track low-risk changes, with a response estimated between a few hours to one business day. If you'd like to request fast tracking, email [lsc-review](https://groups.google.com/a/chromium.org/d/forum/lsc-review) with a link to the bug you filed at the end of [Chrome LSC Workflow](lsc_workflow.md) and we'll get to it as quickly as we can. +* **Why this process?** Changes that operate broadly across the codebase affect many engineers and teams and these changes may generate discussion on the change or the best way to roll it out. This process is in place to make sure that these changes are useful, well-communicated, and to minimize the risk of having to attempt difficult rollbacks. +* **I have an idea for a change, but I’m not sure who to ask about it?** Definitely email the lsc-review@chromium.org list. We should be able to give you at least a vague idea of whether your idea has merit and who to approach as a domain expert to move ahead. +* **My LSC touches third_party, do I have to do anything special?** These changes are likely to be rare. If it does occur, please obey the normal rules for third_party changes. Be aware that some parts of third_party have an external codebase as their source-of-truth, and so are mechanically generated (e.g. by Copybara [[external](https://opensource.google/projects/copybara), [internal, Googlers only](http://go/copybara)]). This means that your changes will be overwritten by the next import. +* **My proposal was accepted for "local approval", now what?** Get your CLs submitted. For changes at this scale, it's usually best to use Find Owners and Auto-Submit. If anything surprising comes up during this process, or you need to ask questions to someone familiar with the process, try your assigned committee reviewer (see your LSC document). As with any wide-scale change, you should also consider announcing it on a mailing list that covers the target audience. +* **My proposal was accepted for "global approval", now what?** Contact your Eng Review reviewer: they should have granted you (or your Googler sponsor) Global Approver power. The techniques for getting these submitted via global owner approval vary based on what is being changed, your approver will know what's what for your particular LSC. As with any wide-scale change, you should also consider announcing it on a mailing list. Any coworker or contributor can be your second-set of eyes to mass Code-Review +1 all of your CLs. +* **My proposal requires multiple large changes, do I need multiple LSC docs & approvals?** No. You can use a single LSC document to describe your entire series of changes even if you are asking for a mix of local and global approvals. Be sure to break each change out into an easily identifiable step in the document. +* **How do I deal with backsliding?** Backsliding (others submitting changes which reverse the intended change) is something which cannot be completely avoided in all cases. A useful techniques to reduce them is to use Tricium (see [go/luci/tricium [internal, Googlers only]](https://goto.google.com/luci/tricium) for details) or Presubmit. + +### Docs for Domain Reviewers {#docs-for-domain-reviewers} + +* [The Chrome LSC Workflow](lsc_workflow.md) + +### FAQ for Domain Reviewers {#faq-for-domain-reviewers} + +* **How do I pick a domain reviewer?** Your domain reviewer should be someone with the expertise and authority to approve the technical direction of the change you're proposing. For example, if you're proposing migrating all users from library A to library B, your domain reviewer might be an OWNER of library A who can confirm the direction of your proposed LSC. This can also be someone from your team, if they're a domain expert. The idea is to get permission from anyone who could say "No, please roll back all these migration CLs." +* **Why am I being asked to comment?** You’ve been selected as a relevant owner/expert for a large-scale change. We need your input as to whether or not the change is a good idea. You don’t need to review in detail (like a code review), but we want to make sure the relevant parties are convinced that the change is good. What we don’t want is to have to roll back after getting part way through a change involving hundreds of CLs. +* **What if I disagree?** That is exactly why you’re being asked: if you don’t like the change, we want to stop it before it gets split up and submitted. +* **What if I think someone else should be contacted?** Absolutely bring them into the discussion. The goal is to make sure that no LSC-approved change will have to be rolled back because of a lack of information or planning. We want to bring any and all relevant experts and stakeholders into the discussion early to minimize the risks. + +### FAQ for Local CL Reviewers {#faq-for-local-cl-reviewers} + +* **How do I review this CL?** Because these tiny cleanup CLs may differ from your normal reviews, you can likely look at them in the context of the entire change process and its approval. +* **I don’t like this change.** You can say no, but must have a good reason. These changes generally involve mass refactoring - if you refuse the change, you may block the payoff of that refactoring for all of Chrome. Be prepared to justify your objections. If your objections to a CL amounts to wanting to do it differently, consider whether it's really worth objecting to - there is value in simply getting things done, and at our scale, it's not always possible to find a solution that is optimal for everyone. \ + \ +That said, as with any CL, the author should be able to provide justification for it, and part of the purpose of LSC policy is to ensure that justification exists and is sufficient. You can ask the author about the change, but first read the description and CL email carefully, including any linked supporting documentation. You may find it interesting to know more about what's going on, and it may save the need for a one-on-one conversation. \ + \ +If you wish to share your concerns, see the contact info below. +* **I get too many cleanup CLs.** We’ll try to use global owners approvers as often as possible to avoid spreading the burden to teams who maintain code/libraries commonly affected by refactorings. +* **I don't actually work on that anymore.** Have you considered removing yourself from the OWNERS file? If you aren't willing and capable of doing reviews, you probably shouldn't be an OWNER. + +## History and evolution {#history-and-evolution} + +Chrome’s LSC process was introduced to reduce the use of self-code-review to bypass OWNERS as we moved to make code review mandatory. It (and this doc) were heavily copied from a Google-internal process that has been in place for more than a decade. We needed an improved workflow for submitting large scale changes (i.e. refactorings or architectural changes) across a big codebase (e.g. chromium/src). Because it had been so slow to get OWNERS approval for dozens of directories across dozens of CLs, folks in chromium/src had the convention of code-review from a main owner and then TBR= every other owner. + +## Whom to contact {#whom-to-contact} + +* lsc-policy@chromium.org for questions about best practices, documentation, or this FAQ. +* lsc-review@chromium.org for questions about a particular change. + +## Additional resources {#additional-resources} + +* [Chrome LSC Workflow](lsc_workflow.md) - Overview of the steps you need to do in order to start a new Chrome LSC. +* [Chrome LSC Template](https://docs.google.com/document/d/10S8ESUvwhEOOBEKr-hn97y8eRTYczavizsUNv5Gvcg8/edit) - Use this form to propose a change. diff --git a/chromium/docs/process/lsc/lsc_workflow.md b/chromium/docs/process/lsc/lsc_workflow.md new file mode 100644 index 00000000000..b2a05b95022 --- /dev/null +++ b/chromium/docs/process/lsc/lsc_workflow.md @@ -0,0 +1,35 @@ +# Chrome LSC Workflow {#chrome-lsc-workflow} + +This document describes the workflow for getting a large-scale change approved ([Chrome LSC](large_scale_changes.md)). + +*** note +Status: _**DRAFT**_ + +Editors: [jclinton@google.com](mailto:jclinton@google.com) +*** + +## 1 Complete the LSC template ([link](https://docs.google.com/document/d/10S8ESUvwhEOOBEKr-hn97y8eRTYczavizsUNv5Gvcg8/edit)) {#1-complete-the-lsc-template-link} + +This short document will describe the large-scale change you want to make: What the change is, why you want to make it, estimated size (#CLs, #files), etc. Once you start sending out CLs, a link to that document should also be included in the description of each CL (more details at [Chrome LSC Template](https://docs.google.com/document/d/10S8ESUvwhEOOBEKr-hn97y8eRTYczavizsUNv5Gvcg8/edit)). + +## 2 Request a domain review by filing a [bug](not.live.yet) {#2-request-a-domain-review-by-filing-a-bug} + +Find [a domain reviewer](large_scale_changes.md#for-domain-reviewers) – someone who is knowledgeable in the area you are changing. Request a domain review for the LSC document you created in step 1 by filing a bug (template: [bug template](not.live.yet)) and assigning it to this person. The bug template contains the instructions for the domain reviewer (briefly, if the domain reviewer approves the proposed LSC, they should add an "LGTM" comment to the bug). Put a link to the bug in the table at the top of the doc. + +## 3 Wait for approval by lsc-review@ {#3-wait-for-approval-by-lsc-review@} + +Once the domain review is complete, the domain reviewer will assign the bug to [lsc-review@chromium.org](https://groups.google.com/a/chromium.org/d/forum/lsc-review), who will review your LSC request. You should expect an initial response within two business days. How long it takes until the request is approved depends on how complex the change is and how much needs to be discussed; for simple changes, it should only take a few days. **Final approval will be indicated by the LSC bug being marked FIXED**. + +## 4 Start sending your CLs! {#4-start-sending-your-cls} + +Once your LSC is approved (the bug from step 2 is marked FIXED) you may begin sending your CLs for review. You probably want to use [`git cl split`](https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-cl.html) to do this. (Please don’t attach the LSC bug to your CLs!) + +If your LSC was approved for "global owners approval", you should get in direct contact with someone who can mass code-review your CLs to coordinate sending the firehose of CLs their way. Good luck! + +*** note +**TODO jclinton**: info on command line tool usage for mass-Owners-Override and mass-Code-Review +*** + +## Questions {#questions} + +If at any point you have questions about this workflow, please contact [lsc-review@chromium.org](mailto:lsc-review@chromium.org) and we'll be happy to help. diff --git a/chromium/docs/security/OWNERS b/chromium/docs/security/OWNERS index a8623751d4e..57ae6172340 100644 --- a/chromium/docs/security/OWNERS +++ b/chromium/docs/security/OWNERS @@ -3,7 +3,6 @@ awhalley@chromium.org dcheng@chromium.org estark@chromium.org felt@chromium.org -mmoroz@chromium.org nasko@chromium.org meacer@chromium.org mkwst@chromium.org @@ -11,4 +10,4 @@ palmer@chromium.org parisa@chromium.org rsesek@chromium.org tsepez@chromium.org -vakh@chromium.org
\ No newline at end of file +vakh@chromium.org diff --git a/chromium/docs/security/autoupgrade-mixed.md b/chromium/docs/security/autoupgrade-mixed.md index 9e03ea33c5a..d5b3e4693cd 100644 --- a/chromium/docs/security/autoupgrade-mixed.md +++ b/chromium/docs/security/autoupgrade-mixed.md @@ -4,7 +4,7 @@ Chrome will now (starting on M80) attempt to upgrade some types of mixed content (HTTP on an HTTPS site) subresources. Subresources that fail to load over HTTPS will not be loaded. For more information see [the official announcement](https://blog.chromium.org/2019/10/no-more-mixed-messages-about-https.html). ## Scope -Currently only audio and video subresources are autoupgraded. On a future version images will be included. +Audio, video, and image subresources are upgraded. Blockable (i.e. all other types of) mixed content are blocked without an autoupgrade attempt. ## Opt-out -Users can disable autoupgrades on a per-site basis through content settings (chrome://settings/content/insecureContent). +Users can disable autoupgrades, and allow blockable mixed content to load, on a per-site basis through content settings (chrome://settings/content/insecureContent). diff --git a/chromium/docs/security/faq.md b/chromium/docs/security/faq.md index 6cdff5cfef6..0c8d3cb1e07 100644 --- a/chromium/docs/security/faq.md +++ b/chromium/docs/security/faq.md @@ -220,11 +220,15 @@ computer. <a name="TOC-Why-aren-t-compromised-infected-machines-in-Chrome-s-threat-model-"></a> ## Why aren't compromised/infected machines in Chrome's threat model? -This is essentially the same situation as with physically-local attacks. The -attacker's code, when it runs as your user account on your machine, can do -anything you can do. (See also [Microsoft's Ten Immutable Laws Of +Although the attacker may now be remote, the consequences are essentially the +same as with physically-local attacks. The attacker's code, when it runs as +your user account on your machine, can do anything you can do. (See also +[Microsoft's Ten Immutable Laws Of Security](https://web.archive.org/web/20160311224620/https://technet.microsoft.com/en-us/library/hh278941.aspx).) +Other cases covered by this section include leaving a debugger port open to +the world, remote shells, and so forth. + <a name="TOC-What-about-unmasking-of-passwords-with-the-developer-tools-"></a> ## What about unmasking of passwords with the developer tools? diff --git a/chromium/docs/security/mojo.md b/chromium/docs/security/mojo.md index 51d325ce1f6..9876d01758d 100644 --- a/chromium/docs/security/mojo.md +++ b/chromium/docs/security/mojo.md @@ -532,8 +532,8 @@ destroyed, e.g.: ```c++ { - base::Callback<int> cb = mojo::WrapCallbackWithDefaultInvokeIfNotRun( - base::Bind([](int) { ... }), -1); + base::OnceCallback<int> cb = mojo::WrapCallbackWithDefaultInvokeIfNotRun( + base::BindOnce([](int) { ... }), -1); } // |cb| is automatically invoked with an argument of -1. ``` @@ -542,7 +542,7 @@ This can be useful for detecting interface errors: ```c++ process->GetMemoryStatistics( mojo::WrapCallbackWithDefaultInvokeIfNotRun( - base::Bind(&MemoryProfiler::OnReplyFromRenderer), <failure args>)); + base::BindOnce(&MemoryProfiler::OnReplyFromRenderer), <failure args>)); // If the remote process dies, &MemoryProfiler::OnReplyFromRenderer will be // invoked with <failure args> when Mojo drops outstanding callbacks due to // a connection error on |process|. diff --git a/chromium/docs/security/permissions-for-powerful-web-platform-features.md b/chromium/docs/security/permissions-for-powerful-web-platform-features.md index f2a25114bbd..8a3babee906 100644 --- a/chromium/docs/security/permissions-for-powerful-web-platform-features.md +++ b/chromium/docs/security/permissions-for-powerful-web-platform-features.md @@ -52,7 +52,7 @@ and summarises why alternative proposals were not taken up. + [__Powerful Web Platform APIs__](https://www.chromium.org/Home/chromium-security/prefer-secure-origins-for-powerful-new-features) are capabilities which carry inherent security or privacy risks when used, but also provide users of web apps with significant utility. A canonical - example is native file system access (i.e. allowing web sites to directly + example is local file system access (i.e. allowing web sites to directly read and write from certain locations on the user's device). Many such capabilities already exist on the web in every browser (e.g. access to camera and microphone hardware), while new capabilities are under constant @@ -246,7 +246,7 @@ mechanisms. For example, some of the following measures could be explored: + restrict what Service Workers may do in the background unless they control a site that is installed. -# Case Study -- Native File System Access +# Case Study -- File System Access We describe a high level case study based on the principles in this document for granting a web site access to a) read any file in a certain directory; b) @@ -279,8 +279,8 @@ write files to a directory. Apps on any desktop or mobile platform require installation to run, and when installed, apps are automatically granted many privileges. We could extend -this concept to the web by restricting powerful APIs like native file system -access only to installed web apps. That is, the drive-by web could not even +this concept to the web by restricting powerful APIs like the File System +Access API only to installed web apps. That is, the drive-by web could not even ask for permission to access an API -- the site would need to be installed. A key argument for using installation in this manner is that some APIs are diff --git a/chromium/docs/security/rule-of-2.md b/chromium/docs/security/rule-of-2.md index 9f68cbe1a01..df449d27355 100644 --- a/chromium/docs/security/rule-of-2.md +++ b/chromium/docs/security/rule-of-2.md @@ -254,6 +254,31 @@ class, which is a Java wrapper [around C++ Skia](https://cs.android.com/android/platform/superproject/+/master:frameworks/base/libs/hwui/jni/BitmapFactory.cpp;l=586;drc=864d304156d1ef8985ee39c3c1858349b133b365). These APIs are therefore not considered memory-safe under the rule. +## Safe Types + +As discussed above in [Normalization](#normalization), there are some types that +are considered "safe," even though they are deserialized from an untrustworthy +source, at high privilege, and in an unsafe language. These types are +fundamental for passing data between processes using IPC, tend to have simpler +grammar or structure, and/or have been audited or fuzzed heavily. + +* `GURL` +* `SkBitmap` +* `SkPixmap` +* Protocol buffers (see above; this is not a preferred option and should be + avoided where possible) + +There are also classes in //base that internally hold simple values that +represent potentially complex data, such as: + +* `base::FilePath` +* `base::Token` and `base::UnguessableToken` +* `base::Time` and `base::TimeDelta` + +The deserialization of these is safe, though it is important to remember that +the value itself is still untrustworthy (e.g. a malicious path trying to escape +its parent using `../`). + ## Existing Code That Violates The Rule We still have a lot of code that violates this rule. For example, until very diff --git a/chromium/docs/security/security-labels.md b/chromium/docs/security/security-labels.md index 45e24a378a8..deb7c037dae 100644 --- a/chromium/docs/security/security-labels.md +++ b/chromium/docs/security/security-labels.md @@ -121,7 +121,8 @@ Other cases where it's OK to set **Security_Impact-None**: Cases where it's *not* OK to set **Security_Impact-None**: * Features enabled via normal UI or settings which users might happen across - in normal usage. For instance, accessibility features. + in normal usage. For instance, accessibility features and the Chrome Labs + experimental features accessible from the toolbar. * Origin trials. Origin trials are only active on some websites, but the affected code does run for Chrome users with the default Chrome configuration. * The impacted code runs behind a feature flag which is *enabled by default*, @@ -161,6 +162,24 @@ the pathname.) OS, and perhaps Fuchsia (?). Views for macOS is increasingly a thing, but Cocoa code (e.g. `ui/message_center/cocoa`) is particular to macOS. +## After the bug is fixed: Merge labels {#TOC-Merge-labels} + +Once you've landed a complete fix for a security bug, please immediately +mark the bug as Fixed. Do not request merges: Sheriffbot will request +appropriate merges to beta or stable according to our guidelines. +However, it is really helpful if you comment upon any unusual stability or +compatibility risks of merging. + +(Some Chromium teams traditionally deal with merges _before_ marking bugs as +Fixed. Please don't do that for security bugs.) + +Please take the opportunity to consider whether there are any variants +or related problems. It's very common for attackers to tweak working attack code +to exploit a similar situation elsewhere. If you've even the remotest thought +that there _might_ be equivalent patterns or variants elsewhere, file a bug +with type=Bug-Security. It can be nearly blank. The important thing is to record +the fact that something may need doing. + ## Sheriffbot automation Security labels guide the actions taken by diff --git a/chromium/docs/security/sheriff.md b/chromium/docs/security/sheriff.md index ed005d88704..667a5961721 100644 --- a/chromium/docs/security/sheriff.md +++ b/chromium/docs/security/sheriff.md @@ -169,7 +169,7 @@ i like that.") bugs into their triage queue, just set OS to the single value of "Chrome". No other steps or labels are needed. * If you need to ping or ask about Chrome OS bug, [ask their current - sheriff](go/whos-the-chromeos-sheriff). + sheriff](http://go/whos-the-chromeos-sheriff). * **If the report smells like a vulnerability, keep going.** ### Verify And Label The Bug @@ -192,24 +192,25 @@ assigning it to someone else. A few components have their own triage processes or points of contact who can help. -* V8 ClusterFuzz bugs can be assigned to the [V8 ClusterFuzz +* **V8 ClusterFuzz bugs** can be assigned to the [V8 ClusterFuzz Sheriff](https://rotation.googleplex.com/status?id=5714662985302016) for triage. Note that V8 CHECK failure crashes can have security implications, so don't triage it yourself and instead assign it to V8 ClusterFuzz Sheriff. They can make an informed decision on whether it is a security vulnerability or not and whether it is safe to strip the security tags (**Type=Bug-Security**, **Restrict-View-SecurityTeam**). -* V8 non-ClusterFuzz bugs shouldn't be assigned to the V8 ClusterFuzz sheriff. +* **V8 non-ClusterFuzz bugs** shouldn't be assigned to the V8 ClusterFuzz sheriff. Instead, Googlers should refer to [the V8 security bug triage instructions](http://go/v8-security-issue-triage-how-to) for lists of component owners. -* Skia bugs can be assigned to hcm@chromium.org. Be careful while triaging +* **Skia bugs** can be assigned to hcm@chromium.org. Be careful while triaging these! The place where we're crashing isn't necessarily the place where the bug was introduced, so blame may be misleading. Skia fuzzing bugs can be assigned to kjlubick@chromium.org, as Skia is heavily fuzzed on OSS-Fuzz and some issues reported in Chromium are already known or even fixed upstream. -* URL spoofing issues, especially related to RTL or IDNs? See +* **URL spoofing issues**, especially related to RTL or IDNs? See [go/url-spoofs](http://go/url-spoofs) for a guide to triaging these. - +* **SQLite bugs** can be assigned to huangdarwin@. CC drhsqlite@ for upstream + issues. Tips for reproducing bugs: diff --git a/chromium/docs/security/web-mitigation-metrics.md b/chromium/docs/security/web-mitigation-metrics.md index b8d797037c1..c2c1dc4da0e 100644 --- a/chromium/docs/security/web-mitigation-metrics.md +++ b/chromium/docs/security/web-mitigation-metrics.md @@ -86,7 +86,7 @@ usage we obtain the following usage counts: * Tracking specific features: `kTrustedTypesPolicyCreated` tracks creation of all Trusted Types policies, `kTrustedTypesDefaultPolicyCreated` - notes whether a "default" policy has been created. `kTrustedTypesAllowDuplicates` + notes whether a "default" policy has been created. `kTrustedTypesAllowDuplicates` records whether an 'allow-duplicates' keyword has been used. * Error tracking: `kTrustedTypesAssignmentError` tracks whether Trusted Types @@ -148,3 +148,21 @@ for pages that set COOP to "same-origin" and COEP to "require-corp". [coep]: https://wicg.github.io/cross-origin-embedder-policy/ [coop]: https://gist.github.com/annevk/6f2dd8c79c77123f39797f6bdac43f3e +## Sanitizer API + +[The Sanitizer API][sanitizer] provides a browser-maintained "ever-green", safe, +and easy-to-use library for user input sanitization as part of the general web +platform. + +* Sanitizer creation: `kSanitizerAPICreated` and + `kSanitizerAPIDefaultConfiguration` tell us how many Sanitizers are + created and how many Sanitizers are created without custom configurations. +* Sanitizing process: `kSanitizerAPIToString` and + `kSanitizerAPIToFragment` counts the usage of two methods, + `Sanitizer::sanitizeToString` and `Sanitizer::sanitize`. +* `kSanitizerAPIActionTaken` shows how many times do the + actual sanitize action has been performed while calling the Sanitizer APIs. +* Input type: `kSanitizerAPIFromString`, `kSanitizerAPIFromDocument` and + `kSanitizerAPIFromFragment` tell us what kind of input people are using. + +[sanitizer]: https://wicg.github.io/sanitizer-api/ diff --git a/chromium/docs/sheriff.md b/chromium/docs/sheriff.md index d804d0c0432..44d1a90222f 100644 --- a/chromium/docs/sheriff.md +++ b/chromium/docs/sheriff.md @@ -20,8 +20,14 @@ necessary authority to fulfill them. In particular, you have the authority to: * Revert changes that you know or suspect are causing breakages * Disable or otherwise mark misbehaving tests -* Use TBRs freely as part of your sheriffing duties +* Use Owners-Override label to override OWNERS checks freely as part of your + sheriffing duties * Pull in any other engineer or team you need to help you do these duties +* For clean reverts and cherry-picks, add the + [Rubber Stamper bot](code_reviews.md#automated-code_review). All other + changes require a +1 from another committer. + +TBRs were removed in Q1 2021. ## How to be a Sheriff diff --git a/chromium/docs/speed/benchmark/harnesses/desktop_ui.md b/chromium/docs/speed/benchmark/harnesses/desktop_ui.md new file mode 100644 index 00000000000..63d3d9053e9 --- /dev/null +++ b/chromium/docs/speed/benchmark/harnesses/desktop_ui.md @@ -0,0 +1,95 @@ +# Desktop UI Benchmark + +## Overview + +Desktop UI Benchmark is used to measure and monitor Desktop UI performance based on the [Telemetry](https://chromium.googlesource.com/catapult/+/HEAD/telemetry/README.md) framework. +It captures important metrics such as + +* Initial load time +* Subsequent load time +* Fetch data time +* Search user input time +* Frame time when scrolling + +in different stories representing different scenarios such as +* tab_search:top10:2020 - Test CUJs with 10 open tabs, after all tabs are loaded +* tab_search:top50:2020 - Test CUJs with 50 open tabs, after all tabs are loaded +* tab_search:top100:2020 - Test CUJs with 100 open tabs, after all tabs are loaded +* tab_search:top10:loading:2020 - Test CUJs with 10 open tabs, before all tabs are loaded +* tab_search:top50:loading:2020 - Test CUJs with 50 open tabs, before all tabs are loaded +* tab_search:top100:loading:2020 - Test CUJs with 100 open tabs, before all tabs are loaded +* tab_search:close_and_open:2020 - Test open, close and reopen Tab Search UI +* tab_search:scroll_up_and_down:2020 - Test srolling down, up and down 100 tabs, before all tabs are loaded + + +For more information please see this [doc](https://docs.google.com/document/d/1-1ijT7wt05hlBZmSKjX_DaTCzVqpxbfTM1y-j7kYHlc). + +## Run the benchmark on pinpoint + +In most cases, you only need to run the benchmark on [pinpoint](https://pinpoint-dot-chromeperf.appspot.com/) before/after a CL that may have performance impact. If you don't have platform requirement linux-perf is recommended since they are the most stable trybots for this benchmark. + + +## Run the benchmark locally + +In some cases, if trybots cannot meet your requirement or you need to debug on your own machine, use the following command to run the benchmark locally. You need an @google account to be able to do that. + +``` +tools/perf/run_benchmark run desktop_ui --browser-executable=out/Default/chrome --story-filter=tab_search:top10:2020 --pageset-repeat=3 +``` + + +## Add new metrics + +There are 3 ways to add metrics to the benchmarking code + +1. Add UMA metrics to your code and include them in the [story definition](../../../../tools/perf/page_sets/desktop_ui/tab_search_story.py). The listed UMA metrics will show up on the result page automatically. +2. Add C++ trace with name starts with "custom_metric:". For example: + ```c++ + void Foo::DoWork() { + TRACE_EVENT0("browser", "custom_metric:Foo:DoWork"); + ... + } + ``` + This method requires 'customMetric' added to your story definition. If your story extends from [MultiTabStory](../../../../tools/perf/page_sets/desktop_ui/multitab_story.py) it's already been taken care of. Also make sure to enable the trace categories you want to add in your story definition. + +3. Add Javascript performance.mark() with names end with ":benchmark_begin" and ":benchmark_end". Time between performance.mark('<YOUR_METRIC_NAME>:benchmark_begin') and performance.mark('<YOUR_METRIC_NAME>:benchmark') will show up as YOUR_METRIC_NAME on the result page. For example: + ```javascript + function calc() { + performance.mark('calc_time:benchmark_begin'); + ... + performance.mark('calc_time:benchmark_end'); + } + ``` + You can also emit metric value directly using performance.mark('<YOUR_METRIC_NAME>:<YOUR_METRIC_VALUE>:benchmark_value'). In the case multiple values needs to be measured asynchronously it's better to do the following instead: + ```javascript + const startTime = performance.now(); + for (const url of urls) { + fetch(url).then(() => { + performance.mark(`fetch_time:${performance.now() - startTime}:benchmark_value`); + }); + } + ``` + This method requires 'customMetric' added to your story definition and enable the trace category 'blink.user_timing'. If your story extends from [MultiTabStory](../../../../tools/perf/page_sets/desktop_ui/multitab_story.py) it's already been taken care of. + + +## Add new stories + +Add new stories to [here](../../../../tools/perf/page_sets/desktop_ui/desktop_ui_stories.py). +Generally we want to put most of the user journeys in the main story so we only need to run 1 story to verify a CL in most cases. However, if the user journey may affect metrics of other user journeys, you should make it a separate story. + +- If your new stories don't need external network requests, you are done now. +- If your new stories need external network requests and extend from MultiTabStory or other existing stories, add an item to [here](../../../../tools/perf/page_sets/data/desktop_ui.json) to reuse some of the existing recorded stories +- If your new stories need external network requests that can't be reused from existing stories, follow the next section to record your own stories. + +Finally, run the new stories locally to make sure they work, then upload the CL and run a pinpoint job to make sure they work before starting code review. + +## Record new stories + +Use the following command to record a story +``` +tools/perf/record_wpr --browser-executable=out/Default/chrome desktop_ui --story-filter=<YOUR_STORY_NAME> +``` +and the following command to upload to the cloud. +``` +upload_to_google_storage.py --bucket chrome-partner-telemetry tools/perf/page_sets/data/desktop_ui_<YOUR_RECORDED_HASH>.wprgo +```
\ No newline at end of file diff --git a/chromium/docs/speed/benchmark/harnesses/webrtc_perf.md b/chromium/docs/speed/benchmark/harnesses/webrtc_perf.md index 07523bfc59f..59498c94426 100644 --- a/chromium/docs/speed/benchmark/harnesses/webrtc_perf.md +++ b/chromium/docs/speed/benchmark/harnesses/webrtc_perf.md @@ -44,7 +44,10 @@ the following command: ./tools/perf/run_benchmark webrtc --browser-executable=out/Release/chrome --story-tag-filter=stress ``` - +Or you can run the single story directly: +``` +./tools/perf/run_benchmark webrtc --story multiple-peerconnections --browser-executable=out/Release/chrome +``` ## Adding Telemetry Tests for WebRTC diff --git a/chromium/docs/speed/binary_size/metrics.md b/chromium/docs/speed/binary_size/metrics.md index 5105f178408..0ec41ce24bf 100644 --- a/chromium/docs/speed/binary_size/metrics.md +++ b/chromium/docs/speed/binary_size/metrics.md @@ -47,6 +47,7 @@ For Googlers, more information available at [go/chrome-apk-size](https://goto.go * With all translations as if they were not missing (estimates size of missing translations based on size of english strings). * Why: Without translation-normalization, translation dumps cause jumps. * Translation-normalization applies only to apks (not to Android App Bundles). + * For Android App Bundles, the normalized size is the sum of the normalized size of all splits that have onDemand="false" (those installed by default). ### Native Code Size Metrics diff --git a/chromium/docs/speed/binary_size/optimization_advice.md b/chromium/docs/speed/binary_size/optimization_advice.md index 8505065ee86..66156ee1e2c 100644 --- a/chromium/docs/speed/binary_size/optimization_advice.md +++ b/chromium/docs/speed/binary_size/optimization_advice.md @@ -113,6 +113,10 @@ There are two mechanisms for compressing Chrome l10n files. ## Android Focused Advice +Googlers: See also [go/abp-performance/apk-size]. + +[go/abp-performance/apk-size]: https://goto.google.com/abp-performance/apk-size + ### How To Tell if It's Worth Spending Time on Binary Size? * Binary size is a shared resource, and thus its growth is largely due to the @@ -201,10 +205,11 @@ Practical advice: .pak files. * Gut-check that all unique string literals being added are actually useful. * If there's a notable increase in `.text`: - * If there are a lot of symbols from C++ templates, try moving functions - that don't use template parameters to - [non-templated helper functions][template_bloat]). - * And extract parts of functions that don't use them into helper functions. + * If there are a lot of symbols from C++ templates: + * Try moving parts of the templatized function that don't use the template + parameters to [non-templated helper functions][template_bloat_one]). + * Or see if the signature can be re-worked such that there are + [fewer variants of template parameters](template_bloat_two). * Try to leverage identical-code-folding as much as possible by making the shape of your code consistent. * E.g. Use PODs wherever possible, and especially in containers. They will @@ -231,9 +236,6 @@ Practical advice: * In C++, static objects are created at compile time, but in Java they are created by executing code within `<clinit>()`. There is often little advantage to initializing class fields statically vs. upon first use. - * Don't use default interface methods on interfaces with multiple implementers. - * Desugaring causes the methods to be added to every implementer separately. - * It's more efficient to use a base class to add default methods. * Use `String.format()` instead of concatenation. * Concatenation causes a lot of StringBuilder code to be generated. * Try to use default values for fields rather than explicit initialization. @@ -256,7 +258,8 @@ Practical advice: [size-trybot]: /tools/binary_size/README.md#Binary-Size-Trybot-android_binary_size [diagnose_bloat]: /tools/binary_size/README.md#diagnose_bloat_py [relocations]: /docs/native_relocations.md -[template_bloat]: https://bugs.chromium.org/p/chromium/issues/detail?id=716393 +[template_bloat_one]: https://bugs.chromium.org/p/chromium/issues/detail?id=716393 +[template_bloat_two]: https://chromium-review.googlesource.com/c/chromium/src/+/2639396 [supersize-console]: /tools/binary_size/README.md#Usage_console ### Optimizing Third-Party Android Dependencies diff --git a/chromium/docs/speed/bot_health_sheriffing/how_to_disable_a_story.md b/chromium/docs/speed/bot_health_sheriffing/how_to_disable_a_story.md index 0f8503d8db4..df549ef9c2f 100644 --- a/chromium/docs/speed/bot_health_sheriffing/how_to_disable_a_story.md +++ b/chromium/docs/speed/bot_health_sheriffing/how_to_disable_a_story.md @@ -65,7 +65,9 @@ Once you've committed your changes locally, your CL can be submitted with: *Please make sure to CC the benchmark owner so that they're aware that they've lost coverage.* -The `Tbr:` and `No-Try:` are permitted and recommended so long as the only file changed is `tools/perf/expectations.config`. If your change touches real code rather than just that configuration data, you'll need a real review before submitting it. +The `Tbr:` and `No-Try:` are permitted and recommended so long as the only file changed is `tools/perf/expectations.config`. +To submit a `Tbr:` change, you will need to add `Rubber Stamper (rubber-stamper@appspot.gserviceaccount.com)` as a reviewer to the patch and wait for the bot's approval. +If your change touches real code rather than just that configuration data, you'll need a real review before submitting it. # How to disable a failing gtest perf test diff --git a/chromium/docs/speed/metrics_changelog/2020_10_cls_2.md b/chromium/docs/speed/metrics_changelog/2020_10_cls_2.md index 61e24d9906c..5a4dbf1f8c1 100644 --- a/chromium/docs/speed/metrics_changelog/2020_10_cls_2.md +++ b/chromium/docs/speed/metrics_changelog/2020_10_cls_2.md @@ -2,6 +2,7 @@ ## Changes in Chrome 87 +### Bug fix to impact fraction calculation In Chrome 86, a [problem was introduced](2020_10_cls.md) to the way the area of the [impact fraction](https://web.dev/cls/#impact-fraction) is calculated for layout shifts of nodes containing positioned content that overflows to the @@ -14,10 +15,22 @@ Before change | After ------------- | -----  |  +### Bug fix to elements with contain:paint styling + +A bug fix was made to how the overflow of shifted elements inside a container +that is styled `contain:paint` is calculated, reducing the computed size of the +shift area for these nodes. +[Source code for this change](https://chromium-review.googlesource.com/c/chromium/src/+/2417061). + ## How does this affect a site's metrics? -Sites which showed this regression in Chrome 86 should return to accurate layout -shift scores in Chrome 87. +Sites whose CLS scores were negatively impacted by the +[regression in impact fraction calculation](2020_10_cls.md) in Chrome 86 should +return to accurate layout shift scores in Chrome 87. Developers can test in +Chrome 87.0.4280.51 and later to see the impact of this change. + +Sites with contain:paint styling on elements may see an improvement in CLS +scores in Chrome 87. ## When were users affected? diff --git a/chromium/docs/speed/metrics_changelog/2020_11_cls.md b/chromium/docs/speed/metrics_changelog/2020_11_cls.md index de6b42106e0..019570da0e7 100644 --- a/chromium/docs/speed/metrics_changelog/2020_11_cls.md +++ b/chromium/docs/speed/metrics_changelog/2020_11_cls.md @@ -26,6 +26,15 @@ Going forward, there should be no CLS penalty for such elements. (Note that there still may be a layout shift for onscreen elements adjacent to (but not descendants of) the `content-visibility: auto` element. +### Ignore layout shift when visibility:hidden becomes visible + +We have been always ignoring layout shifts when the element has +visibility:hidden. However before Chrome 89, if the element having +visibility:hidden became visible and shift at the same time, we reported layout +shift. Now we also consider the previous visibility and ignore layout shift in +the case. +[Source code for this change](https://chromium-review.googlesource.com/c/chromium/src/+/2591367) + ## How does this affect a site's metrics? All of these changes only affect sites with specific types of content. Here are @@ -47,6 +56,18 @@ Sites using `content-visibility: auto` should no longer see any impact of this feature on their Cumulative Layout Shift scores, resulting in a decrease in their scores. +### Ignore layout shift when visibility:hidden becomes visible + +Sites using visibility:hidden to hide layout changes may see a decrease in +their Cumulative Layout Shift scores. + +### Fixed bug when recording paint offset translation deltas + +A bug was introduced in Chrome 86 in which Cumulative Layout Shift was measured +incorrectly for certain cases involving nested out-of-flow elements +(absolute-position or fixed-position). This caused reported shifts even in +some cases where there was not actually a shift. + ## When were users affected? Chrome 88 is currently scheduled to be released the week of January 19, 2021. diff --git a/chromium/docs/speed/metrics_changelog/2020_11_fcp.md b/chromium/docs/speed/metrics_changelog/2020_11_fcp.md new file mode 100644 index 00000000000..5a0bd24e1cb --- /dev/null +++ b/chromium/docs/speed/metrics_changelog/2020_11_fcp.md @@ -0,0 +1,22 @@ +# First Contentful Paint Changes in M89 + +## Changes in Chrome 89 + +In Chrome 89, an improvement was made to the code that reports First Contentful +Paint to [URL-Keyed Metrics](https://chromium.googlesource.com/chromium/src/+/master/services/metrics/ukm_api.md). +This change increases the number of reports of First Contentful Paint that Chrome +receives right before the user closes a tab. +[Source code for this change](https://chromium-review.googlesource.com/c/chromium/src/+/2552628). + +## How does this affect a site's metrics? + +We don't see any difference in the values produced by the metric after this +change. However, after this change Chrome will now receive more reports of +First Contentful Paint, which means that some sites which previously had too +little data to be included in the +[Chrome User Experience Report](https://developers.google.com/web/tools/chrome-user-experience-report) +will now be included, especially on desktop where tab switching is more common. + +## When were users affected? + +Chrome 89 is currently scheduled to be released the week of March 2, 2021. diff --git a/chromium/docs/speed/metrics_changelog/2020_11_lcp.md b/chromium/docs/speed/metrics_changelog/2020_11_lcp.md new file mode 100644 index 00000000000..f4ce887d62f --- /dev/null +++ b/chromium/docs/speed/metrics_changelog/2020_11_lcp.md @@ -0,0 +1,54 @@ +# Largest Contentful Paint Changes in M88 + +## Changes in Chrome 88 + +In Chrome 88, some changes were made to Largest Contentful Paint to bring its +implementation in line with the updated [specification](https://wicg.github.io/largest-contentful-paint/), +and fix bugs in the chromium implementation. + +The changes are: + + * Full viewport images, which are visually equivalent to background images, are + no longer considered as the largest contentful paint. + [Source code for this change](https://chromium-review.googlesource.com/c/chromium/src/+/2441732). + * Images which are later removed from the page are now considered as potential + largest contentful paints. + [Source code for this change](https://chromium-review.googlesource.com/c/chromium/src/+/2480845). + * Largest Contentful Paint stops recording after an input in an out of process + iframe. This change only affects Chrome's reporting of Largest Contentful + Paint in the [Chrome User Experience Report](https://developers.google.com/web/tools/chrome-user-experience-report), + as the API doesn't include iframes. + [Source code for this change](https://chromium-review.googlesource.com/c/chromium/src/+/2436770). + * Prior to Chrome 88, Largest Contentful Paint had a bug where it incorrectly + classified some images as invisible. This could happen in the case of + offscreen or tiny placeholder images which were replaced with large visible + images. The bug has been fixed, and the large visible images are now counted + as possible largest contentful paints. + [Source code for this change](https://chromium-review.googlesource.com/c/chromium/src/+/2561270). + +## How does this affect a site's metrics? + +Each change only affects sites with specific types of content. + +The change to ignore full viewport images as potential largest contentful paints +will improve Largest Contentful Paint times for pages with background images. +This is intended as Largest Contentful Paint attempts to time when the main +content on the page is visible, and this generally is +[not background images](https://github.com/anniesullie/LCP_Examples/blob/master/body_background/README.md). + +The change to include images which are later removed from the DOM as possible +largest contentful paints will improve Largest Contentful Paint times on sites +which have images of the same size inserted multiple times. This is a common +pattern for carousels, as well as some JavaScript frameworks which do +server-side rendering. + +The change to stop recording after an input in an out of process iframe will +improve Largest Contentful Paint times on sites which have cross-process iframes. + +The bug fix for invisible elements may result in increased Largest Contentful +Paint timings for sites using a placeholder pattern that swaps in the largest +contentful image. + +## When were users affected? + +Chrome 88 is currently scheduled to be released the week of January 19, 2021. diff --git a/chromium/docs/speed/metrics_changelog/2020_12_cls.md b/chromium/docs/speed/metrics_changelog/2020_12_cls.md index 6f71bdf7569..1aa12c842f4 100644 --- a/chromium/docs/speed/metrics_changelog/2020_12_cls.md +++ b/chromium/docs/speed/metrics_changelog/2020_12_cls.md @@ -2,24 +2,32 @@ ## Changes in Chrome 89 -### Ignore layout shift when visibility:hidden becomes visible +### Ignore layout shift under opacity:0 -We have been always ignoring layout shifts when the element has -visibility:hidden. However before Chrome 89, if the element having -visibility:hidden became visible and shift at the same time, we reported layout -shift. Now we also consider the previous visibility and ignore layout shift in -the case. -[Source code for this change](https://chromium-review.googlesource.com/c/chromium/src/+/2591367) +[Source code for this change](https://chromium-review.googlesource.com/c/chromium/src/+/2591907) + +### Clip layout shift rect by visual viewport + +On user agents supporting mobile viewport, the actual visual viewport may be +smaller than the layout viewport. Now layout shifts out of the visual viewport +are ignored. +[Source code for this change](https://chromium-review.googlesource.com/c/chromium/src/+/2593180) ## How does this affect a site's metrics? All of these changes only affect sites with specific types of content. Here are the specifics for each change: -### Ignore layout shift when visibility:hidden becomes visible +### Ignore layout shift under opacity:0 + +Sites using opacity:0 to hide layout changes should see a decrease in their +Cumulative Layout Shift scores. + +### Clip layout shift rect by visual viewport -Sites using visibility:hidden to hide layout changes may see a decrease in -their Cumulative Layout Shift scores. +Sites may see a decrease in their Cumulative Layout Shift scores on user agents +supporting mobile viewport when the visual viewport is smaller than the layout +viewport. ## When were users affected? diff --git a/chromium/docs/speed/metrics_changelog/2021_02_cls.md b/chromium/docs/speed/metrics_changelog/2021_02_cls.md new file mode 100644 index 00000000000..c2a30a4ddca --- /dev/null +++ b/chromium/docs/speed/metrics_changelog/2021_02_cls.md @@ -0,0 +1,36 @@ +# Cumulative Layout Shift Changes in Chrome 90 + +### Bug fixes involving changes to transform, effect, clip or position + +[Source code for change 1](https://chromium-review.googlesource.com/c/chromium/src/+/2660679) +[Source code for change 2](https://chromium-review.googlesource.com/c/chromium/src/+/2666949) +[Source code for change 3](https://chromium-review.googlesource.com/c/chromium/src/+/2665761) +[Source code for change 4](https://chromium-review.googlesource.com/c/chromium/src/+/2690998) + +### Consider transform change countering layout shift + +Corresponds to [the spec change](https://github.com/WICG/layout-instability/pull/94) +[Source code](https://chromium-review.googlesource.com/c/chromium/src/+/2673965) + +### Ignore layout shift for more invisible elements + +The following nodes are ignored for layout shift: +* texts with unrenderable font or containing all whitespaces, +* blocks without any decorations or any children. + +If you still see layout shift reported for an invisible element, you can +try to add 'visibility:hidden' to the element's style. + +[Source code](https://chromium-review.googlesource.com/c/chromium/src/+/2743811) + +### Ignore inline direction shift moving from/to out of view + +[Source code](https://chromium-review.googlesource.com/c/chromium/src/+/2747689) + +### Improvement for shift with counterscroll + +[Source code](https://chromium-review.googlesource.com/c/chromium/src/+/2741240) + +## When were users affected? + +Chrome 90 is currently scheduled to be released the week of April 13, 2021. diff --git a/chromium/docs/speed/metrics_changelog/cls.md b/chromium/docs/speed/metrics_changelog/cls.md index 5867effffa1..a12683234f0 100644 --- a/chromium/docs/speed/metrics_changelog/cls.md +++ b/chromium/docs/speed/metrics_changelog/cls.md @@ -2,14 +2,23 @@ This is a list of changes to [Cumulative Layout Shift](https://web.dev/cls). +* Chrome 90 + * Metric definition improvement: [Bug fixes involving changes to transform, effect, clip or position](2021_02_cls.md) + * Metric definition improvement: [Consider transform change countering layout shift](2021_02_cls.md) + * Metric definition improvement: [Ignore layout shift for more invisible elements](2021_02_cls.md) + * Metric definition improvement: [Ignore inline direction shift moving from/to out of view](2021_02_cls.md) + * Metric definition improvement: [Improvement for shift with counterscroll](2021_02_cls.md) * Chrome 89 - * Metric definition improvement: [Ignore layout shift when visibility:hidden becomes visible](2020_12_cls.md) + * Metric definition improvement: [Ignore layout shift under opacity:0](2020_12_cls.md) + * Metric definition improvement: [Clip layout shift rect by visual viewport](2020_12_cls.md) * Chrome 88 * Metric definition improvement: [Cumulative layout shift properly detects shifts of fixed position elements](2020_11_cls.md) * Metric definition improvement: [Cumulative layout shift properly detects shifts of descendents of a sticky element](2020_11_cls.md) * Metric definition improvement: [no penalty for content-visibility: auto content](2020_11_cls.md) + * Metric definition improvement: [Ignore layout shift when visibility:hidden becomes visible](2020_11_cls.md) * Chrome 87 * Metric definition improvement: [Fix problem in Cumulative Layout shift calculation of impact region](2020_10_cls_2.md) + * Metric definition improvement: [Cumulative Layout Shift properly handles clipping of elements styled contain:paint](2020_10_cls_2.md) * Chrome 86 * Metric definition changes and bug: [Cumulative Layout Shift score changes and regressions in impact region calculation](2020_10_cls.md) * Chrome 85 diff --git a/chromium/docs/speed/metrics_changelog/fcp.md b/chromium/docs/speed/metrics_changelog/fcp.md index ef2c9f4798c..e6309b31f58 100644 --- a/chromium/docs/speed/metrics_changelog/fcp.md +++ b/chromium/docs/speed/metrics_changelog/fcp.md @@ -2,6 +2,8 @@ This is a list of changes to [First Contentful Paint](https://web.dev/fcp). +* Chrome 89 + * Metric definition improvement: [First Contentful Paint reported for quick tab switches](2020_11_fcp.md) * Chrome 86 * Metric definition improvements: [First Contentful Paint reported for videos, webgl2 canvases, and non-contentful SVG](2020_07_fcp.md) * Chrome 84 diff --git a/chromium/docs/speed/metrics_changelog/lcp.md b/chromium/docs/speed/metrics_changelog/lcp.md index 5d114ddf428..510e60e0989 100644 --- a/chromium/docs/speed/metrics_changelog/lcp.md +++ b/chromium/docs/speed/metrics_changelog/lcp.md @@ -2,6 +2,11 @@ This is a list of changes to [Largest Contentful Paint](https://web.dev/lcp). +* Chrome 88 + * Metric definition improvement: [Largest Contentful Paint ignores full viewport images](2020_11_lcp.md) + * Metric definition improvement: [Largest Contentful Paint stops recording after input in an iframe](2020_11_lcp.md) + * Metric definition improvement: [Largest Contentful Paint ignores removed content by default](2020_11_lcp.md) + * Metric definition improvement: [Largest Contentful Paint bug fix for some images with source chagnes](2020_11_lcp.md) * Chrome 86 * Metric definition improvement: [Largest Contentful Paint ignores paints with opacity 0](2020_08_lcp.md) * Chrome 83 diff --git a/chromium/docs/speed/perf_lab_platforms.md b/chromium/docs/speed/perf_lab_platforms.md index 72c441f03a6..30766be44b3 100644 --- a/chromium/docs/speed/perf_lab_platforms.md +++ b/chromium/docs/speed/perf_lab_platforms.md @@ -13,18 +13,26 @@ * [android-pixel2-perf](https://ci.chromium.org/p/chrome/builders/ci/android-pixel2-perf): Android OPM1.171019.021. * [android-pixel2_weblayer-perf](https://ci.chromium.org/p/chrome/builders/ci/android-pixel2_weblayer-perf): Android OPM1.171019.021. * [android-pixel2_webview-perf](https://ci.chromium.org/p/chrome/builders/ci/android-pixel2_webview-perf): Android OPM1.171019.021. + * [android-pixel4-perf](https://ci.chromium.org/p/chrome/builders/ci/android-pixel4-perf): Android R. + * [android-pixel4_weblayer-perf](https://ci.chromium.org/p/chrome/builders/ci/android-pixel4_weblayer-perf): Android R. + * [android-pixel4_webview-perf](https://ci.chromium.org/p/chrome/builders/ci/android-pixel4_webview-perf): Android R. * [android-pixel4a_power-perf](https://ci.chromium.org/p/chrome/builders/ci/android-pixel4a_power-perf): Android QD4A.200102.001.A1. +## Chromeos + + * [lacros-eve-perf](https://ci.chromium.org/p/chrome/builders/ci/lacros-eve-perf): . + ## Linux - * [linux-perf](https://ci.chromium.org/p/chrome/builders/ci/linux-perf): Ubuntu-14.04, 8 core, NVIDIA Quadro P400. + * [linux-perf](https://ci.chromium.org/p/chrome/builders/ci/linux-perf): Ubuntu-18.04, 8 core, NVIDIA Quadro P400. + * [linux-perf-rel](https://ci.chromium.org/p/chrome/builders/ci/linux-perf-rel): Ubuntu-18.04, 8 core, NVIDIA Quadro P400. ## Mac * [mac-10_12_laptop_low_end-perf](https://ci.chromium.org/p/chrome/builders/ci/mac-10_12_laptop_low_end-perf): MacBook Air, Core i5 1.8 GHz, 8GB RAM, 128GB SSD, HD Graphics. * [mac-10_13_laptop_high_end-perf](https://ci.chromium.org/p/chrome/builders/ci/mac-10_13_laptop_high_end-perf): MacBook Pro, Core i7 2.8 GHz, 16GB RAM, 256GB SSD, Radeon 55. * [mac-arm_dtk_arm-perf](https://ci.chromium.org/p/chrome/builders/ci/mac-arm_dtk_arm-perf): Mac ARM DTK (ARM Chrome). - * [mac-arm_dtk_x86-perf](https://ci.chromium.org/p/chrome/builders/ci/mac-arm_dtk_x86-perf): Mac ARM DTK (X86 Chrome). + * [mac-m1_mini_2020-perf](https://ci.chromium.org/p/chrome/builders/ci/mac-m1_mini_2020-perf): Mac M1 Mini 2020. ## Win diff --git a/chromium/docs/speed/perf_regression_sheriffing.md b/chromium/docs/speed/perf_regression_sheriffing.md index 33375a0e350..a4cdf797b18 100644 --- a/chromium/docs/speed/perf_regression_sheriffing.md +++ b/chromium/docs/speed/perf_regression_sheriffing.md @@ -62,9 +62,10 @@ issues: ## Follow up on Performance Regressions Please spend any spare time driving down bugs from the [regression -backlog](http://go/triage-backlog). Treat these bugs as you would your own -- -investigate the regressions, find out what the next step should be, and then -move the bug along. Some possible next steps and questions to answer are: +backlog](https://bugs.chromium.org/p/chromium/issues/list?can=2&q=Performance%3DSheriff+Type%3ABug+modified-before%3Atoday-6&sort=-modified). +Treat these bugs as you would your own -- investigate the regressions, find out +what the next step should be, and then move the bug along. Some possible next steps +and questions to answer are: * Should the bug be closed? * Are there questions that need to be answered? diff --git a/chromium/docs/speed_metrics/webperf_okrs.md b/chromium/docs/speed_metrics/webperf_okrs.md index 8e3d775ef45..28aa641e6f5 100644 --- a/chromium/docs/speed_metrics/webperf_okrs.md +++ b/chromium/docs/speed_metrics/webperf_okrs.md @@ -2,6 +2,65 @@ [TOC] +## 2021 Q1 Objectives + +* **performance.measureMemory**: ship the API. +* **Single Page Apps**: + * Publish an explainer about SPA issues. + * Determine whether User Timing hints conventions are still useful. +* **Abandonment**: + * Gather concrete feedback form analytics providers and other potential users of this API. + * Improve confidence on the abandonment rates computed by implementing a renderer-side flushing. + * Update analysis on rates when the above fix has reached Chrome Stable. +* **Smoothness**: + * Continue refining the definition of dropped frames. + * Move proposal to WICG. +* **Back-forward cache**: + * Expand scope to include FCP and FID in the values reported after back-forward navigations. + * Investigate backwards compatibility of adding new entries and propose an API shaped based on the outcome. +* **Responsiveness**: + * Investigate correctness of existing internal metrics and implement fixes as needed. + * Further investigate scrolling performance and how it should be integrated with metric. + * Define user interactions that we care about for this API. + * Create a manual test corpus to test ideas about the 'end time' of a user interaction. +* **First Contentful Paint**: improve implementation to pass more + [tests](https://wpt.fyi/results/paint-timing?label=master&label=experimental). +* **Longtasks**: add system time, including garbage collection + ([bug](https://bugs.chromium.org/p/chromium/issues/detail?id=1091754)). + * Present proposal to security team, and begin socializing the proposal externally. +* **A/B testing**: organize workshop on client-side A/B testing. +* **JS Sampling Profiler**: + * Complete the GC integration work. + * Ship the API. + +## 2020 Q4 Progress + +### New web performance APIs + +* **performance.measureMemory**: added support for cross-origin iframes in the same process and sent + [Intent to Ship](https://groups.google.com/a/chromium.org/g/blink-dev/c/RExJ9a3SmQw). +* **Page abandonment**: made some data available publicly and socialized it in a + [blogpost](https://calendar.perfplanet.com/2020/abandonment/). +* **JS Sampling Profiler**: + * Implemented the API so it requires COOP/COEP and gated it behind Document Policy. + * Finished a prototype of GC integration for the V8 sampling profiler (which will help reduce profiler startup time). + * Landed some initial support for code object refcounting. +* **Smoothness**: published a proposal around dropped frames and presented it at TPAC. +* **Back-forward cache**: determined that it is backwards compatible to expose a PerformanceNavigationTiming + entry for back-forward navigations. +* **Responsiveness**: + * Investigated some internal metrics, but found some metric quality issues that need to be investigated. + * Started brainstorm on capturing asynchronous work as well as which user interactions to capture. + * Did investigation on scrolling and determined that in most cases pages do not seem to suffer from poor + scrolling performance. + +### Existing web performance API improvements + +* **Largest Contentful Paint**: include removed nodes ([bug](https://bugs.chromium.org/p/chromium/issues/detail?id=1045640)) + and ignored images occupying the full viewport ([bug](https://bugs.chromium.org/p/chromium/issues/detail?id=1133883)). +* **Cumulative Layout Shift**: implemented various fixes, see + [changelog](https://chromium.googlesource.com/chromium/src/+/master/docs/speed/metrics_changelog/README.md). + ## 2020 Q4 Objectives ### New web performance APIs diff --git a/chromium/docs/static_initializers.md b/chromium/docs/static_initializers.md index d8fee93e896..790506aa31e 100644 --- a/chromium/docs/static_initializers.md +++ b/chromium/docs/static_initializers.md @@ -6,6 +6,8 @@ Some background on the original decision to ban static initializers: http://neugierig.org/software/chromium/notes/2011/08/static-initializers.html +Note: Another name for static initializers is "global constructors". + # How Static Initializers are Checked * For Linux and Mac: @@ -23,17 +25,7 @@ Common fixes include: ## Listing Static Initializers -### Method 1 - Ask compiler to report them -1. Edit [//build/config/BUILDCONFIG.gn](https://cs.chromium.org/chromium/src/build/config/BUILDCONFIG.gn) -and add `"//build/config/compiler:wglobal_constructors"` to `default_compiler_configs` -2. Set GN arg `treat_warnings_as_errors=false` -3. Compile and look at warnings - -This will produce far more warnings than there are static initializers because -it (seems to) trigger on global variables initialized with constexpr -constructors. - -### Method 2 - Use objdump to report them +### Step 1 - Use objdump to report them For Linux: tools/linux/dump-static-initializers.py out/Release/chrome @@ -46,8 +38,32 @@ For Android (from easiest to hardest): ninja chrome/android:monochrome_static_initializers # or: tools/binary_size/diagnose_bloat.py HEAD # See README.md for flags. - # or: - tools/linux/dump-static-initializers.py --toolchain-prefix third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/arm-linux-androideabi- out/Release/libmonochrome.so + # or (the other two use this under the hood): + tools/linux/dump-static-initializers.py --toolchain-prefix third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/arm-linux-androideabi- out/Release/lib.unstripped/libmonochrome.so + # arm32 ^^ vv arm64 + tools/linux/dump-static-initializers.py --toolchain-prefix third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android- out/Release/lib.unstripped/libmonochrome.so + # Note: For arm64, having use_thin_lto=true seems to dump a couple extra + # initializers that don't actually exist. + +The last one may actually be the easiest if you've already properly built +`libmonochrome.so` with `is_official_build=true`. + +### Step 2 - Ask compiler to report them + +If the source of the new initiazers is not obvious from Step 1, you can ask the +compiler to pinpoint the exact source line. + +1. Edit [//build/config/BUILDCONFIG.gn](https://cs.chromium.org/chromium/src/build/config/BUILDCONFIG.gn) +and add `"//build/config/compiler:wglobal_constructors"` to `default_compiler_configs` +2. Remove the config from the `configs` in `//base:base` +3. Set GN arg `treat_warnings_as_errors=false` +4. Compile and look for warnings **from the files identified by step 1** (may want to pipe ninja output to a file). + +*** note +The compiler warning triggers for every static initializer that exists +*before optimization*. We care only about those that survive optimization. +More details in [crbug/1136086](https://bugs.chromium.org/p/chromium/issues/detail?id=1136086). +*** * For more information about `diagnose_bloat.py`, refer to its [README.md](/tools/binary_size/README.md#diagnose_bloat.py) * List of existing static initializers documented in [static_initializers.gni](/chrome/android/static_initializers.gni) diff --git a/chromium/docs/sync/model_api.md b/chromium/docs/sync/model_api.md index 5ab353a0a59..94fea98e814 100644 --- a/chromium/docs/sync/model_api.md +++ b/chromium/docs/sync/model_api.md @@ -10,7 +10,7 @@ API] (aka Directory), which as of mid-2019 is still used by several legacy model types, but "wrapped into" USS (see [SyncableServiceBasedBridge]). [SyncableService API]: https://www.chromium.org/developers/design-documents/sync/syncable-service-api -[SyncableServiceBasedBridge]: https://cs.chromium.org/chromium/src/components/sync/model_impl/syncable_service_based_bridge.h +[SyncableServiceBasedBridge]: https://cs.chromium.org/chromium/src/components/sync/model/syncable_service_based_bridge.h [TOC] @@ -81,7 +81,7 @@ to add one (e.g. a GUID, though be wary as they have the potential to conflict). While the hash gets written to disk as part of the metadata, the tag itself is never persisted locally. -[EntityData]: https://cs.chromium.org/chromium/src/components/sync/model/entity_data.h +[EntityData]: https://cs.chromium.org/chromium/src/components/sync/engine/entity_data.h ## Storage @@ -265,7 +265,6 @@ the next client restart. [`ChromeSyncClient::CreateDataTypeControllers`][CreateDataTypeControllers]. * Add your KeyedService dependency to [`ProfileSyncServiceFactory`][ProfileSyncServiceFactory]. -* Add to the [start order list][kStartOrder]. * Add an field for encrypted data to [`NigoriSpecifics`][NigoriSpecifics]. * If your type should have its own toggle in sync settings, add an entry to the [`UserSelectableType`][UserSelectableType] enum, add a @@ -285,7 +284,6 @@ the next client restart. [CreateCommonDataTypeControllers]: https://cs.chromium.org/search/?q="ProfileSyncComponentsFactoryImpl::CreateCommonDataTypeControllers" [CreateDataTypeControllers]: https://cs.chromium.org/search/?q="ChromeSyncClient::CreateDataTypeControllers" [ProfileSyncServiceFactory]: https://cs.chromium.org/search/?q=:ProfileSyncServiceFactory%5C(%5C) -[kStartOrder]: https://cs.chromium.org/search/?q="kStartOrder[]" [NigoriSpecifics]: https://cs.chromium.org/chromium/src/components/sync/protocol/nigori_specifics.proto [UserSelectableType]: https://cs.chromium.org/chromium/src/components/sync/base/user_selectable_type.h?type=cs&q="enum+class+UserSelectableType" [pref_names]: https://cs.chromium.org/chromium/src/components/sync/base/pref_names.h diff --git a/chromium/docs/sync/uss/client_tag_based_model_type_processor.md b/chromium/docs/sync/uss/client_tag_based_model_type_processor.md index 3d17936c036..a66a1075f2e 100644 --- a/chromium/docs/sync/uss/client_tag_based_model_type_processor.md +++ b/chromium/docs/sync/uss/client_tag_based_model_type_processor.md @@ -14,10 +14,10 @@ remote changes provided via the [`ModelTypeWorker`][MTW], or local changes reported by the [`ModelTypeSyncBridge`][MTSB]) must specify a client tag, which is considered (after being hashed) the main global identifier of a sync entity. -[SMTP]: https://cs.chromium.org/chromium/src/components/sync/model_impl/client_tag_based_model_type_processor.h +[SMTP]: https://cs.chromium.org/chromium/src/components/sync/model/client_tag_based_model_type_processor.h [MTSB]: https://cs.chromium.org/chromium/src/components/sync/model/model_type_sync_bridge.h [MTCP]: https://cs.chromium.org/chromium/src/components/sync/model/model_type_change_processor.h -[MTW]: https://cs.chromium.org/chromium/src/components/sync/engine_impl/model_type_worker.h +[MTW]: https://cs.chromium.org/chromium/src/components/sync/engine/model_type_worker.h [CQ]: https://cs.chromium.org/chromium/src/components/sync/engine/commit_queue.h [MTP]: https://cs.chromium.org/chromium/src/components/sync/engine/model_type_processor.h @@ -116,5 +116,5 @@ The tracker holds the metadata in memory forever, which is needed so we know what to update the on-disk memory with when we get a new local or remote change. Changing this would require being able to handle updates asynchronously. -[PET]: https://cs.chromium.org/chromium/src/components/sync/model_impl/processor_entity.h +[PET]: https://cs.chromium.org/chromium/src/components/sync/model/processor_entity.h [EM]: https://cs.chromium.org/chromium/src/components/sync/protocol/entity_metadata.proto diff --git a/chromium/docs/tab_helpers.md b/chromium/docs/tab_helpers.md index 9f0ca4fa841..945c012714f 100644 --- a/chromium/docs/tab_helpers.md +++ b/chromium/docs/tab_helpers.md @@ -11,20 +11,20 @@ how `WebContents`es are used to build tabs in browser windows. What is a "tab helper"? It is a `WebContentsObserver` owned by the `WebContents` itself. Let's break that down. -## `WebContentsObserver` +## WebContentsObserver `WebContentsObserver` is a -[simple interface](https://source.chromium.org/chromium/chromium/src/+/HEAD:content/public/browser/web_contents_observer.) +[simple interface](https://source.chromium.org/chromium/chromium/src/+/HEAD:content/public/browser/web_contents_observer.h) that allows an object to observe events in the life of a `WebContents`. As an example, if we look at the `TabStripModel`, there are times when it need to watch out for WebContents being deleted. So it creates a -[TabStripModel::WebContentsData](https://source.chromium.org/chromium/chromium/src/+/HEA:chrome/browser/ui/tabs/tab_strip_model.cc). +[TabStripModel::WebContentsData](https://source.chromium.org/chromium/chromium/src/+/HEAD:chrome/browser/ui/tabs/tab_strip_model.cc). That object overrides `WebContentsDestroyed()`, and when a `WebContents` gets destroyed, the callback is called and the object processes the message. Note that `TabStripModel::WebContentsData` object is not owned by the `WebContents`. It is owned indirectly by the `TabStripModel`. -## `SupportsUserData` and `WebContentsUserData` +## SupportsUserData and WebContentsUserData There is a mechanism used in Chromium called [`SupportsUserData`](https://source.chromium.org/chromium/chromium/src/+/HEAD:base/supports_user_data.h) @@ -87,7 +87,7 @@ you go. Create a tab helper, and add it (in alphabetical order, please!) to `AttachTabHelpers()` yourself. `AttachTabHelpers()` is only for `WebContents` that are in browser tabs, and all of those code paths are already written. -## Reusing tab helpers with non-browser tab `WebContents`es +## Reusing tab helpers with non-browser tab WebContentses Sometimes it's useful to re-use tab helpers for `WebContents`es that aren't browser tabs. For example, the Chrome Apps code wants to be able to print, and @@ -101,7 +101,7 @@ decision about which tab helpers you need. Chances are, you don't need them all; you probably only need a handful. In fact, most tab helpers assume they are attached to browser tabs, so only add the bare minimum. -## Not every `WebContents` has every tab helper +## Not every WebContents has every tab helper The other consequence of this design is that you can't make the assumption that an arbitrary `WebContents` will have an arbitrary tab helper. The diff --git a/chromium/docs/testing/batching_instrumentation_tests.md b/chromium/docs/testing/batching_instrumentation_tests.md index ec96a3934a4..1c9ea9e3420 100644 --- a/chromium/docs/testing/batching_instrumentation_tests.md +++ b/chromium/docs/testing/batching_instrumentation_tests.md @@ -61,6 +61,10 @@ will run the suite in its own batch. This will reduce the complexity of managing and leaking state from these tests as you only have to think about tests within the suite. For smaller and less complex test suites, see Custom below. +If you use different @Features annotations on test methods, you can use the +@Batch.SplitByFeature annotation to run tests with different features in +separate batches. + ### Custom This batching type is best for smaller and less complex test suites, that diff --git a/chromium/docs/testing/chromeos_debugging_tips.md b/chromium/docs/testing/chromeos_debugging_tips.md index afe581c4554..9cc3892acc8 100644 --- a/chromium/docs/testing/chromeos_debugging_tips.md +++ b/chromium/docs/testing/chromeos_debugging_tips.md @@ -35,6 +35,8 @@ system log filename. For instance, the previous `ui.WindowControl` failure matches the [system_logs/chrome/chrome_20201029-195153] browser log, which contains the culprit Chrome crash and backtrace. +- **Symbolizing a browser crash dump**: See [below](#symbolizing-a-crash-dump). + ### Disabling a test There a couple ways to disable a test on Chrome's builders: @@ -50,6 +52,40 @@ disabled tests for the step's GN target. For example, to disable a test in the In both cases, please make sure a bug is filed for the test, and route it to the appropriate owners. +### Symbolizing a crash dump + +If a test fails due to a browser crash, there should be a Minidump crash report +present in the test's isolated out under the prefix `crashes/chrome...`. These +reports aren't very useful by themselves, but with a few commands you can +symbolize the report locally to get insight into what conditions caused Chrome +to crash. + +To do so, first download both the task's input isolate (this provides the +symbols and the symbolizing tools) as well as the task's output isolate (this +provides the crash reports). See the commands listed under the *Reproducing the +task locally* section on the task page. For example, to download them for +[this task](https://chrome-swarming.appspot.com/task?id=506a01dd12c8a610), `cd` +into a tmp directory and run: +``` +$CHROME_DIR/tools/luci-go/isolated download -I https://chrome-isolated.appspot.com --namespace default-gzip -isolated 64919fee8b02d826df2401544a9dc0f7dfa2172d -output-dir input +python $CHROME_DIR/tools/swarming_client/swarming.py collect -S chrome-swarming.appspot.com 506a01dd12c8a610 --task-output-dir output +``` + +Once both isolates have been fetched you must then generate the breakpad +symbols by pointing the `generate_breakpad_symbols.py` script to the input's +build dir: +``` +python input/components/crash/content/tools/generate_breakpad_symbols.py --symbols-dir symbols --build-dir input/out/Release/ --binary input/out/Release/chrome +``` + +That will generate the symbols in the `symbols/` dir. Then to symbolize a Chrome +crash report present in the task's output (such as +`chrome.20201211.041043.31022.5747.dmp`): +``` +./input/out/Release/minidump_stackwalk output/0/crashes/chrome.20201211.041043.31022.5747.dmp symbols/ +``` + + ### Running a test locally To run a Tast test the same way it's ran on Chrome's builders: @@ -87,7 +123,7 @@ To run a Tast test the same way it's ran on Chrome's builders: [tests/ui.WindowControl/messages]: https://isolateserver.appspot.com/browse?namespace=default-gzip&digest=baefbcfd24c02b3ada4617d259dc6b4220b413b9&as=messages [system_logs/chrome/chrome_20201029-195153]: https://isolateserver.appspot.com/browse?namespace=default-gzip&digest=272166c85f190c336a9885f0267cbdea912e31da&as=chrome_20201029-195153 [Tast attributes]: https://chromium.googlesource.com/chromiumos/platform/tast/+/HEAD/docs/test_attributes.md -[this list]: https://codesearch.chromium.org/chromium/src/chromeos/BUILD.gn?rcl=7b0393a9091fd02edc9ae773739124f7be5a0782&l=242 +[this list]: https://codesearch.chromium.org/chromium/src/chromeos/tast_control.gni [Simple Chrome]: https://chromium.googlesource.com/chromiumos/docs/+/master/simple_chrome_workflow.md [deploy_chrome.py]: https://chromium.googlesource.com/chromiumos/docs/+/master/simple_chrome_workflow.md#Deploying-Chrome-to-the-device [here]: https://chromium.googlesource.com/chromiumos/docs/+/master/cros_vm.md#in-simple-chrome diff --git a/chromium/docs/testing/code_coverage.md b/chromium/docs/testing/code_coverage.md index b3c9fff93c9..a97ed214575 100644 --- a/chromium/docs/testing/code_coverage.md +++ b/chromium/docs/testing/code_coverage.md @@ -124,7 +124,8 @@ by CQ bot. Or see this The [coverage script] automates the process described below and provides a one-stop service to generate code coverage reports locally in just one command. -This script is currently supported on Linux, Mac, iOS and ChromeOS platforms. +This script is currently supported on Android, Linux, Mac, iOS and ChromeOS +platforms. Here is an example usage: @@ -185,10 +186,11 @@ hundred, resulting in the generation of a few hundred gigabytes’ raw profiles. To limit the number of raw profiles, `%Nm` pattern in `LLVM_PROFILE_FILE` environment variable is used to run tests in multi-process mode, where `N` is the number of raw profiles. With `N = 4`, the total size of -the raw profiles are limited to a few gigabytes. +the raw profiles are limited to a few gigabytes. (If working on Android, the +.profraw files will be located in ./out/coverage/coverage by default.) ``` -$ export LLVM_PROFILE_FILE=”out/report/crypto_unittests.%4m.profraw” +$ export LLVM_PROFILE_FILE="out/report/crypto_unittests.%4m.profraw" $ ./out/coverage/crypto_unittests $ ls out/report/ crypto_unittests.3657994905831792357_0.profraw @@ -233,6 +235,9 @@ $ llvm-cov show -output-dir=out/report -format=html \ out/coverage/crypto_unittests ``` +If creating a report for Android, the -object arg would be the lib.unstripped +file, ie out/coverage/lib.unstripped/libcrypto_unittests__library.so + For more information on how to use llvm-cov, please refer to the [guide]. ## Contacts diff --git a/chromium/docs/testing/code_coverage_in_gerrit.md b/chromium/docs/testing/code_coverage_in_gerrit.md index 44767b64e91..97d77adab14 100644 --- a/chromium/docs/testing/code_coverage_in_gerrit.md +++ b/chromium/docs/testing/code_coverage_in_gerrit.md @@ -67,7 +67,7 @@ in Gerrit. [file a bug]: https://bugs.chromium.org/p/chromium/issues/entry?components=Infra%3ETest%3ECodeCoverage [code-coverage group]: https://groups.google.com/a/chromium.org/forum/#!forum/code-coverage [code_coverage.md]: code_coverage.md -[clang_code_coverage_wrapper]: clang_code_coverage_wrapper.md +[clang_code_coverage_wrapper]: https://chromium.googlesource.com/chromium/src/+/master/docs/clang_code_coverage_wrapper.md [chromium-coverage Gerrit plugin]: https://chromium.googlesource.com/infra/gerrit-plugins/code-coverage/ [Chromium on Chromium OS]: https://chromium.googlesource.com/chromium/src/+/master/docs/chromeos_build_instructions.md [Chromium on Linux]: https://chromium.googlesource.com/chromium/src/+/master/docs/linux/build_instructions.md diff --git a/chromium/docs/testing/gtest_flake_tips.md b/chromium/docs/testing/gtest_flake_tips.md index f9cf55bb40e..5077c81d74e 100644 --- a/chromium/docs/testing/gtest_flake_tips.md +++ b/chromium/docs/testing/gtest_flake_tips.md @@ -58,12 +58,20 @@ reproduce the flake with more information. If the test is flakily timing out, consider any asynchronous code that may cause race conditions, where the test subject may early exit and miss a callback, or return faster than the test can start waiting for it (i.e. make sure event -listeners are spawned before invoking the event). +listeners are spawned before invoking the event). Make sure event listeners are +for the proper event instead of a proxy (e.g. [Wait for the correct event in +test](https://chromium.googlesource.com/chromium/src/+/6da09f7510e94d2aebbbed13b038d71c511d6cbc)). + +Consider possible bugs in the system or test infrastructure (e.g. [races in +glibc](https://bugs.chromium.org/p/chromium/issues/detail?id=1010318)). For browsertest flakes, consider possible inter-process issues, such as the -renderer taking too long or returning something unexpected. +renderer taking too long or returning something unexpected (e.g. [flaky +RenderFrameHostImplBrowserTest](https://bugs.chromium.org/p/chromium/issues/detail?id=1120305)). ->TODO: Add more tips for common flake causes +For browsertest flakes that check EvalJs results, make sure test objects are not +destroyed before JS may read their values (e.g. [flaky +PaymentAppBrowserTest](https://chromium.googlesource.com/chromium/src/+/6089f3480c5036c73464661b3b1b6b82807b56a3)). ## Preventing similar flakes diff --git a/chromium/docs/testing/regression-test-selection.md b/chromium/docs/testing/regression-test-selection.md new file mode 100644 index 00000000000..1877a063908 --- /dev/null +++ b/chromium/docs/testing/regression-test-selection.md @@ -0,0 +1,11 @@ +# Regression test selection (RTS) + +Regression Test Selection (RTS) is a technique to intellegently select tests to +run, without spending too many resources on testing, but still detecting bad +code changes. + +[TOC] + +## Design Doc + +- [Browser RTS](bit.ly/chromium-rts) diff --git a/chromium/docs/testing/testing_in_chromium.md b/chromium/docs/testing/testing_in_chromium.md index b7e97bed0f5..33e2868710b 100644 --- a/chromium/docs/testing/testing_in_chromium.md +++ b/chromium/docs/testing/testing_in_chromium.md @@ -160,7 +160,7 @@ development pain for other and then fix it later. "[How do I disable a flaky test]" has instructions on how to disable a flaky test. [gtest]: https://github.com/google/googletest -[Simple gtests]: https://github.com/google/googletest/blob/master/googletest/docs/primer.md#simple-tests +[Simple gtests]: https://github.com/google/googletest/blob/master/docs/primer.md#simple-tests [Junit]: https://developer.android.com/training/testing/junit-rules [Instrumentation Tests]: https://chromium.googlesource.com/chromium/src/+/master/testing/android/docs/instrumentation.md [EarlGrey]: https://github.com/google/EarlGrey diff --git a/chromium/docs/testing/web_platform_tests.md b/chromium/docs/testing/web_platform_tests.md index eb166a1fc1b..0abf3ec2ae1 100644 --- a/chromium/docs/testing/web_platform_tests.md +++ b/chromium/docs/testing/web_platform_tests.md @@ -74,13 +74,13 @@ allowed in WPT for this purpose. Please reach out to ecosystem-infra@chromium.org before following the process below for adding a new test-only API: - 1. Create a full list of `*.mojom.js` files that you need, including all - dependencies. `mojo_bindings.js` loads dependencies recursively by default, + 1. Create a full list of `*.mojom.m.js` files that you need, including all + dependencies. Generated modules load dependencies recursively by default, so you can check the network panel of DevTools to see the full list of dependencies it loads. 2. Check [FILES.cfg](../../chrome/tools/build/linux/FILES.cfg) and add any - missing `*.mojom.js` files to the `mojojs.zip` archive. Globs are supported - in `filename`. Do not copy Mojom bindings into WPT. + missing `*.mojom.m.js` files to the `mojojs.zip` archive. Globs are + supported in `filename`. Do not copy Mojom bindings into WPT. 3. Meanwhile in Chromium, you can create a helper for your WPT tests to do browser-specific setup using [test-only-api.js](../../third_party/blink/web_tests/external/wpt/resources/test-only-api.js). @@ -139,8 +139,9 @@ Please see the `wpt_internal` **Note**: A significant downside of `wpt_internal` is that your tests may be broken by upstream changes to the resources scripts (e.g. `testharness.js`), as `wpt_internal` does not use the forked version of `testharness.js` used by all -other non-`external/wpt` tests. Use of [WPT-NOTIFY](#wpt_notify) is recommended -to ensure you are notified of breakages. +other non-`external/wpt` tests. Use of [new failure +notifications](#new-failure-notifications) is recommended to ensure you are +notified of breakages. ## Running tests @@ -200,19 +201,24 @@ For maintainers: - If the importer starts misbehaving, it can be disabled by landing a [CL to skip the update step](https://crrev.com/c/1961906/). -### WPT-NOTIFY +### New failure notifications Test owners can elect to have the importer automatically file bugs against a component when imported changes introduce failures. This includes new tests that fail in Chromium, as well as new failures introduced to an existing test. To -opt-in to this functionality, create an `OWNERS` file in the appropriate -`external/wpt/` subdirectory that contains the `WPT-NOTIFY` tag. For example, -`external/wpt/css/css-grid/OWNERS` looks like: +opt-in to this functionality, create an `DIR_METADATA` file in the appropriate +`external/wpt/` subdirectory that contains at least `wpt.notify` and +`monorail.component` fields. For example, `external/wpt/css/css-grid/DIR_METADATA` +looks like: ``` -# TEAM: layout-dev@chromium.org -# COMPONENT: Blink>Layout>Grid -# WPT-NOTIFY: true +monorail { + component: "Blink>Layout>Grid" +} +team_email: "layout-dev@chromium.org" +wpt { + notify: YES +} ``` When a test under `external/wpt/css/css-grid/` newly fails in a WPT import, the @@ -220,8 +226,8 @@ importer will automatically file a bug against the Blink>Layout>Grid component in [crbug.com][https://crbug.com], with details of which test failed and the output. -Note that we are considering making WPT-NOTIFY opt-out instead of opt-in: see -https://crbug.com/845232 +Note that we are considering making the notifications opt-out instead of +opt-in: see https://crbug.com/845232 ### Skipped tests (and how to re-enable them) diff --git a/chromium/docs/testing/web_platform_tests_wptrunner.md b/chromium/docs/testing/web_platform_tests_wptrunner.md index 54120ac8a70..2150f759f78 100644 --- a/chromium/docs/testing/web_platform_tests_wptrunner.md +++ b/chromium/docs/testing/web_platform_tests_wptrunner.md @@ -69,14 +69,14 @@ have been brought up: * [linux-wpt-identity-fyi-rel](https://ci.chromium.org/p/chromium/builders/ci/linux-wpt-identity-fyi-rel), which runs tests under `external/wpt/webauthn/`. -* [linux-wpt-payments-fyi-rel](https://ci.chromium.org/p/chromium/builders/ci/linux-wpt-payments-fyi-rel), - which runs tests under `external/wpt/payment-{handler, method-basic-card, - request}`. +* [linux-wpt-input-fyi-rel](https://ci.chromium.org/p/chromium/builders/ci/linux-wpt-input-fyi-rel), + which runs tests under `external/wpt/{input-events, pointerevents, uievents}`, + as well as `external/wpt/infrastructure/testdriver/actions/` These bots run on the waterfall, but can also be run on CLs by clicking the `Choose Tryjobs` button in Gerrit followed by searching for the bot name in the modal dialog that appears. One can also include the tag `Cq-Include-Trybots: -luci.chromium.try:linux-wpt-identity-fyi-rel` (or payments) in the description +luci.chromium.try:linux-wpt-identity-fyi-rel` (or input) in the description for the CL, which will make the bot mandatory for that CL. Results for the bots use the existing layout test results viewer diff --git a/chromium/docs/testing/web_test_expectations.md b/chromium/docs/testing/web_test_expectations.md index ea0fb232e72..bc37e1faa30 100644 --- a/chromium/docs/testing/web_test_expectations.md +++ b/chromium/docs/testing/web_test_expectations.md @@ -102,11 +102,17 @@ results from try jobs, by using the command-tool 1. First, upload a CL. 2. Trigger try jobs by running `blink_tool.py rebaseline-cl`. This should trigger jobs on - [tryserver.blink](https://build.chromium.org/p/tryserver.blink/builders). + [tryserver.blink](https://ci.chromium.org/p/chromium/g/tryserver.blink/builders). + In addition, this will also trigger the CQ try builders that run blink web tests. + linux-rel, mac-rel and win10_chromium_x64_rel_ng. + Optionally one can choose to trigger only blink try bots alone. + Run the tool with the option - + `blink_tool.py rebaseline-cl --use-blink-try-bots-only` 3. Wait for all try jobs to finish. 4. Run `blink_tool.py rebaseline-cl` again to fetch new baselines. By default, this will download new baselines for any failing tests - in the try jobs. + in the blink try jobs and CQ try bots. + Again, there is an option to use only blink try jobs results for rebaselining. (Run `blink_tool.py rebaseline-cl --help` for more specific options.) 5. Commit the new baselines and upload a new patch. @@ -160,7 +166,7 @@ details. longer than the usual timeout to run. Slow tests are given 5x the usual timeout. * [SmokeTests](../../third_party/blink/web_tests/SmokeTests): A small subset - of tests that we run on the Android bot. + of tests that we run on the Fuchsia bots. * [StaleTestExpectations](../../third_party/blink/web_tests/StaleTestExpectations): Platform-specific lines that have been in TestExpectations for many months. They're moved here to get them out of the way of people doing rebaselines diff --git a/chromium/docs/testing/web_tests.md b/chromium/docs/testing/web_tests.md index 33963609749..168155a44fb 100644 --- a/chromium/docs/testing/web_tests.md +++ b/chromium/docs/testing/web_tests.md @@ -23,6 +23,15 @@ Chrome-specific tests only. ## Running Web Tests +### Supported Platforms + +* Linux +* MacOS +* Windows +* Fuchsia + +Android is [not supported](https://crbug.com/567947). + ### Initial Setup Before you can run the web tests, you need to build the `blink_tests` target @@ -32,16 +41,6 @@ to get `content_shell` and all of the other needed binaries. autoninja -C out/Default blink_tests ``` -On **Android** (web test support -[currently limited to KitKat and earlier](https://crbug.com/567947)) you need to -build and install `content_shell_apk` instead. See also: -[Android Build Instructions](../android_build_instructions.md). - -```bash -autoninja -C out/Default content_shell_apk -adb install -r out/Default/apks/ContentShell.apk -``` - On **Mac**, you probably want to strip the content_shell binary before starting the tests. If you don't, you'll have 5-10 running concurrently, all stuck being examined by the OS crash reporter. This may cause other failures like timeouts @@ -53,8 +52,6 @@ strip ./xcodebuild/{Debug,Release}/content_shell.app/Contents/MacOS/content_shel ### Running the Tests -TODO: mention `testing/xvfb.py` - The test runner script is in `third_party/blink/tools/run_web_tests.py`. To specify which build directory to use (e.g. out/Default, out/Release, @@ -65,14 +62,10 @@ use the build in `out/Default`, use: third_party/blink/tools/run_web_tests.py -t Default ``` -For Android (if your build directory is `out/android`): - -```bash -third_party/blink/tools/run_web_tests.py -t android --android -``` - *** promo -Windows users need to use `third_party/blink/tools/run_web_tests.bat` instead. +* Windows users need to use `third_party/blink/tools/run_web_tests.bat` instead. +* Linux users should not use `testing/xvfb.py`; `run_web_tests.py` manages Xvfb + itself. *** Tests marked as `[ Skip ]` in @@ -86,12 +79,11 @@ learn more about TestExpectations and related files. *** promo Currently only the tests listed in -[SmokeTests](../../third_party/blink/web_tests/SmokeTests) -are run on the Android bots, since running all web tests takes too long on -Android (and may still have some infrastructure issues). Most developers focus -their Blink testing on Linux. We rely on the fact that the Linux and Android -behavior is nearly identical for scenarios outside those covered by the smoke -tests. +[SmokeTests](../../third_party/blink/web_tests/SmokeTests) are run on the +Fuchsia bots, since running all web tests takes too long on Fuchshia. Most +developers focus their Blink testing on Linux. We rely on the fact that the +Linux and Fuchsia behavior is nearly identical for scenarios outside those +covered by the smoke tests. *** To run only some of the tests, specify their directories or filenames as diff --git a/chromium/docs/testing/web_tests_in_content_shell.md b/chromium/docs/testing/web_tests_in_content_shell.md index 3de16d97a8c..d806f410420 100644 --- a/chromium/docs/testing/web_tests_in_content_shell.md +++ b/chromium/docs/testing/web_tests_in_content_shell.md @@ -1,5 +1,7 @@ # Running web tests using the content shell +[TOC] + ## Compiling If you want to run web tests, @@ -64,7 +66,7 @@ browser](#As-a-simple-browser). In rare cases, to run Content Shell in the exact same way as `run_web_tests.py` runs it, you need to run it in the -[protocol mode](../../content/shell/browser/web_test/test_info_extractor.h). +[protocol mode](../../content/web_test/browser/test_info_extractor.h). *** note On the Mac, use `Content Shell.app`, not `content_shell`. diff --git a/chromium/docs/threading_and_tasks.md b/chromium/docs/threading_and_tasks.md index d13dcd74750..cf6335cb8df 100644 --- a/chromium/docs/threading_and_tasks.md +++ b/chromium/docs/threading_and_tasks.md @@ -27,8 +27,9 @@ This documentation assumes familiarity with computer science ## Core Concepts * **Task**: A unit of work to be processed. Effectively a function pointer with - optionally associated state. In Chrome this is `base::Callback` created via - `base::Bind` + optionally associated state. In Chrome this is `base::OnceCallback` and + `base::RepeatingCallback` created via `base::BindOnce` and + `base::BindRepeating`, respectively. ([documentation](https://chromium.googlesource.com/chromium/src/+/HEAD/docs/callback.md)). * **Task queue**: A queue of tasks to be processed. * **Physical thread**: An operating system provided thread (e.g. pthread on @@ -323,9 +324,9 @@ best practices and pitfalls to avoid. In order to write non-blocking code, many APIs in Chrome are asynchronous. Usually this means that they either need to be executed on a particular thread/sequence and will return results via a custom delegate interface, or they -take a `base::Callback<>` object that is called when the requested operation is -completed. Executing work on a specific thread/sequence is covered in the -PostTask sections above. +take a `base::OnceCallback<>` (or `base::RepeatingCallback<>`) object that is +called when the requested operation is completed. Executing work on a specific +thread/sequence is covered in the PostTask sections above. ## Posting Multiple Tasks to the Same Thread diff --git a/chromium/docs/tpm_quick_ref.md b/chromium/docs/tpm_quick_ref.md index eae952bb573..7ccd9fb8190 100644 --- a/chromium/docs/tpm_quick_ref.md +++ b/chromium/docs/tpm_quick_ref.md @@ -17,7 +17,7 @@ be up to date at any given point, but it's a wiki so you know what to do. (no active use elsewhere): * [NVRAM use for OS rollback attack protection](http://git.chromium.org/gitweb/?p=chromiumos/platform/vboot_reference.git;a=blob;f=firmware/lib/rollback_index.c) * [Tamper evident storage](http://git.chromium.org/gitweb/?p=chromiumos/platform/cryptohome.git;a=blob;f=README.lockbox) -* [Tamper-evident storage for avoiding runtime device management mode changes](http://git.chromium.org/gitweb/?p=chromium/chromium.git;a=blob;f=chrome/browser/chromeos/login/enrollment/enterprise_enrollment_screen.cc) +* [Tamper-evident storage for avoiding runtime device management mode changes](http://git.chromium.org/gitweb/?p=chromium/chromium.git;a=blob;f=chrome/browser/ash/login/enrollment/enterprise_enrollment_screen.cc) * [User key/passphrase and cached data protection](http://git.chromium.org/gitweb/?p=chromiumos/platform/cryptohome.git;a=blob;f=README.homedirs) * A TPM in a Chrome device has an EK certificate that is signed by an intermediate certificate authority that is dedicated to the specific TPMs diff --git a/chromium/docs/trusted_types_on_webui.md b/chromium/docs/trusted_types_on_webui.md new file mode 100644 index 00000000000..df48190afe5 --- /dev/null +++ b/chromium/docs/trusted_types_on_webui.md @@ -0,0 +1,220 @@ +# Trusted Types on WebUI + +[TOC] + +## What is Trusted Types? + +[Trusted Types](https://web.dev/trusted-types/) is a defense in depth +mitigation for DOM-based Cross Site Scripting attacks. Trusted Types +introduces a runtime type system for dangerous sinks (e.g. `elem.innerHTML`, +`eval`, `scriptElem.src`, etc), and only allows Trusted Types (i.e. +`TrustedHTML`, `TrustedScript`, or `TrustedScriptURL`) as an assignment to those +sinks. + +While [CSP](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) in general +tries to mitigate the exploitability of an injection by only allowing certain +script to be executed (via allow-list of host, nonce, hash, etc), Trusted Types +provides a way to enforce validation for all injections. This is ideal because +without Trusted Types, any other resources such as CSS, image, video, audio, etc +can be injected by default in WebUI pages, which could cause other types of bugs +in the WebUI renderer such as memory corruption bugs. + +## How can I "Trusted Type" my code? + +**Note: If your JS code will also run on Chromium for iOS (i.e. WebKit), you +should have an if statement to check the Trusted Types support before using +methods/properties under `window.trustedTypes` :** + +``` +if (window.trustedTypes) { + // Trusted Types is supported, let's use Trusted Types 😎 + elem.innerHTML = trustedTypes.emptyHTML; +} else { + // Trusted Types is NOT supported 😔 + elem.innerHTML = ''; +} +``` + +### Change empty string assignment to dangerous sinks + +Example code: + +``` +document.body.innerHTML = ''; +``` + +This will be a Trusted Types violation because the value we are assigning to +a dangerous sink is not a Trusted Type. +This can be converted to: + +``` +document.body.innerHTML = trustedTypes.emptyHTML; +``` + +There is also `trustedTypes.emptyScript` to clear script contents. + +### Change text assignment to dangerous sinks + +Example code: + +``` +document.body.innerHTML = 'Hello Guest!'; +``` + +Because this is just a text assignment, this can be converted to: + +``` +document.body.textContent = 'Hello Guest!'; +``` + +### Change HTML assignment to dangerous sinks + +#### Use _template_ element + +Example code: + +``` +document.body.innerHTML = '<div><p>' + loadTimeData.getString('foo') + '</p> +</div>'; +``` + +This can be converted by adding _template_ element to HTML file: + +``` +<template id="foo-template"> + <div> + <p></p> + </div> +</template> +``` + +And then adding following JS code to JS file: + +``` +// body might already have some contents, so let's clear those first 😊 +document.body.innerHTML = trustedTypes.emptyHTML; + +const temp = document.querySelector('#foo-template').cloneNode(true).content; +temp.querySelector('p').textContent = loadTimeData.getString('foo'); +document.body.appendChild(temp); +``` + +#### Use DOM APIs + +In cases where you don't have control over the HTML file (e.g. converting common +JS libraries), you can use DOM APIs. +Example code: + +``` +document.body.innerHTML += '<p>' + loadTimeData.getString('foo') + '</p>'; +``` + +And then adding following JS code to JS file: + +``` +const p = document.createElement('p'); +p.textContent = loadTimeData.getString('foo'); +document.body.appendChild(p); +``` + +#### Use `trustedTypes.createPolicy` + +If you don't have control of the HTML file and use of DOM APIs isn't ideal for +readability, you can [use `trustedTypes.createPolicy`] +(https://web.dev/trusted-types/#create-a-trusted-type-policy). +Example code: + +``` +document.body.innerHTML = '<div class="tree-row">' + + '<span class="expand-icon"></span>' + + '<span class="tree-label-icon"></span>' + + '<span class="tree-label"></span>' + + '</div>' + + '<div class="tree-children" role="group"></div>'; +``` + +This can be converted to: + +``` +const htmlString = '<div class="tree-row">' + + '<span class="expand-icon"></span>' + + '<span class="tree-label-icon"></span>' + + '<span class="tree-label"></span>' + + '</div>' + + '<div class="tree-children" role="group"></div>'; + +const staticHtmlPolicy = trustedTypes.createPolicy( + 'foo-static', {createHTML: () => htmlString}); + +// Unfortunately, a string argument to createHTML is required. +// https://github.com/w3c/webappsec-trusted-types/issues/278 +document.body.innerHTML = staticHtmlPolicy.createHTML(''); +``` + +This case also requires changes in C++, as we need to allow the `foo-static` +Trusted Type policy (created above in `trustedTypes.createPolicy`) in the CSP +header. + +``` +source->OverrideContentSecurityPolicy( + network::mojom::CSPDirectiveName::TrustedTypes, + "trusted-types foo-static;"); +``` + +### Change script URL assignment to dangerous sinks + +Example code: + +``` +const script = document.createElement('script'); +script.src = 'chrome://resources/foo.js'; +document.body.appendChild(script); +``` + +This can be converted to: + +``` +const staticUrlPolicy = trustedTypes.createPolicy( + 'foo-js-static', + {createScriptURL: () => 'chrome://resources/foo.js'}); + +const script = document.createElement('script'); +// Unfortunately, a string argument to createScriptURL is required. +// https://github.com/w3c/webappsec-trusted-types/issues/278 +script.src = staticUrlPolicy.createScriptURL(''); +document.body.appendChild(script); +``` + +This case also requires changes in C++, as we need to allow the `foo-js-static` +Trusted Type policy (created above in `trustedTypes.createPolicy`) in the CSP +header. + +``` +source->OverrideContentSecurityPolicy( + network::mojom::CSPDirectiveName::TrustedTypes, + "trusted-types foo-js-static;"); +``` + +## How to disable Trusted Types? + +In case there is no way to support Trusted Types in a WebUI page, you can +disable Trusted Types with following code: + +``` +source->DisableTrustedTypesCSP(); +``` + +## How to add a test for Trusted Types on a WebUI page? + +You can add your WebUI page to [this list] +(https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/webui/chrome_url_data_manager_browsertest.cc;l=194;drc=de8ade0753244ff6d1ef20cb2a38fe292fe9ba0a) and it will check for +Trusted Types violations on your WebUI page. + +## Sample CLs + +1. [Remove innerHTML usage in chrome://interstitials] +(https://chromium-review.googlesource.com/c/chromium/src/+/2245937) +2. [Trusted Type various WebUI] +(https://chromium-review.googlesource.com/c/chromium/src/+/2236992) +3. [Trusted Type WebRTC internals] +(https://chromium-review.googlesource.com/c/chromium/src/+/2208950)
\ No newline at end of file diff --git a/chromium/docs/ui/android/browser_controls.md b/chromium/docs/ui/android/browser_controls.md index 77b0bd3a55b..becbdd7dcc2 100644 --- a/chromium/docs/ui/android/browser_controls.md +++ b/chromium/docs/ui/android/browser_controls.md @@ -16,15 +16,15 @@ The browser controls need to be in sync with the web contents when scrolling. An There are several classes that are used to draw composited textures: - [ViewResourceFrameLayout](https://source.chromium.org/chromium/chromium/src/+/master:components/browser_ui/widget/android/java/src/org/chromium/components/browser_ui/widget/ViewResourceFrameLayout.java): A view group that can easily be transformed into a texture used by the compositor system. -- [SceneLayer (native)](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/android/compositor/scene_layer/scene_layer.h): A wrapper that provides a [cc::Layer](https://source.chromium.org/chromium/chromium/src/+/master:cc/layers/layer.h) and dictates how that layout is supposed to interact with the layout system. -- [SceneLayer](https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/src/org/chromium/chrome/browser/compositor/scene_layer/SceneLayer.java): The Java representation of SceneLayer. -- [SceneOverlay](https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/SceneOverlay.java): An interface that allows for other texture-like things to be drawn on a layout without being a layout itself. -- [SceneOverlayLayer](https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/src/org/chromium/chrome/browser/compositor/scene_layer/SceneOverlayLayer.java): Extends SceneLayer for SceneOverlay. -- [LayoutManager](https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManager.java): The class that manages the Layouts. In the browser controls context, this is the manager that adds the SceneOverlay to the Layout to be drawn. +- [SceneLayer (native)](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/android/layouts/scene_layer.h): A wrapper that provides a [cc::Layer](https://source.chromium.org/chromium/chromium/src/+/master:cc/layers/layer.h) and dictates how that layout is supposed to interact with the layout system. +- [SceneLayer](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/android/layouts/java/src/org/chromium/chrome/browser/layouts/scene_layer/SceneLayer.java): The Java representation of SceneLayer. +- [SceneOverlay](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/android/layouts/java/src/org/chromium/chrome/browser/layouts/SceneOverlay.java): An interface that allows for other texture-like things to be drawn on a layout without being a layout itself. +- [SceneOverlayLayer](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/android/layouts/java/src/org/chromium/chrome/browser/layouts/scene_layer/SceneOverlayLayer.java): Extends SceneLayer for SceneOverlay. +- [LayoutManager](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/android/layouts/java/src/org/chromium/chrome/browser/layouts/LayoutManager.java): The class that manages the Layouts. In the browser controls context, this is the manager that adds the SceneOverlay to the Layout to be drawn. -The Android view is wrapped in a `ViewResourceFrameLayout`. If we look at [bottom_control_container.xml](https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/res/layout/bottom_control_container.xml), the xml layout for the bottom controls, the views are wrapped in [ScrollingBottomViewResourceFrameLayout](https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/ScrollingBottomViewResourceFrameLayout.java). +The Android view is wrapped in a `ViewResourceFrameLayout`. If we look at [bottom_control_container.xml](https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/res/layout/bottom_control_container.xml), the xml layout for the bottom controls, the views are wrapped in [ScrollingBottomViewResourceFrameLayout](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/bottom/ScrollingBottomViewResourceFrameLayout.java). -A scene layer ([ScrollingBottomViewSceneLayer](https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/src/org/chromium/chrome/browser/compositor/scene_layer/ScrollingBottomViewSceneLayer.java) and its native counterpart [scrolling_bottom_view_scene_layer.cc](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/android/compositor/scene_layer/scrolling_bottom_view_scene_layer.cc) in this example) is responsible for creating a compositor layer using the view resource. [LayoutManager](https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManager.java) adds the scene layer to the global layout. +A scene layer ([ScrollingBottomViewSceneLayer](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/android/toolbar/java/src/org/chromium/chrome/browser/toolbar/bottom/ScrollingBottomViewSceneLayer.java) and its native counterpart [scrolling_bottom_view_scene_layer.cc](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/android/compositor/scene_layer/scrolling_bottom_view_scene_layer.cc) in this example) is responsible for creating a compositor layer using the view resource. [LayoutManager](https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManager.java) adds the scene layer to the global layout. See these example CLs [adding a scene layer](https://chromium-review.googlesource.com/c/chromium/src/+/1769631) and [adding the Android view to use as a resource](https://chromium-review.googlesource.com/c/chromium/src/+/1809813). diff --git a/chromium/docs/ui/android/bytecode_rewriting.md b/chromium/docs/ui/android/bytecode_rewriting.md new file mode 100644 index 00000000000..d5742eb15c8 --- /dev/null +++ b/chromium/docs/ui/android/bytecode_rewriting.md @@ -0,0 +1,81 @@ +# Bytecode Rewriting + +## TL;DR + +We modify the return type of AndroidX's `Fragment.getActivity()` method from `FragmentActivity` +to `Activity` to more easily add Fragments from multiple ClassLoaders into the same Fragment tree. + +## Why? + +In Java, two instances of the same class loaded from two different ClassLoaders aren't compatible +with each other at runtime. Because AndroidX libraries are bundled with each APK that use them, and +different APKs are loaded with different ClassLoaders, AndroidX classes from one APK cannot be used +with the same class from another APK. This causes problems for Fragment-based UIs in WebLayer, where +the implementation is in a different ClassLoader than the embedding app, so its Fragments cannot be +added to the embedding app's Fragment tree. + +Note that this issue doesn't apply to Framework or standard library classes. Java ClassLoaders form +a tree, and if a ClassLoader can't find a particular class, it delegates to its parent. +The leaf ClassLoader used to load an app is responsible for loading the app's class files, while one +of its parents will load system-level classes. Because AndroidX classes get loaded by the +app-specific ClassLoader, different apps will load mutually incompatible versions, but a class like +Activity, which gets loaded from a parent ClassLoader, *will* be compatible between APKs at runtime, +because it ends up gets loaded from a common ClassLoader. + +To get around this incompatibility, we can create a RemoteFragment that lives in the embedding app, +and a RemoteFragmentImpl that lives in another APK. The RemoteFragment can be added to the original +Fragment tree, and will forward all Fragment lifecycle events over an AIDL interface to +RemoteFragmentImpl. The fake Fragment in the secondary APK (RemoteFragmentImpl) can create a +FragmentController, which allows it to become the host of its own Fragment tree, and any UIs from +the secondary ClassLoaded can be added to this new Fragment tree that's been essentially grafted +onto the original. + +This mostly works, but runs into issues when Fragments call `Fragment.getActivity()`, which they do +a lot. The getActivity implementation takes the Activity given to the FragmentController constructor +(via FragmentHostCallback), and casts it to a FragmentActivity before returning it. The original +Activity will typically be a FragmentActivity from the embedding app's ClassLoader, which means that +due to the aforementioned issues, this cast will fail when run in the secondary ClassLoader's +Fragment class because even though the Activity is a FragmentActivity, it's from the wrong +ClassLoader. + +To fix this second issue, we modify the bytecode of `Fragment.getActivity()` in the AndroidX +prebuilt .aar files to return a plain Activity instead of a FragmentActivity. This allows us to +continue calling getActivity() as normal. Note that this does mean FragmentActivity-specific methods +can no longer be used in Fragments, but there were no uses of them in Chromium that couldn't be +trivially removed as of late 2020. + +## How does it work? + +The bytecode rewriting happens at build time by +[FragmentActivityReplacer](https://source.chromium.org/chromium/chromium/src/+/master:build/android/bytecode/java/org/chromium/bytecode/FragmentActivityReplacer.java), +which is specified as a bytecode rewriter via the `bytecode_rewriter_target` rule. Compilation errors +related to this should get detected by +[compile_java.py](https://source.chromium.org/chromium/chromium/src/+/master:build/android/gyp/compile_java.py), +and print a message pointing users here, which is likely why you're reading this :) + +If you need to apply FragmentActivityReplacer to a given target then add … + +``` +bytecode_rewriter_target = "//build/android/bytecode:fragment_activity_replacer" +``` + +… to the build configuration for that target. + +## How does this affect my code? + +The goal is for these changes to be as transparent as possible; most code shouldn't run into issues. +However, if there's no way around calling a FragmentActivity method in your code, **and your +Fragment is in //chrome**, you could cast the Activity to a FragmentActivity as AndroidX used to do. +If your Fragment is in //components, FragmentActivity methods will likely not work directly, and may +need to be forwarded to an implementation in the original ClassLoader somehow. + +The more important thing to note is that in a multi-ClassLoader world, `getActivity()` and +`getContext()` will typically return two different objects, so we need to be more careful about which +method we call, particularly for code in //components. `getActivity()` will return the Activity from +the original ClassLoader, and should be used to for calls like `.finish*()`, `.setTitle(), and +`.startActivity()` (which live in Activity anyway). When loading resources, you should default to +calling `getContext()`, as resources usually come from the same ClassLoader as the Fragment, and the +Context should be configured to load them correctly. + +As a rule of thumb, prefer `getContext()` to `getActivity()`, unless you need to operate on the +Activity itself, or you know the resource or setting you need belongs to the original Activity. diff --git a/chromium/docs/ui/index.md b/chromium/docs/ui/index.md index 749ab494a0e..e49d6a6b9c3 100644 --- a/chromium/docs/ui/index.md +++ b/chromium/docs/ui/index.md @@ -20,6 +20,7 @@ Details on Chrome UI. * [Platform Styling](/docs/ui/views/platform_style.md) * [Product Excellence](/docs/ui/product_excellence/index.md) * [UI Devtools](/docs/ui/ui_devtools/index.md) +* [Input Event Routing](/docs/ui/input_event/index.md) Archival Documentation on Chrome UI. * [Aura](/docs/ui/aura/index.md) diff --git a/chromium/docs/ui/input_event/images/aura-event-flow.png b/chromium/docs/ui/input_event/images/aura-event-flow.png Binary files differnew file mode 100644 index 00000000000..c2fb17df624 --- /dev/null +++ b/chromium/docs/ui/input_event/images/aura-event-flow.png diff --git a/chromium/docs/ui/input_event/images/event-processor.png b/chromium/docs/ui/input_event/images/event-processor.png Binary files differnew file mode 100644 index 00000000000..f824365d13f --- /dev/null +++ b/chromium/docs/ui/input_event/images/event-processor.png diff --git a/chromium/docs/ui/input_event/images/high-level-input-pipeline.png b/chromium/docs/ui/input_event/images/high-level-input-pipeline.png Binary files differnew file mode 100644 index 00000000000..2b5350875ce --- /dev/null +++ b/chromium/docs/ui/input_event/images/high-level-input-pipeline.png diff --git a/chromium/docs/ui/input_event/images/mac-event-flow.png b/chromium/docs/ui/input_event/images/mac-event-flow.png Binary files differnew file mode 100644 index 00000000000..30ef1cd1882 --- /dev/null +++ b/chromium/docs/ui/input_event/images/mac-event-flow.png diff --git a/chromium/docs/ui/input_event/images/views-event-flow.png b/chromium/docs/ui/input_event/images/views-event-flow.png Binary files differnew file mode 100644 index 00000000000..324928cb74b --- /dev/null +++ b/chromium/docs/ui/input_event/images/views-event-flow.png diff --git a/chromium/docs/ui/input_event/images/views-tree.png b/chromium/docs/ui/input_event/images/views-tree.png Binary files differnew file mode 100644 index 00000000000..498f7d54c7c --- /dev/null +++ b/chromium/docs/ui/input_event/images/views-tree.png diff --git a/chromium/docs/ui/input_event/index.md b/chromium/docs/ui/input_event/index.md new file mode 100644 index 00000000000..88554ee4424 --- /dev/null +++ b/chromium/docs/ui/input_event/index.md @@ -0,0 +1,445 @@ +# The Life of an Input Event in Desktop Chrome UI + +[TOC] + +## Background + +The goal of this document is to improve the understanding of the input event +system in the desktop Chrome UI. + +## Overview + + + +The Chrome UI system handles input events (typically key downs or mouse clicks) +in three stages: + +- At the very beginning, the OS generates a native input event and sends it to a + Chrome browser window. +- Then, the Chrome Windowing system receives the event and converts it into a + platform-independent ui::Event. It then sends the event to the Views system. \ + The interaction with IME ([Input Method](https://en.wikipedia.org/wiki/Input_method), + for non-English text input) is also handled at this stage. +- Lastly, the Views system sends the event to the control that expects to + receive it. For example, a character key event will insert one letter to a + focused textfield. + +## Window Abstraction + +### Aura + +Aura is the window abstraction layer used on Windows, Linux, and ChromeOS. An +event goes through several phases in Aura and is eventually passed into views. + + + +**Phase 0** - DesktopWindowTreeHost + +After the user presses a key or clicks the mouse, the OS generates a low-level +input event and pumps it into a message loop. After some low-level os-specific +plumbing, the event is then delivered to a DesktopWindowTreeHost that hosts a +native window and handles events in DesktopWindowTreeHost::DispatchEvent(). + +**Phase 1** - EventProcessor pre-dispatch + + + +Next, the event is passed to a WindowEventDispatcher which is an EventProcessor +owned by the DesktopWindowTreeHost. On ChromeOS, some ui::EventRewriters may +rewrite the event before passing. + +An EventProcessor delivers the event to the right target. It provides a root +EventTarget and a default EventTargeter. EventTargeter is responsible for +finding the event target. An EventTarget can also provide an EventTargeter and +the EventProcessor prefers its root EventTarget’s targeter over the default +targeter. + +The EventProcessor delivers the event to the first target found by the targeter. +If the event is not marked as handled, it will ask the targeter to find the next +target and repeat the procedure until the event is handled. + +The EventProcessor can also have pre- and post-dispatch phases that happen +before and after the event is dispatched to the target. + +In the case of WindowEventDispatcher, it has a pre-dispatch phase for different +types of events. + +- For mouse move events, it may synthesize and dispatch a ET_MOUSE_EXITED event + to notify that mouse exits from previous UI control. +- For key events, it + [forwards](https://source.chromium.org/chromium/chromium/src/+/master:ui/aura/window_event_dispatcher.cc;drc=d9fa208c0b5d3d454df1ff1cbc724b5fd708cf7a;l=1053) + the key to ui::InputMethod::DispatchKeyEvent() and the event will be handled + there. Depending on IME involvement, later phases of WindowEventDispatcher may + be **SKIPPED**. Details are explained later. + +If the event is not marked handled in the EventProcessor pre-dispatch phase, it +will be passed to the target. For key events, the target is the aura::Window +that owns the focus. For mouse events, the target is the aura::Window under the +cursor. + +Each browser window comes with an aura::Window. It is worth noting that the web +content and dialog bubble live in their own aura::Windows. These different +aura::Windows treat accelerators differently and the detail will be explained +later. + +**Phase 2** - EventTarget pre-target + +Like EventProcessor, an EventTarget consumes the event in three phases. it owns +one target handler and optionally multiple pre-targets and post-target handlers. +An event will first be passed to pre-target handlers, and if not consumed by +them, then to the default target handler, and lastly to post-target handlers. + +Non-content aura::Window uses pre-handler to forward key events to FocusManager +in views. If the key is an accelerator, the event will be intercepted and later +phases will be SKIPPED. + +Mouse events at present are not processed in pre-handlers. Content aura::Window +does not have any pre-handlers, either. + +**Phase 3** - EventTarget regular + +At this phase, non-content aura::Window asks (Desktop)NativeWidgetAura::OnEvent() +to handle the event. + +DesktopNativeWidgetAura is the native implementation of a top-level +Widget. Non top-level widgets, e.g. dialog bubble, use NativeWidgetAura instead. +The native widget then passes the event to Widget::OnMouseEvent(), +Widget::OnClickEvent(), or other Widget methods depending on the event type. The +event is then handled in views and is explained in a later section. + +Content aura::Window instead asks RenderWidgetHostViewAura::OnEvent() to handle +the event. The event will be sent to Blink and then to BrowserCommandController +if not consumed by the webpage. Some important shortcuts, e.g. Ctrl+T, are +[preserved](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/browser_command_controller.cc;drc=02f28360f2f6588ca2428ef93c8129e0a8cc231c;l=210) +and will not be sent to the web page. + +**Phase 4** - EventTarget post-target + +This phase is not effective in window abstraction. + +**Phase 5** - EventProcessor post-dispatch + +For touch events, WindowEventDispatcher may +[recognize](https://source.chromium.org/chromium/chromium/src/+/master:ui/aura/window_event_dispatcher.cc;drc=d9fa208c0b5d3d454df1ff1cbc724b5fd708cf7a;l=574) +the event as a gesture event and dispatch it. + +**Key event handling and IME interoperability** + +We mentioned in phase 1 pre-dispatch that a key event may be consumed in _this_ +phase and no later phases. This is because we need to interact with IME through +[InputMethod::DispatchKeyEvent()](https://source.chromium.org/chromium/chromium/src/+/master:ui/base/ime/input_method.h;drc=d9fa208c0b5d3d454df1ff1cbc724b5fd708cf7a;l=101) +in pre-dispatch. + +If the IME accepts this key event, Chrome will stop any further event handling +because IMEs have their own interpretation to the event. Instead, Chrome +[exits](https://source.chromium.org/chromium/chromium/src/+/master:ui/base/ime/linux/input_method_auralinux.cc;drc=d9fa208c0b5d3d454df1ff1cbc724b5fd708cf7a;l=109) +phase 1 with a fake VKEY_PROCESSKEY event indicating the event has been +processed by IME, and waits for new events emitted by IME and handles them +accordingly. For example, Chrome on Linux +[listens](https://source.chromium.org/chromium/chromium/src/+/master:ui/gtk/input_method_context_impl_gtk.cc;drc=340909edba6daccd34d5875de93599551b218902;l=74) +for the GTK `preedit-changed` event that indicates a change in the composition +text. + +If the IME does not accept this key event, WindowEventDispatcher will re-enter +phase 1 but with +[IME explicitly skipped](https://source.chromium.org/chromium/chromium/src/+/master:ui/aura/window_tree_host.cc;drc=d9fa208c0b5d3d454df1ff1cbc724b5fd708cf7a;l=277), +so that the event can be passed to phase 2 where accelerators are handled. + +### MacViews + +_MacViews_ is an umbrella term that covers the broader effort to adopt views in +Chrome Mac. Before this, Chrome Mac was using native Cocoa controls. In this +document, we use MacViews to refer to the windows abstraction part of Chrome +Mac. + +Mac does not use Aura and is significantly different from Aura in that it hosts +native NSWindow in +[RemoteCocoa](https://source.chromium.org/chromium/chromium/src/+/master:components/remote_cocoa/) +that talks to views through a mojo interface. This design allows RemoteCocoa to +either live within the browser process or in a separate process for PWA mode. +This design is largely due to the requirement of PWAs on Mac. +[[ref](https://docs.google.com/document/d/1Cym6LpmrYZU6Jl1BYKhuWqzRJV_OL8LYYepGGAJw7oE/edit)] + +Mac’s event handling borrows heavily from +[Cocoa’s Event architecture](https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/EventOverview/EventArchitecture/EventArchitecture.html) +but applies its own handling where appropriate. + +During startup ChromeBrowserMainParts will kick off NSApp’s main run loop that +will continue to service Chrome application event messages for the life of the +program. These messages are picked up by BrowserCrApplication (NSApplication +subclass) and for the most part forwarded to the appropriate +NativeWidgetMacNSWindow (NSWindow subclass). + +A key departure from how typical Cocoa applications are architected is that +Chrome uses a single root NSView (the BridgedContentView) as the contentView for +it’s NSWindow. This view is largely responsible for adapting native NSEvents and +funneling them through to the Views framework. + +The below two examples demonstrate two key event flows through the Cocoa layers +of Chrome through to the Views framework. + + + +#### Right mouse down event (clicking a button in the browser window) + +The below diagram demonstrates points of interest during dispatch of a right +mouse down event on a Chrome browser window button. + +Summary: + +- The Window Server is responsible for determining which NSWindow a mouse event + belongs to. +- Once the NSWindow has been identified the Window Server will place the mouse + down event in Chrome’s + [BrowserCrApplication ](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/chrome_browser_application_mac.h)(NSApplication) + event queue. +- BrowserCrApplication’s main run loop reads from the event queue. +- BrowserCrApplication delivers the event to the + [NativeWidgetMacNSWindow ](https://source.chromium.org/chromium/chromium/src/+/master:components/remote_cocoa/app_shim/native_widget_mac_nswindow.h)(NSWindow) + which delivers the mouseDown event to its root NSView + [contentView](https://developer.apple.com/documentation/appkit/nswindow/1419160-contentview). +- [BridgedContentView](https://source.chromium.org/chromium/chromium/src/+/master:components/remote_cocoa/app_shim/bridged_content_view.h) + aggregates all mouse related + [NSResponder messages](https://source.chromium.org/chromium/chromium/src/+/master:ui/base/cocoa/base_view.mm;l=114;drc=3930cf3f0e2404a517b4580b21e598a3fc648ff0) + (rightMouseDown, mouseMoved, leftMouseUp etc) into the mouseEvent: method. +- The + [mouseEvent method](https://source.chromium.org/chromium/chromium/src/+/master:components/remote_cocoa/app_shim/bridged_content_view.mm;l=580;drc=dd5a8d8907f60050eeaaac0299be624fe36cad8a) + performs NSEvent conversion into ui::Event and sends the event to the + [NativeWidgetMacNSWindowHost](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/cocoa/native_widget_mac_ns_window_host.h)’s + [OnMouseEvent()](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/cocoa/native_widget_mac_ns_window_host.mm;l=790;drc=d18ea8bb2c8089e74838b8f781ba5f08420b8516) + method. +- BridgedContentView communicates to the NativeWidgetMacNSWindowHost via a + bridge. + - NativeWidgetMacNSWindowHost implements a Mojo remote + [remote_cocoa::mojom::NativeWidgetNSWindowHost](https://source.chromium.org/chromium/chromium/src/+/master:components/remote_cocoa/common/native_widget_ns_window_host.mojom;l=41;drc=e6d7cbba3a95a52176a5d5ca37572296f9b9aaa8) + such that the BridgedContentView and the NativeWidgetMacNSWindowHost can + communicate via message passing (needed in the case these exist across + process boundaries). +- [NativeWidgetMac](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/widget/native_widget_mac.mm;l=118;drc=1748fd17d0e2b542b0051955a9cccb2153ff6ea7) + owns a NativeWidgetMacNSWindowHost instance. + +#### Key Down event (entering text into the browser’s omnibox) + +The following demonstrates key points of interest in the event flow that occurs +when a user presses a character key with the intention to enter text into the +browser’s omnibox. + +Summary: + +- The Window Server will deliver key events to the + [CrBrowserApplication’s ](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/chrome_browser_application_mac.h)(NSApplication) + event queue. +- Provided the keyDown event is not a key equivalent or keyboard interface + control, the BrowserCrApplication sends the event to + [NativeWidgetMacNSWindow ](https://source.chromium.org/chromium/chromium/src/+/master:components/remote_cocoa/app_shim/native_widget_mac_nswindow.h)(NSWindow) + that is associated with the first responder. +- The window dispatches the event as a keyDown event to it’s first responder (in + this case the + [BridgedContentView ](https://source.chromium.org/chromium/chromium/src/+/master:components/remote_cocoa/app_shim/bridged_content_view.h)which + serves as the NSWindow’s + [contentView](https://developer.apple.com/documentation/appkit/nswindow/1419160-contentview)). +- BridgedContentView inherits from NSTextInputClient which is required for + Chrome to interact properly with Cocoa’s text input management system. +- BridgedContentView forwards the keyEvent to + [interpretKeyEvents](https://developer.apple.com/documentation/appkit/nsresponder/1531599-interpretkeyevents): + method. + - This invokes Cocoa’s input management system. + - This checks the pressed key against all key-binding dictionaries. + - If there is a match in the keybinding dictionary it sends a + doCommandBySelector: message back to the view. (commands include insertTab, + insertNewline, insertLineBreak, moveLeft etc). + - If no command matches it sends an insertText: message back to the + BridgedContentView. +- BridgedContentView + [converts the NSString to UFT16 and sends it through to it’s TextInputHost](https://source.chromium.org/chromium/chromium/src/+/master:components/remote_cocoa/app_shim/bridged_content_view.mm;l=504;drc=dd5a8d8907f60050eeaaac0299be624fe36cad8a). + - [TextInputHost ](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/cocoa/text_input_host.h)implements + the remote Mojo interface + [remote_cocoa::mojom::TextInputHost](https://source.chromium.org/chromium/chromium/src/+/master:components/remote_cocoa/common/text_input_host.mojom;l=13;drc=4692fed3d7ff9afec997e3cd5fc5cc7050022cc7) + and BridgedContextView communicates with the TextInputHost via Mojo message + passing. +- The TextInputHost calls + [InsertText()](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/cocoa/text_input_host.mm;l=254;drc=7d9087d7348118b8ea5b12df100f4267dc56cadf) + on it’s + [ui::TextInputClient](https://source.chromium.org/chromium/chromium/src/+/master:ui/base/ime/text_input_client.h). + - This should be the TextInputClient of the currently focused view. + +## Views + +The Window Abstraction layer will pass the input event to Views. Views is +Chrome’s (mostly) platform-independent UI framework that orchestrates UI +elements in a tree structure. Every node in the views tree is a +[View](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/view.h;bpv=1;bpt=1), +which is a UI element similar to an HTML DOM element. + + + +A +[Widget](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/widget/widget.h;l=93;bpv=1;bpt=1?q=Widget&sq=&ss=chromium%2Fchromium%2Fsrc) +hosts the views tree and is a window-like surface that draws its content onto a +canvas provided by the underlying window abstraction. Every widget can have at +most one focused view which is tracked by a FocusManager owned by the widget. + +The root of Views tree is a +[RootView](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/widget/root_view.h;drc=62bf27aca5418212ceadd8daf9188d2aa437bfcc;l=52), +which is a special subclass of View that helps bridging between children views +and the wrapping Widget. + + + +### Character Key Event + +Suppose the omnibox is focused and the user presses down a key, say character +‘a’. How is this key routed through the system and delivered to the omnibox? We +will only study the stack after the Window Abstraction layer passes the event to +Views. + +Surprisingly, the stack that the event needs to go through is not deep. On Aura, +it can be summarized as a path of Widget -> RootView -> focused View. RootView +will ask FocusManger for the focused view and the event will be directly +delivered to it. There is no tree traversal. The details can be broken down +into: + +1. [Widget::OnKeyEvent()](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/widget/widget.cc;drc=5b8933e94139a0ab5be46141666fdfcce0f624f6;l=1250): + The key event will be passed from platform-dependent NativeWidget to Widget. + \ + The event is then processed by [EventRewriter](https://source.chromium.org/chromium/chromium/src/+/master:ui/events/event_source.cc;drc=17cd6070f4da0bb1c0396778afba06910f98fd7b;l=139)(s) + attached to the Widget. Rewriters are used **only** on ChromeOS. +2. [EventProcessor::OnEventFromSource()](https://source.chromium.org/chromium/chromium/src/+/master:ui/events/event_processor.cc;drc=0cd9a52effdb8f2a02b99e8044d680001ba8090f;l=16): + The key event is then passed to the root view of the widget. Note that a + RootView _is an_ EventProcessor. \ + An EventProcessor is a _multicaster_. It tries to deliver the event to multiple + targets [until the event is marked as handled](https://source.chromium.org/chromium/chromium/src/+/master:ui/events/event_processor.cc;drc=5b8933e94139a0ab5be46141666fdfcce0f624f6;l=48): + It delegates the target enumeration task to an EventTargeter. \ + In the case of RootView, the EventTargeter is a **ViewTargeter**. A + ViewTargeter will return the currently focused view as the target for the key + event. +3. [EventHandler::OnEvent()](https://source.chromium.org/chromium/chromium/src/+/master:ui/events/event_handler.cc;drc=5b8933e94139a0ab5be46141666fdfcce0f624f6;l=29): + OmniboxViewViews receives the event. Every view is an EventHandler and + OmniboxViewViews is no exception. +4. [OmniboxViewViews::HandleKeyEvent()](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/views/omnibox/omnibox_view_views.cc;drc=5b8933e94139a0ab5be46141666fdfcce0f624f6;l=2171): + Eventually where the event is consumed and the text in the omnibox gets + updated. + +On Mac, the event will be funneled directly from the Window Abstraction layer to +the focused view, i.e. Widget and RootView are not involved in event routing. +This is achieved by having NativeWidgetMacNSWindowHost save a pointer to the +focused view in a +[TextInputHost](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/cocoa/text_input_host.mm;drc=7d9087d7348118b8ea5b12df100f4267dc56cadf;l=268). +NativeWidgetMac +[registers](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/widget/native_widget_mac.mm;drc=1748fd17d0e2b542b0051955a9cccb2153ff6ea7;l=895) +itself as an observer of focus change in FocusManager and updates +NativeWidgetMacNSWindowHost’s TextInputHost on focus change. + +### Accelerator Key Event + +In the case when the keystroke is an accelerator, for example, a Ctrl+T to open +a new tab, the focused view is not the expected view to handle it. + +On Aura, when the focus is not on the web content, accelerators are handled by a +**[pre-handler](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/widget/focus_manager_event_handler.cc;drc=df872ce8fcce25af51aa6b0f9fe8b1135b687524;l=17)** +that happens before the handler for character keys. This pre-handlers is hooked +on aura::Window in the Window Abstraction layer. If a key event is consumed as +accelerators, the character key handler path will be skipped. + +In the accelerator path, the **FocusManager** +[relays](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/widget/focus_manager_event_handler.cc;drc=df872ce8fcce25af51aa6b0f9fe8b1135b687524;l=26) +the event from Window Abstraction to views::AcceleratorManager, and finally to +chrome::BrowserCommandController. + +Not all accelerators are handled by views::AcceleratorManager. A notable +exception is the Tab key which is used to switch the focused view and will be +[handled](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/focus/focus_manager.cc;drc=df872ce8fcce25af51aa6b0f9fe8b1135b687524;l=70) +directly by the FocusManager. + +If the focus is on the web content, the event will be handled by +RenderWidgetHostViewEventHandler where the web content has the priority to +receive most keys except for important navigation accelerators. + +Mac interprets accelerators early in the event pipeline. +[Key Equivalents](https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/EventOverview/HandlingKeyEvents/HandlingKeyEvents.html#//apple_ref/doc/uid/10000060i-CH7-SW11) +like Cmd+T will be +[handled](https://source.chromium.org/chromium/chromium/src/+/master:components/remote_cocoa/app_shim/native_widget_mac_nswindow.mm;drc=6bf8746951f47dda5ce87be9a6218a1abfa064d0;l=291) +early in the Window Abstraction layer and the event never goes into Views. + +### Mouse Click Event + +Views like textfield will grab focus on a click event. + +After the Widget has received the event from the window abstraction, the +RootView +[traverses](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/widget/root_view.cc;drc=5b8933e94139a0ab5be46141666fdfcce0f624f6;l=387) +the views tree from leaf to root and looks for the first view that accepts the +click event. Here, the leaf view is the lowest descendant view that contains the +cursor location of the event. The task of searching for this view is delegated +to +[ViewTargeter](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/view_targeter_delegate.cc;drc=df872ce8fcce25af51aa6b0f9fe8b1135b687524;l=33). + +When the click event is dispatched to a Textfield, it will +[notify](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/controls/textfield/textfield.cc;drc=281086aeca412de952c180411560c7ca68a5b97b;l=2172) +the FocusManager to update the focused view. + +## Focus Management + +A focused control receives keyboard events. Focus change can be triggered by +mouse click or pressing Tab key to switch to the next focusable view. Chrome +usually draws a +[FocusRing](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/controls/focus_ring.h;bpv=1;bpt=1;l=30?q=FocusRing&ss=chromium&gsn=FocusRing&gs=kythe%3A%2F%2Fchromium.googlesource.com%2Fchromium%2Fsrc%3Flang%3Dc%252B%252B%3Fpath%3Dsrc%2Fui%2Fviews%2Fcontrols%2Ffocus_ring.h%23FocusRing%253Aviews%2523c%2523oPBiT3XRtlN&gs=kythe%3A%2F%2Fchromium.googlesource.com%2Fchromium%2Fsrc%3Flang%3Dc%252B%252B%3Fpath%3Dsrc%2Fui%2Fviews%2Fcontrols%2Ffocus_ring.h%23FocusRing%253Aviews%2523c%2523ljYID0QylaH) +around the focused view. Focus rings are drawn separate from its view drawing. + +### Focusable Controls + +On all desktop platforms other than Mac, all the controls should be focusable by +default. However, on Mac, by default, only TextField and List are focusable. +Other controls such as Button, Combobox are not focusable. Full keyboard access +in system settings needs to be enabled to have navigation of all controls on the +screen. + +To comply with Mac’s behavior, Chrome sets +[kDefaultFocusBehavior](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/style/platform_style.h?q=kDefaultFocusBehavior) in +the platform style to ACCESSIBLE_ONLY on Mac as opposed to ALWAYS on other +platforms. Controls that are +[marked](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/views/toolbar/toolbar_button.cc;drc=93819857a690b79b1524052513747846c068ca90;l=104) +as ACCESSIBLE_ONLY will be skipped in search of the next view to focus if the +Full keyboard access is off. + +### Focus Handling in Window Abstraction + +Windows and Linux have more resemblance to handling focus on the window level. +They both have a ‘top-level’ window concept. A top-level window is a window that +has no parent. Chrome uses only top-level windows and the focus changing event +is only triggered between top-level windows. + +**Windows** + +Chrome observes WM_ACTIVATE messages to monitor the active window change +[[ref](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/win/hwnd_message_handler.cc?q=WM_ACTIVATE)]. + +The event is then routed up through the NativeWidget (DesktopNativeWidgetAura) +and eventually to Widget. Chrome then uses its own focus management to further +route those focus events to the view which is supposed to have the actual +keyboard focus. + +**Linux** + +Chrome on Linux observes the platform native focus change events FocusIn and +FocusOut +[[ref](https://source.chromium.org/chromium/chromium/src/+/master:ui/base/x/x11_window.cc?q=x11::Input::CrossingEvent::FocusIn)] +to respond to focus change events. Nevertheless, eventually just like on +Windows, the events will be interpreted as active window change. + +**Mac** + +On Mac, the ‘key window’ is the window that receives keyboard events. Chrome +observes the key status change by implementing +[windowDidBecomeKey](https://source.chromium.org/chromium/chromium/src/+/master:components/remote_cocoa/app_shim/views_nswindow_delegate.mm;drc=e6d7cbba3a95a52176a5d5ca37572296f9b9aaa8;l=115) +on an NSWindow and handles according +[[ref](https://source.chromium.org/chromium/chromium/src/+/master:ui/views/widget/native_widget_mac.mm?q=NativeWidgetMac::OnWindowKeyStatusChanged)], +which is similar to Windows’ active window change. + +### Focus Handling in Views + +We have almost identical focus handling in views across different platforms. In +views, a widget owns a FocusManager who manages the focus for all the widgets in +this tree. FocusManager handles proper routing of keyboard events, including the +handling of keyboard accelerators. The FocusManager also handles focus traversal +among child widgets in addition to between Views. diff --git a/chromium/docs/ui/learn/bestpractices/colors.md b/chromium/docs/ui/learn/bestpractices/colors.md index 681ac8250d5..6bcba9243a4 100644 --- a/chromium/docs/ui/learn/bestpractices/colors.md +++ b/chromium/docs/ui/learn/bestpractices/colors.md @@ -119,7 +119,7 @@ constructor since it seemed reasonable to do: **Best practice** -The [current version](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/views/native_file_system/native_file_system_usage_bubble_view.cc;l=196;drc=532834e1da3874a57dde3ed76511f53b8eb8ecdf) +The [current version](https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/views/file_system_access/file_system_access_usage_bubble_view.cc;l=196;drc=532834e1da3874a57dde3ed76511f53b8eb8ecdf) moves this to `OnThemeChanged()` and thus handles theme changes correctly: |||---||| diff --git a/chromium/docs/ui/learn/bestpractices/layout.md b/chromium/docs/ui/learn/bestpractices/layout.md index 37bc61c3752..e57ec9c006e 100644 --- a/chromium/docs/ui/learn/bestpractices/layout.md +++ b/chromium/docs/ui/learn/bestpractices/layout.md @@ -335,7 +335,7 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView( group_modifier_container->AddChildView( std::make_unique<ColorPickerView>( colors_, background_color(), initial_color, - base::Bind( + base::BindRepeating( &TabGroupEditorBubbleView::UpdateGroup, base::Unretained(this)))); ... @@ -433,7 +433,7 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView( group_modifier_container->AddChildView( std::make_unique<ColorPickerView>( this, colors_, initial_color_id, - base::Bind( + base::BindRepeating( &TabGroupEditorBubbleView::UpdateGroup, base::Unretained(this)))); color_selector_->SetProperty( diff --git a/chromium/docs/ui/learn/index.md b/chromium/docs/ui/learn/index.md index bd23aefe8e1..61069b64e14 100644 --- a/chromium/docs/ui/learn/index.md +++ b/chromium/docs/ui/learn/index.md @@ -11,6 +11,7 @@ * [Views](/docs/ui/views/overview.md) * [Product Excellence](/docs/ui/product_excellence/index.md) * [UI Devtools](/docs/ui/ui_devtools/index.md) +* [Input Event Routing](/docs/ui/input_event/index.md) # Archival Documentation on Chrome UI. diff --git a/chromium/docs/ui/views/class_splitting.md b/chromium/docs/ui/views/class_splitting.md new file mode 100644 index 00000000000..fd62da843d6 --- /dev/null +++ b/chromium/docs/ui/views/class_splitting.md @@ -0,0 +1,287 @@ +## Views Dialog Class Splitting + +This document outlines how to refactor a common old Views design pattern into a +cluster of smaller objects with less individual responsibility that are easier +to test. The result is broadly similar to the model-view-controller paradigm but +with some Views-specific differences. + +This document is specifically applicable to dialogs, bubbles, and secondary UI +surfaces that have their own Widgets. The techniques described here work well +for subclasses of: + +* `WidgetDelegate` +* `DialogDelegate` +* `BubbleDialogDelegate` + +This document generally uses `DialogDelegate` throughout. + +### The Old Pattern: DialogDelegateView Controllers + +Legacy Views dialogs often have a class like this: + +```c++ +class MyDialogView : public views::DialogDelegateView, + public SomeViewListener, + public MyModelListener { + public: + MyDialogView(MyModel* model); + ~MyDialogView() override; + + // DialogDelegate: + base::string16 GetWindowTitle() const override; + bool ShouldShowCloseButton() const override; + ... + + // MyViewListener: + void OnMyViewClicked(MyView* view) override; + + // MyModelListener: + void OnMyModelChanged(MyModel* model) override; + + private: + MyModel* model_; + Label* status_; + + ... +}; + +void MyDialogView::OnMyViewClicked(MyView* view) { + // ... complex business logic ... + model_->Update(new_state); +} + +void MyDialogView::OnModelChanged(Model* model) { + // ... complex presentation logic ... +} +``` + +### The Motivation & The Single Responsibility Principle + +The single responsibility principle is as follows: each class should have one +responsibility. The class given above has several responsibilities: + +* It functions as a `DialogDelegate` for a given `Widget` +* It functions as a `View` within that `Widget` +* It handles translating user actions on the dialog to model updates +* It handles translating model updates into visual changes in the dialog + +To make matters worse, classes of this pattern often do this in their +constructor: + +```c++ +MyDialogView::MyDialogView(MyModel* model) { + // ... lots of setup ... + views::DialogDelegate::CreateDialogWidget(this, context, parent)->Show(); +} +``` + +The last line of code of the constructor packs a lot of meaning: + +* `CreateDialogWidget` does or does not take ownership of `this`, depending on + whether `MyDialogView` overrides `DeleteDelegate` +* By default, `DialogDelegateView` uses itself as the contents view of the + created widget +* The created widget takes ownership of itself, and is shown on screen as a + side-effect of the constructor + +Doing this makes classes of this pattern exceptionally difficult to test. + +### The New Pattern: Decomposed Classes + +Here's how this class might look in "new style", using callbacks rather than +observer/listener interfaces, and using composition instead of inheritance for +both `View` and `DialogDelegate`: + +```c++ +class MyDialog { + public: + MyDialog(MyModel* model); + ~MyDialog(); + + void Show(gfx::NativeWindow context, gfx::NativeView parent); + + private: + void OnModelChanged(MyModel* model); + void OnMyViewClicked(MyView* view); + + std::unique_ptr<View> MakeContentsView(); + std::unique_ptr<DialogDelegate> MakeDialogDelegate(); + + base::CallbackListSubscription model_subscription_; + + std::unique_ptr<DialogDelegate> delegate_; + + Widget* widget_ = nullptr; // if needed + const Model* model_; // usually needed + + Label* status_ = nullptr; // or similar Views that are needed later +}; + +MyDialog::MyDialog(MyModel* model) : model_(model) { + model_subscription_ = model->RegisterUpdateCallback( + base::BindRepeating(&MyDialog::OnModelChanged, base::Unretained(this))); + + delegate_ = MakeDialogDelegate(MakeContentsView()); +} + +void MyDialog::Show(gfx::NativeWindow context, gfx::NativeView parent) { + widget_ = views::DialogDelegate::CreateDialogWidget( + std::move(delegate_), context, parent); + widget_->Show(); + // Or if we don't need to store widget_ we could just do: + views::DialogDelegate::CreateDialogWidget( + std::move(delegate_), context, parent)->Show(); +} + +std::unique_ptr<View> MyDialog::MakeContentsView() { + // Create the contents view, set up its LayoutManager, create any needed + // subviews, and store weak pointers to them. For example: + auto contents = std::make_unique<BoxLayoutView>(); + + auto status = std::make_unique<Label>(); + status->ConfigureAsDesired(); + status_ = contents->AddChildView(std::move(status)); + + // We don't need a reference to this view after creation time so we don't + // bother storing it: + auto help = std::make_unique<MyView>( + base::BindRepeating(&MyDialog::OnMyViewClicked, base::Unretained(this))); + contents->AddChildView(std::move(help)); +} + +std::unique_ptr<DialogDelegate> MyDialog::MakeDialogDelegate( + std::unique_ptr<View> contents) { + // Create a DialogDelegate and configure it as needed: + auto delegate = std::make_unique<DialogDelegate>(); + delegate->SetContentsView(std::move(contents)); + delegate->SetShowCloseButton(...); + delegate->SetTitle(...); + return delegate; +} +``` + +Now `MyDialog` has a single responsibility: it ties together a `DialogDelegate`, +a `View`, and a `MyModel`. The ownership of `MyDialog` is now very clear: +`MyDialog` is always owned by the client regardless of what happens with the +underlying `Widget`. The created `DialogDelegate` is owned by `MyDialog` unless +and until it is handed off to a `Widget`. The contents view (and hence all the +other views) are owned by the `DialogDelegate` until they are handed off (by +`DialogDelegate` itself) to the `Widget`'s `RootView`. + +### Going Stateless + +Some dialogs don't actually have any meaningful internal state. For example, +let's suppose we are displaying a dialog to the user that prompts them for a +text string, then calls a method on a given controller object with that text +string. + +In the "old style" that class might look like: + +```c++ +class MyPromptView : public DialogDelegateView { + public: + static void Show(MyController* controller, + gfx::NativeWindow context, + gfx::NativeView parent); + + private: + MyPromptView(MyController* controller); + ~MyPromptView() override; + + void OnDialogAccepted(); + + Textfield* field_; + MyController* controller_; +}; + +// static +void MyPromptView::Show(MyController* controller, gfx::NativeWindow context, + gfx::NativeView parent) { + views::DialogDelegate::CreateDialogView( + new MyPromptView(controller), context, parent)->Show(); +} + +MyPromptView::MyPromptView(MyController* controller) : controller_(controller) { + SetDialogButtons(ui::DIALOG_BUTTON_OK); + SetAcceptCallback(base::BindOnce( + &MyPromptView::OnDialogAccepted, base::Unretained(this))); + + SetLayoutManager(...); + field_ = AddChildView(std::make_unique<Textfield>(...)); + AddChildView(std::make_unique<Label>(...)); +} + +void MyPromptView::OnDialogAccepted() { + controller_->OnUserEnteredText(field_->GetText()); +} +``` + +Note that actually, the entire public interface of `MyPromptView` is a single +static method, so if we use composition instead, we get: + +### The Stateless Way + +```c++ +namespace { + +void OnDialogAccepted(MyController* controller, Textfield* field) { + controller->OnUserEnteredText(field->GetText()); +} + +std::unique_ptr<DialogDelegate> MakePromptDialog(MyController* controller) { + auto contents = std::make_unique<View>(...); + contents->SetLayoutManager(...); + + auto* field = contents->AddChildView(std::make_unique<Textfield>(...)); + contents->AddChildView(std::make_unique<Label>(...)); + + auto delegate = std::make_unique<DialogDelegate>(...); + delegate->SetAcceptCallback(base::BindOnce(&OnDialogAccepted, + base::Unretained(controller), base::Unretained(field))); + return delegate; +} + +} + +void ShowPromptDialog(MyController* controller, gfx::NativeWindow context, + gfx::NativeView parent) { + DialogDelegate::CreateDialogWidget(MakePromptDialog(controller), + context, parent)->Show(); +} +``` + +... the entire dialog class vanishes in a puff of refactoring, replaced by a +single function! + +### Refactoring step-by-step + +Let's say we are refactoring `MyDialogDelegateView` and want to produce +`MyDialog`. Here we'll assume `MyDialogDelegateView` subclasses +`DialogDelegateView`, but these steps work with any other `WidgetDelegate` +subclass. + +1. Replace every override of a `DialogDelegate` method with a call to one of the + new setter methods from that class. This can require some care, since the + values of getters may depend on state that is not available until after + construction time. +2. Have `MyDialogDelegateView` construct a separate `DialogDelegate` as needed, + possibly storing a reference to it for later use if needed. Migrate all the + setup code from (1) to target that new, separate delegate instead. Have + `MyDialogDelegateView` retain ownership of this new `DialogDelegate` member. +3. Have any code that uses `MyDialogDelegateView` as a `DialogDelegate` instead + use the new `DialogDelegate` member of that class. +4. Make `MyDialogDelegateView` not inherit from `DialogDelegate`, and rename it + to `MyDialogView` (inheriting from View rather than `DialogDelegateView`). + Since `MyDialogView` is still used as the `DialogDelegate`'s contents view, + instances of `MyDialogView` will end up owned by `Views`. +5. Have `MyDialogView` construct a separate `View` to be the dialog's contents + view, rather than using itself. Change all calls to `View` methods on + `MyDialogView` to instead target the new contents view member, and have all + external users that treat `MyDialogView` as a `View` instead use that View + member. +6. Make `MyDialogView` not inherit from `View`, and rename it to `MyDialog`. + Note that instances of `MyDialog` are not owned by anyone now, which is bad. + If the class still needs to exist at all, give it ownership semantics; + otherwise, it may be possible to refactor the `MyDialog` class away entirely + into a single function that creates the `DialogDelegate` and `View` members, + sets up the `Widget`, and returns. diff --git a/chromium/docs/use_counter_wiki.md b/chromium/docs/use_counter_wiki.md index 6b74125302b..33e6893c701 100644 --- a/chromium/docs/use_counter_wiki.md +++ b/chromium/docs/use_counter_wiki.md @@ -20,7 +20,8 @@ break-downs. UseCounter measures feature usage via UMA histogram and UKM. To add your feature to UseCounter, simply: -+ Add your feature to the blink::WebFeature enum; ++ Add your feature to the + [blink::mojom::WebFeature enum](https://cs.chromium.org/chromium/src/third_party/blink/public/mojom/web_feature/web_feature.mojom); + Usage can be measured via: * MeasureAs=\<enum value\> in the feature's IDL definition; Or * blink::UseCounter::Count() for blink side features; Or @@ -46,7 +47,7 @@ OR ```c++ MyInterface::MyBlinkSideFunction() { ... - UseCounter::Count(context, WebFeature::MyFeature); + UseCounter::Count(context, WebFeature::kMyFeature); ... } ``` @@ -55,7 +56,7 @@ OR MyBrowserSideFunction() { ... page_load_metrics::MetricsWebContentObserver::RecordFeatureUsage( - render_frame_host, blink::mojom::WebFeature::MyFeature); + render_frame_host, blink::mojom::WebFeature::kMyFeature); ... } ``` diff --git a/chromium/docs/vscode.md b/chromium/docs/vscode.md index 523473ff3c6..e29f5dab4cf 100644 --- a/chromium/docs/vscode.md +++ b/chromium/docs/vscode.md @@ -55,7 +55,7 @@ page accordingly. ### Installation Follow the steps on https://code.visualstudio.com/docs/setup/setup-overview. To -run it on Linux, just navigate to `chromium/src` folder and type `code .` in a +run it on Linux, just navigate to Chromium's `src` folder and type `code .` in a terminal. The argument to `code` is the base directory of the workspace. VS Code does not require project or solution files. However, it does store workspace settings in a `.vscode` folder in your base directory. @@ -75,24 +75,6 @@ integration to work: } ``` -#### Rendering of underscore on Linux - -As mentioned in [#35901](https://github.com/Microsoft/vscode/issues/35901), VS -Code will not show underscore (`_`) properly on Linux by default. You can work -around this issue by forcing another font such as the default `monospace` or -changing the font size in your settings: - -```json -{ - // If you want to use the default "monospace" font: - //"terminal.integrated.fontFamily": "monospace" - // If you would rather just increase the size of the font: - //"terminal.integrated.fontSize": 15 - // If you would rather decrease the size of the font: - //"terminal.integrated.fontSize": 13 -} -``` - ### Useful Extensions Up to now, you have a basic version of VS Code without much language support. @@ -146,11 +128,6 @@ The following extensions might be useful for you as well: * ***Instant Markdown*** - Instant markdown (.md) preview in your browser as you type. This document was written with this extension! -* ***you-complete-me*** - - Alternative autocomplete extension. Can be configured to use a variety of - language servers, so helpful if not using clangd for code completion. - See [You-Complete-Me extension setup](#You-Complete-Me-extension-setup) - for additional setup instructions. Also be sure to take a look at the [VS Code marketplace](https://marketplace.visualstudio.com/VSCode) to check out @@ -256,8 +233,8 @@ $ cp tools/vscode/settings.json5 .vscode/settings.json ``` Note: these settings assume that the workspace folder (the root folder displayed -in the Explorer tab) is chromium/src. If this is not the case, replace any -references to ${workspaceFolder} with the path to chromium/src. +in the Explorer tab) is Chromium's `src/` directory. If this is not the case, +replace any references to ${workspaceFolder} with the path to your `src/`. ### Tasks Next, we'll tell VS Code how to compile our code, run tests, and to read @@ -400,13 +377,10 @@ Keyboard Shortcuts` and add `{ "key": "ctrl+r", "command": sufficient to press `Ctrl+R` and enter `<n>`. #### Working on Laptop -Because autocomplete is provided by the You-Complete-Me extension, consider -disabling C/C++ autocomplete and indexing to save battery. In addition, you -might want to disable git status autorefresh as well. +You might want to disable git status autorefresh to save battery. ``` "git.autorefresh": false, -"C_Cpp.autocomplete": "Disabled", ``` ### Unable to open $File resource is not available when debugging Chromium on Linux @@ -415,31 +389,10 @@ the file path to be relative to the output dir. Check `gn args out/$dir --list` if `strip_absolute_paths_from_debug_symbols` is true (which is the default), set `cwd` to the output dir. otherwise, set `cwd` to `${workspaceRoot}`. -### You-Complete-Me extension setup -If using the You-Complete-Me extension, complete its installation by entering -these commands in a terminal: - -``` -$ git clone https://github.com/Valloric/ycmd.git ~/.ycmd -$ cd ~/.ycmd -$ git submodule update --init --recursive -$ ./build.py --clang-completer -``` -If it fails with "Your C++ compiler does NOT fully support C++11." but you know -you have a good compiler, hack cpp/CMakeLists.txt to set CPP11_AVAILABLE true. - -On Mac, replace the last command above with the following. - -``` -$ ./build.py --clang-completer --system-libclang -``` - -On Windows, if depot_tools' Python is the only one installed, a separate Python -3 install is needed. The last command should then be run as follows. - -``` -> <Python 3 directory>/python.exe build.py --clang-completer -``` +### You-Complete-Me +The You-Complete-Me VS Code extension is now +[deprecated](https://github.com/richard1122/vscode-youcompleteme#deprecated) +with a suggestion to use clangd. ### More More tips and tricks can be found diff --git a/chromium/docs/webui_explainer.md b/chromium/docs/webui_explainer.md index f62e208a4c7..fbbb482b1aa 100644 --- a/chromium/docs/webui_explainer.md +++ b/chromium/docs/webui_explainer.md @@ -258,8 +258,10 @@ So, the given C++ code: ```c++ void OvenHandler::RegisterMessages() { - web_ui()->RegisterMessageHandler("bakeDonuts", - base::Bind(&OvenHandler::HandleBakeDonuts, base::Unretained(this))); + web_ui()->RegisterMessageCallback( + "bakeDonuts", + base::BindRepeating(&OvenHandler::HandleBakeDonuts, + base::Unretained(this))); } void OvenHandler::HandleBakeDonuts(const base::ListValue* args) { @@ -359,11 +361,11 @@ chrome/browser/ui/webui/webui\_util.\* contains a number of methods to simplify common configuration tasks. <a name="AddLocalizedStringsBulk"></a> -### webui::AddLocalizedStringsBulk() +### WebUIDataSource::AddLocalizedStrings() Many Web UI data sources need to be set up with a large number of localized strings. Instead of repeatedly calling <code>AddLocalizedString()</code>, create -an array of all the strings and use <code>AddLocalizedStringsBulk()</code>: +an array of all the strings and use <code>AddLocalizedStrings()</code>: ```c++ static constexpr webui::LocalizedString kStrings[] = { @@ -372,16 +374,15 @@ an array of all the strings and use <code>AddLocalizedStringsBulk()</code>: {"ariaRoleDescription", IDS_HISTORY_ARIA_ROLE_DESCRIPTION}, {"bookmarked", IDS_HISTORY_ENTRY_BOOKMARKED}, }; - AddLocalizedStringsBulk(source, kStrings); + source->AddLocalizedStrings(kStrings); ``` -<a name="AddResourcePathsBulk"></a> -### webui::AddResourcePathsBulk() +<a name="AddResourcePaths"></a> +### WebUIDataSource::AddResourcePaths() Similar to the localized strings, many Web UIs need to add a large number of -resource paths. In this case, use <code>AddResourcePathsBulk()</code> to -replace repeated calls to <code>AddResourcePath()</code>. There are two -versions. One works almost identically to the strings case: +resource paths. In this case, use <code>AddResourcePaths()</code> to +replace repeated calls to <code>AddResourcePath()</code>. ```c++ static constexpr webui::ResourcePath kPdfResources[] = { @@ -389,20 +390,18 @@ versions. One works almost identically to the strings case: {"pdf/constants.js", IDR_PDF_CONSTANTS_JS}, {"pdf/controller.js", IDR_PDF_CONTROLLER_JS}, }; - webui::AddResourcePathsBulk(source, kStrings); + source->AddResourcePaths(kStrings); ``` -The second version instead accepts a span of <code>GritResourceMap</code> so -that it can directly use constants defined by autogenerated grit resources map -header files. For example, the autogenerated print\_preview\_resources\_map.h -header defines a <code>GritResourceMap</code> named -<code>kPrintPreviewResources</code> and a -<code>size\_t kPrintPreviewResourcesSize</code>. All the resources in this +The same method can be leveraged for cases that directly use constants defined +by autogenerated grit resources map header files. For example, the autogenerated +print\_preview\_resources\_map.h header defines a +<code>webui::ResourcePath</code> array named <code>kPrintPreviewResources</code> +and a <code>size\_t kPrintPreviewResourcesSize</code>. All the resources in this resource map can be added as follows: ```c++ - webui::AddResourcePathsBulk( - source, + source->AddResourcePaths( base::make_span(kPrintPreviewResources, kPrintPreviewResourcesSize)); ``` @@ -727,7 +726,8 @@ renderer: v8::Local<v8::Object> chrome = GetOrCreateChromeObject(isolate, context); chrome->Set(gin::StringToSymbol(isolate, "send"), gin::CreateFunctionTemplate( - isolate, base::Bind(&WebUIExtension::Send))->GetFunction()); + isolate, + base::BindRepeating(&WebUIExtension::Send))->GetFunction()); ``` The `chrome.send()` method takes a message name and argument list. @@ -883,28 +883,34 @@ since taking control of a WebUI page can sometimes be sufficient to escape Chrome's sandbox. To make sure that the special powers granted to WebUI pages are safe, WebUI pages are restricted in what they can do: -* WebUI pages cannot embed http/https resources or frames +* WebUI pages cannot embed http/https resources * WebUI pages cannot issue http/https fetches In the rare case that a WebUI page really needs to include web content, the safe -way to do this is by using a `<webview>` tag. Using a `<webview>` tag is more -secure than using an iframe for multiple reasons, even if Site Isolation and -out-of-process iframes keep the web content out of the privileged WebUI process. - -First, the content inside the `<webview>` tag has a much reduced attack surface, -since it does not have a window reference to its embedder or any other frames. -Only postMessage channel is supported, and this needs to be initiated by the -embedder, not the guest. - -Second, the content inside the `<webview>` tag is hosted in a separate -StoragePartition. Thus, cookies and other persistent storage for both the WebUI -page and other browser tabs are inaccessible to it. - -This greater level of isolation makes it safer to load possibly untrustworthy or -compromised web content, reducing the risk of sandbox escapes. - -For an example of switching from iframe to webview tag see -https://crrev.com/c/710738. +way to do this is by using an `<iframe>` tag. Chrome's security model gives +process isolation between the WebUI and the web content. However, some extra +precautions need to be taken, because there are properties of the page that are +accessible cross-origin and malicious code can take advantage of such data to +attack the WebUI. Here are some things to keep in mind: + +* The WebUI page can receive postMessage payloads from the web and should + ensure it verifies any messages as they are not trustworthy. +* The entire frame tree is visible to the embedded web content, including + ancestor origins. +* The web content runs in the same StoragePartition and Profile as the WebUI, + which reflect where the WebUI page was loaded (e.g., the default profile, + Incognito, etc). The corresponding user credentials will thus be available to + the web content inside the WebUI, possibly showing the user as signed in. + +Note: WebUIs have a default Content Security Policy which disallows embedding +any frames. If you want to include any web content in an <iframe> you will need +to update the policy for your WebUI. When doing so, allow only known origins and +avoid making the policy more permissive than strictly necessary. + +Alternatively, a `<webview>` tag can be used, which runs in a separate +StoragePartition, a separate frame tree, and restricts postMessage communication +by default. However, `<webview>` does not support Site Isolation and +therefore it is not advisable to use for any sensitive content. ## See also diff --git a/chromium/docs/webui_in_chrome.md b/chromium/docs/webui_in_chrome.md new file mode 100644 index 00000000000..f24a9c5eacd --- /dev/null +++ b/chromium/docs/webui_in_chrome.md @@ -0,0 +1,330 @@ +<style> +.note::before { + content: 'Note: '; + font-variant: small-caps; + font-style: italic; +} + +.doc h1 { + margin: 0; +} +</style> + +# Creating WebUI Interfaces outside components/ +This guide is based on [Creating WebUI Interfaces in components](webui_in_components), and comments from reviewers when creating the ChromeOS emoji picker. + +[TOC] + +<a name="creating_web_ui_page"></a> +WebUI pages live in `chrome/browser/resources`. You should create a folder for your project `chrome/browser/resources/hello_world`. +When creating WebUI resources, follow the [Web Development Style Guide](https://chromium.googlesource.com/chromium/src/+/master/styleguide/web/web.md). For a sample WebUI page you could start with the following files: + +`chrome/browser/resources/hello_world/hello_world.html` +```html +<!DOCTYPE HTML> +<html> +<head> + <meta charset="utf-8"> + <link rel="stylesheet" href="hello_world.css"> + <script type="module" src="hello_world.js"></script> +</head> +<body> + <h1>Hello World</h1> + <div id="example-div"></div> +</body> +</html> +``` + +`chrome/browser/resources/hello_world/hello_world.css` +```css +body { + margin: 0; +} +``` + +`chrome/browser/resources/hello_world/hello_world.js` +```js +import './strings.m.js'; + +import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js'; +import {$} from 'chrome://resources/js/util.m.js'; + +function initialize() { + const block = document.createElement('div'); + block.innerText = loadTimeData.getString('message'); + $('example-div').appendChild(block); +} + +document.addEventListener('DOMContentLoaded', initialize); +``` + +Add a `BUILD.gn` file to get Javascript type checking: + +`chrome/browser/resources/hello_world/BUILD.gn` +``` +import("//third_party/closure_compiler/compile_js.gni") + +js_library("hello_world") { + deps = [ + "//ui/webui/resources/js:load_time_data.m", + "//ui/webui/resources/js:util.m", + ] +} + +js_type_check("closure_compile") { + deps = [ ":hello_world" ] +} +``` + +Then refer to the new `:closure_compile` target from `chrome/browser/resources/BUILD.gn`: + +``` +group("closure_compile) { + deps = [ + ... + "hello_world:closure_compile" + ... + ] +``` + +Finally, create an `OWNERS` file for the new folder. + +## Adding the resources +Resources for the browser are stored in `grd` files. Current best practice is to autogenerate a grd file for your +component in the `BUILD` file we created earlier + +`chrome/browser/resources/hello_world/BUILD.gn` +``` +import("//tools/grit/grit_rule.gni") +import("//ui/webui/resources/tools/generate_grd.gni") + +resources_grd_file = "$target_gen_dir/resources.grd" +generate_grd("build_grd") { + grd_prefix = "hello_world" + out_grd = resources_grd_file + input_files = [ + "hello_world.css", + "hello_world.html", + "hello_world.js", + ] + input_files_base_dir = rebase_path(".", "//") +} + +grit("resources") { + enable_input_discovery_for_gn_analyze = false + source = resources_grd_file + deps = [ ":build_grd" ] + outputs = [ + "grit/hello_world_resources.h", + "grit/hello_world_resources_map.cc", + "grit/hello_world_resources_map.h", + "hello_world_resources.pak", + ] + output_dir = "$root_gen_dir/chrome" +} +``` + +Then add the new resource target to `chrome/browser/resources/BUILD.gn` +``` +group("resources") { + public_deps += [ + ... + "hello_world:resources" + ... + ] +} +``` + +## Adding URL constants for the new chrome URL + +`chrome/common/webui_url_constants.cc:` +```c++ +const char kChromeUIHelloWorldURL[] = "chrome://hello-world/"; +const char kChromeUIHelloWorldHost[] = "hello-world"; +``` + +`chrome/common/webui_url_constants.h:` +```c++ +extern const char kChromeUIHelloWorldURL[]; +extern const char kChromeUIHelloWorldHost[]; +``` + +## Adding a WebUI class for handling requests to the chrome://hello-world/ URL +Next we need a class to handle requests to this new resource URL. Typically this will subclass `WebUIController` (WebUI +dialogs will also need another class which will subclass `WebDialogDelegate`, this is shown later). + +`chrome/browser/ui/webui/hello_world_ui.h` +```c++ +#ifndef CHROME_BROWSER_UI_WEBUI_HELLO_WORLD_HELLO_WORLD_H_ +#define CHROME_BROWSER_UI_WEBUI_HELLO_WORLD_HELLO_WORLD_H_ + +#include "base/macros.h" +#include "content/public/browser/web_ui_controller.h" + +// The WebUI for chrome://hello-world +class HelloWorldUI : public content::WebUIController { + public: + explicit HelloWorldUI(content::WebUI* web_ui); + ~HelloWorldUI() override; + private: +}; + +#endif // CHROME_BROWSER_UI_WEBUI_HELLO_WORLD_HELLO_WORLD_H_ +``` + +`chrome/browser/ui/webui/hello_world_ui.cc` +```c++ +#include "chrome/browser/ui/webui/hello_world_ui.h" + +#include "chrome/browser/ui/webui/webui_util.h" +#include "components/hello_world/constants.h" +#include "content/public/browser/browser_context.h" +#include "content/public/browser/web_contents.h" +#include "content/public/browser/web_ui.h" +#include "content/public/browser/web_ui_data_source.h" + + +HelloWorldUI::HelloWorldUI(content::WebUI* web_ui) + : content::WebUIController(web_ui) { + // Set up the chrome://hello-world source. + content::WebUIDataSource* html_source = + content::WebUIDataSource::Create(chrome::kChromeUIHelloWorldHost); + + // As a demonstration of passing a variable for JS to use we pass in some + // a simple message. + html_source->AddString("message", "Hello World!"); + html_source->UseStringsJs(); + + // Add required resources. + webui::SetupWUIDataSource(html_source, base::make_span(kHelloWorldResources, kHelloWorldResourcesSize), IDR_HELLO_WORLD_HELLO_WORLD_HTML); + + content::BrowserContext* browser_context = + web_ui->GetWebContents()->GetBrowserContext(); + content::WebUIDataSource::Add(browser_context, html_source); +} + +HelloWorldUI::~HelloWorldUI() {} +``` + +To ensure that your code actually gets compiled, you need to add it to `chrome/browser/ui/BUILD.gn`: + +``` +static_library("ui") { + sources = [ + ... (lots) + "webui/hello_world/hello_world_ui.cc", + "webui/hello_world/hello_world_ui.h", +``` + +## Adding your WebUI request handler to the Chrome WebUI factory + +The Chrome WebUI factory is where you setup your new request handler. + +`chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:` +```c++ ++ #include "chrome/browser/ui/webui/hello_world_ui.h" +... ++ if (url.host() == chrome::kChromeUIHelloWorldHost) ++ return &NewWebUI<HelloWorldUI>; +``` + +## Check everything works + +You're done! Assuming no errors (because everyone gets their code perfect the first time) you should be able to compile +and run chrome and navigate to `chrome://hello-world/` and see your nifty welcome text! + + +## Making a WebUI Dialog + +Instead of having a full page for your WebUI, you might want a dialog in order to have a fully independent window. To +do that, some small changes are needed to your code. First, we need to add a new class which inherits from +`ui::WebDialogDelegate`. The easiest way to do that is to edit the `hello_world_ui.*` files + + +`chrome/browser/ui/webui/hello_world_ui.h` +```c++ + // Leave the old content, but add this new code + class HelloWorldDialog : public ui::WebDialogDelegate { + public: + static void Show(); + ~HelloWorldDialog() override; + + private: + HelloWorldDialog(); + // ui::WebDialogDelegate: + ui::ModalType GetDialogModalType() const override; + base::string16 GetDialogTitle() const override; + GURL GetDialogContentURL() const override; + void GetWebUIMessageHandlers( + std::vector<content::WebUIMessageHandler*>* handlers) const override; + void GetDialogSize(gfx::Size* size) const override; + std::string GetDialogArgs() const override; + void OnDialogShown(content::WebUI* webui) override; + void OnDialogClosed(const std::string& json_retval) override; + void OnCloseContents(content::WebContents* source, + bool* out_close_dialog) override; + bool ShouldShowDialogTitle() const override; + + content::WebUI* webui_ = nullptr; + + DISALLOW_COPY_AND_ASSIGN(HelloWorldDialog); +}; +``` + +`chrome/browser/ui/webui/hello_world_ui.cc` +```c++ + // Leave the old content, but add this new stuff + +HelloWorldDialog::HelloWorldDialog() {} + +void HelloWorldDialog::Show() { + chrome::ShowWebDialog(nullptr, ProfileManager::GetActiveUserProfile(), + new HelloWorldDialog()); +} + +ui::ModalType HelloWorldDialog::GetDialogModalType() const { + return ui::MODAL_TYPE_NONE; +} + +base::string16 HelloWorldDialog::GetDialogTitle() const { + return base::UTF8ToUTF16("Hello world"); +} + +GURL HelloWorldDialog::GetDialogContentURL() const { + return GURL(chrome::kChromeUIHelloWorldURL[); +} + +void HelloWorldDialog::GetWebUIMessageHandlers( + std::vector<content::WebUIMessageHandler*>* handlers) const {} + +void HelloWorldDialog::GetDialogSize(gfx::Size* size) const { + const int kDefaultWidth = 544; + const int kDefaultHeight = 628; + size->SetSize(kDefaultWidth, kDefaultHeight); +} + +std::string HelloWorldDialog::GetDialogArgs() const { + return ""; +} + +void HelloWorldDialog::OnDialogShown(content::WebUI* webui) { + webui_ = webui; +} + +void HelloWorldDialog::OnDialogClosed(const std::string& json_retval) { + delete this; +} + +void HelloWorldDialog::OnCloseContents(content::WebContents* source, + bool* out_close_dialog) { + *out_close_dialog = true; +} + +bool HelloWorldDialog::ShouldShowDialogTitle() const { + return true; +} + +HelloWorldDialog::~HelloWorldDialog() = default; +``` + +Finally, you will need to do something to actually show your dialog, which can be done by calling `HelloWorldDialog::Show()`. diff --git a/chromium/docs/win_cross.md b/chromium/docs/win_cross.md index 53195762539..4ca0c16407d 100644 --- a/chromium/docs/win_cross.md +++ b/chromium/docs/win_cross.md @@ -41,7 +41,7 @@ If you're at Google, this will automatically download the Windows SDK for you. If this fails with an error: Please follow the instructions at - https://chromium.googlesource.com/chromium/src/+/master/docs/windows_build_instructions.md + https://chromium.googlesource.com/chromium/src/+/HEAD/docs/win_cross.md then you may need to re-authenticate via: @@ -82,25 +82,14 @@ Add `target_os = "win"` to your args.gn. Then just build, e.g. ## Goma -*** note -**Warning:** This is unsupported and known to not work at times. -*** - -For now, one needs to use the rbe backend, not the borg backend -(default for Googlers). -Use cloud backend instead. +This should be supported by the default (Goma RBE) backend. However, there may +be issues with arbitrary toolchain support on Linux (b/177871873). +This can be disabled via: ```shell - goma_auth.py login - - # GOMA_* are needed for Googlers only - export GOMA_SERVER_HOST=goma.chromium.org - export GOMA_RPC_EXTRA_PARAMS=?rbe - - goma_ctl.py ensure_start +GOMA_ARBITRARY_TOOLCHAIN_SUPPORT=false goma_ctl restart ``` - ## Copying and running chrome A convenient way to copy chrome over to a Windows box is to build the diff --git a/chromium/docs/win_order_files.md b/chromium/docs/win_order_files.md index 2366f78a4dd..aa521ed1cd6 100644 --- a/chromium/docs/win_order_files.md +++ b/chromium/docs/win_order_files.md @@ -1,86 +1,8 @@ # Updating the Windows .order files -The `chrome/build/*.orderfile` files are used to specify the order in which -the linker should lay out symbols in the binary it's producing. By ordering -functions in the order they're typically executed during start-up, the start-up -time can be reduced slightly. - -The order files are used automatically when building with Clang for Windows with -the gn flag `is_official_build` set to `true`. - -To update the order files: - -1. Build with instrumentation enabled: - - The instrumentation will capture the couple of million function calls - in a binary as it runs and write them to a file in the `\src\tmp` directory. - Make sure this directory exists. - - ```shell - gn gen out\instrument --args="is_debug=false is_official_build=true generate_order_files=true symbol_level=1" - ninja -C out\instrument chrome - ``` - - (If you have access to Goma, add `use_goma=true` to the gn args and `-j500` - to the Ninja invocation.) - - -1. Run the instrumented binaries: - - (Some binaries such as `mksnapshot`, `yasm`, and `protoc` already ran with - instrumentation during the build process. The instrumentation output should - be available under `\src\tmp`.) - - Open the Task Manager's Details tab or - [Process Explorer](https://docs.microsoft.com/en-us/sysinternals/downloads/process-explorer) - to be able to see the Process IDs of running programs. - - Run Chrome: - - ```shell - out\instrument\chrome - ``` - - Note the Process ID of the browser process. - - Check in `\src\tmp\` for instrumentation output from the process, for - example `cygprofile_14652.txt`. The files are only written once a certain - number of function calls have been made, so sometimes you need to browse a - bit for the file to be produced. - -1. If the file appears to have sensible contents (a long list of function names - that eventually seem related to what the browser should - do), copy it into `chrome\build\`: - - ```shell - copy \src\tmp\cygprofile_25392.txt chrome\build\chrome.x64.orderfile - ``` - -1. Re-build the `chrome` target. This will re-link `chrome.dll` - using the new order file and surface any link errors if - the order file is broken. - - ```shell - ninja -C out\instrument chrome - ``` - - -1. Repeat the previous steps with a 32-bit build, i.e. passing - `target_cpu="x86"` to gn and storing the file as `.x86.orderfile`. - - -1. Upload the order files to Google Cloud Storage. They will get downloaded - by a `gclient` hook based on the contents of the `.orderfile.sha1` files. - - You need to have write access to the `chromium-browser-clang` GCS bucket - for this step. - - ```shell - cd chrome\build\ - upload_to_google_storage.py -b chromium-browser-clang/orderfiles -z orderfile chrome.x64.orderfile chrome.x86.orderfile - gsutil.py setacl public-read gs://chromium-browser-clang/orderfiles/* - ``` - - -1. Check in the `.sha1` files corresponding to the orderfiles created by the - previous step. +Since [#824529](https://crrev.com/824529), order files are no longer used for +linking Chromium on Windows. Instead, the linker orders the binary contents +based on profile-guided optimization (PGO) data, using LLD's call graph profile +sort feature. That provides similar benefits and uses use the existing PGO +infrastructure instead of requiring maintenance of the order files. See +[crbug.com/1077279](https://crbug.com/1077279). diff --git a/chromium/docs/windows_build_instructions.md b/chromium/docs/windows_build_instructions.md index 05fe102f0fa..5a58b355537 100644 --- a/chromium/docs/windows_build_instructions.md +++ b/chromium/docs/windows_build_instructions.md @@ -17,7 +17,7 @@ Are you a Google employee? See * At least 100GB of free disk space on an NTFS-formatted hard drive. FAT32 will not work, as some of the Git packfiles are larger than 4GB. * An appropriate version of Visual Studio, as described below. -* Windows 7 or newer. +* Windows 10 or newer. ## Setting up Windows diff --git a/chromium/docs/workflow/debugging-with-swarming.md b/chromium/docs/workflow/debugging-with-swarming.md index ace0bb8622c..b0845a71077 100644 --- a/chromium/docs/workflow/debugging-with-swarming.md +++ b/chromium/docs/workflow/debugging-with-swarming.md @@ -16,7 +16,7 @@ of [Borg], or to [Kubernetes]. An *isolate* is an archive containing all the files needed to do a specific task on the swarming infrastructure. It contains binaries as well as any libraries they link against or support data. An isolate can be thought of like a tarball, -but held by the "isolate server" and identified by a hash of its contents. The +but held by the CAS server and identified by a digest of its contents. The isolate also includes the command(s) to run, which is why the command is specified when building the isolate, not when executing it. @@ -50,7 +50,7 @@ want to do is: or perhaps: ``` - isolate = upload_to_isolate_server(target_you_built_locally) + isolate = upload_to_cas(target_you_built_locally) use_swarming_to_run(type, isolate) ``` @@ -167,19 +167,21 @@ Use your google.com account for this. ## Uploading an isolate -You can then upload the resulting isolate to the isolate server: +You can then upload the resulting isolate to the CAS server: ``` $ tools/luci-go/isolate archive \ - -I https://isolateserver.appspot.com \ + -cas-instance chroimum-swarm \ -i $outdir/$target.isolate \ - -s $outdir/$target.isolated + -dump-json $outdir/$target.archive.json ``` -The `isolate` tool will emit something like this: +The archive json looks like this: ``` -e625130b712096e3908266252c8cd779d7f442f1 unit_tests +{ + "unit_tests": "5bee0815d2ddd2b876b49d4cce8aaa23de8a6f9e2dbf134ec409fbdc224e8c64/398" +} ``` Do not ctrl-c it after it does this, even if it seems to be hanging for a @@ -187,16 +189,15 @@ minute - just let it finish. ## Running an isolate -Now that the isolate is on the isolate server with hash `$hash` from the +Now that the isolate is on the CAS server with digest `$digest` from the previous step, you can run on bots of your choice: ``` $ tools/luci-go/swarming trigger \ -server https://chromium-swarm.appspot.com \ - -isolate-server https://isolateserver.appspot.com \ -dimension pool=$pool \ $criteria \ - -isolated $hash + -digest $digest ``` There are two more things you need to fill in here. The first is the pool name; |
