[iOS] Prepare CWVWebView web embedder for use in iOS #26957

kylehickinson · 2024-12-10T18:15:38Z

This is the split-up contents of #24657. This PR in particular only adds the CWVWebView API and exposes it to the iOS app but does not actually contain the code that uses CWVWebView in the iOS app. There is a shell target added (WebShell) for simple tests, for more comprehensive tests involving multiple tabs, please use the ios-cwvwebview-impl branch

Resolves brave/brave-browser#42838

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on
All relevant documentation has been updated, for instance:

Test Plan:

github-actions · 2025-01-27T18:12:54Z

Chromium major version is behind target branch (132.0.6834.111 vs 133.0.6943.27). Please rebase.

bridiver · 2025-01-27T23:06:57Z

chromium_src/ios/CPPLINT.cfg

@@ -0,0 +1,2 @@
+# Common linting false posititives from Obj-C syntax in headers
+filter=-readability/casting,-whitespace/parens,-whitespace/operators


why do we have an issue with this if upstream doesn't have it in their config?

Im not sure, we have the same issue in our own ios folder https://github.com/brave/brave-core/blob/master/ios/CPPLINT.cfg

what specifically is triggering it?

src/brave/chromium_src/ios/web_view/internal/cwv_web_view_configuration_internal.h:34: (cpplint) Using C-style cast. Use reinterpret_cast<CWVWebView*>(...) instead [readability/casting] [4] src/brave/chromium_src/ios/web_view/internal/cwv_web_view_internal.h:15: (cpplint) Using C-style cast. Use reinterpret_cast<NSData*>(...) instead [readability/casting] [4] ** Presubmit Messages: 1 **

The actual lines it references don't make sense. For instance the first one is just a standard Obj-C method:

- (void)registerWebView:(CWVWebView*)webView;

The second is

+ (BOOL)_isRestoreDataValid:(NSData*)data;

cc @goodov if you have any ideas?

bridiver · 2025-01-27T23:09:07Z

chromium_src/ios/chrome/browser/https_upgrades/model/DEPS

@@ -0,0 +1,4 @@
+include_rules = [
+  "+brave/components/https_upgrade_exceptions/browser",
+  "+brave/ios/browser/application_context",


this can probably go in chromium_src/ios/chrome/browser/DEPS

bridiver · 2025-01-27T23:12:19Z

patches/ios-web_view-public-cwv_navigation_delegate.h.patch

+ // (usually by crashing, though possibly by other means).
+ - (void)webViewWebContentProcessDidTerminate:(CWVWebView*)webView;
+
+- (BOOL)webView:(CWVWebView*)webView shouldBlockUniversalLinksForRequest:(NSURLRequest*)request;


#define webViewWebContentProcessDidTerminate webView:(CWVWebView*)webView didRequestHTTPAuthForProtectionSpace... rest of methods we add... webViewWebContentProcessDidTerminate

We can't use chromium_src overrides for these public headers because they are copied into the BraveCore framework target (public headers folder), so they have to be patched at rest

bridiver · 2025-01-27T23:37:35Z

patches/ios-web_view-internal-cwv_web_view.mm.patch

+ #pragma mark - Autofill
+
+ - (CWVAutofillController*)autofillController {
+  BRAVE_NOP_SERVICE


can we override AutofillAgent to be a no-op?

We could be this is still to ensure you cant access it from the CWVWebView property

bridiver · 2025-01-27T23:42:55Z

patches/ios-web_view-internal-cwv_web_view.mm.patch

+   }
+
+   [self attachSecurityInterstitialHelpersToWebStateIfNecessary];
+  BRAVE_ATTACH_TAB_HELPERS


why are we attaching general tab helpers here? Shouldn't this happen automatically through BrowserWebStateListDelegate if we are adding them in upstream AttachTabHelpers?

None of the ios/web_view parts go through Browser/WebStateList

bridiver · 2025-01-27T23:43:02Z

patches/ios-web_view-internal-cwv_web_view.mm.patch

+ #pragma mark - Translation
+
+ - (CWVTranslationController*)translationController {
+  BRAVE_NOP_SERVICE


same as below

patches/ios-web_view-public-cwv_web_view.h.patch

bridiver · 2025-01-28T19:20:39Z

ios/brave-ios/App/WebShell/ContentView.swift

+}()
+
+@dynamicMemberLookup
+class WebViewModel: ObservableObject {


What is the class for/what does it do? Please add comments

Sorry this was just thrown together to demonstrate the CWVWebView in a small shell app. This code specifically model glue for SwiftUI so the shell UI updates properly when observable properties change

oic, this was the thing I was asking about to run

can you add comments explaining?

bridiver · 2025-02-26T17:55:22Z

chromium_src/ios/web_view/internal/cwv_web_view.mm

+  return (cachedProtobufStorage != nil || cachedSessionStorage != nil);
+}
+
+ (id)objectFromValue:(const base::Value*)value {


why does this need to be a method on CWVWebView? Can't we just use NSObjectFromValue directly?

Its a function in an anonymous namespace defined in cwv_web_view.mm and not exposed in cwv_web_view_internal.h so this seemed like the easiest solution: https://source.chromium.org/chromium/chromium/src/+/main:ios/web_view/internal/cwv_web_view.mm;l=114

I think we should keep the same name and not drop NS?

It'd be against typical Obj-C naming conventions, you don't include the class prefixes in the method naming

this is effectively private since it's internal, right? It can only be used within CWVWebView?

bridiver · 2025-02-26T18:31:06Z

ios/web_view/headers.gni

+  "//brave/ios/web_view/public/cwv_ssl_status_extras.h",
+  "//brave/ios/web_view/public/cwv_x509_certificate_extras.h",
+  "//brave/ios/web_view/public/cwv_web_view_extras.h",
+


empty line?

To separate the //brave specific extras and the headers coming from Chromium's //ios/web_view

bridiver · 2025-02-26T18:32:10Z

ios/browser/web/brave_web_main_parts.mm

- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this file,
- * You can obtain one at http://mozilla.org/MPL/2.0/. */
+// Copyright (c) 2019 The Brave Authors. All rights reserved.


we backdated it?

This file was moved into the web subdirectory to match chromium, but its been around since 2019

bridiver · 2025-02-26T18:38:48Z

ios/browser/api/content_settings/default_host_content_settings.h

+
+/// A list of default website preferences.
+OBJC_EXPORT
+@interface DefaultHostContentSettings : NSObject


this is fine for now, but we should really have an api wrapper for host content settings map in general

bridiver · 2025-02-26T20:45:04Z

ios/browser/web/brave_web_client.mm

+  features.push_back(
+      security_interstitials::IOSSecurityInterstitialJavaScriptFeature::
+          GetInstance());
+  features.push_back(translate::TranslateJavaScriptFeature::GetInstance());


I thought we weren't using chromium translate?

We actually are now but not through JavaScriptFeature, this is actually supposed to be removed though until we can properly support it as a JavaScriptFeature

bridiver · 2025-02-26T20:50:43Z

ios/web_view/internal/cwv_web_view_extras.mm

+      }));
+}
+
+- (WKWebView*)underlyingWebView {


why aren't we just exposing the CRWWebController as an attribute and then we can also use that for createPDF, etc...

As per DM:

We don't want to expose CRWWebController as its essentially the WKWebView handler, which we want to avoid using and overtime will use it less (for when we eventually can support a use_blink build)

We can't expose CRWWebController as-is, would have to wrap it anyways

discussed in DM, but maybe this should be internalWebView or something?

bridiver · 2025-02-26T21:28:06Z

ios/web_view/internal/cwv_web_view_extras.mm

+  config_provider.UpdateScripts();
+}
+
+- (void)createPDF:(void (^)(NSData* _Nullable))completionHandler {


createFullPagePdf to match upstream method. It's much easier to find things when names match

bridiver

lgtm after removing https upgrade code that needs fixes in desktop first

github-actions · 2025-02-27T15:29:33Z

[puLL-Merge] - brave/brave-core@26957

Description

This PR adds support for CWVWebView (Chromium WebView) integration in Brave iOS, allowing Brave to reuse Chromium's WebKit implementation while adding Brave-specific features and customizations. The changes include:

Integration of CWVWebView functionality into Brave's codebase
Implementation of Brave-specific web features and protections
Addition of a WebShell test app for validating web view functionality
Override and extension of Chromium's web view implementation

Changes

Changes

By File Category:

Core Web Client Changes:

brave_web_client.h/mm: Updates web client implementation to support Brave features
brave_web_main_parts.h/mm: Modified web main parts initialization

WebView Extensions:

cwv_web_view_extras.h/mm: Adds Brave-specific functionality to CWVWebView
cwv_ssl_status_extras.h/mm: Extended SSL status functionality
cwv_x509_certificate_extras.h/mm: Additional certificate handling capabilities

Build & Project Config:

Added new WebShell.xcscheme
Modified Client.xcodeproj to include WebShell target
Updated various BUILD.gn and DEPS files

Test App:

Added WebShell test application with SwiftUI implementation
WebShellApp.swift: Main app entry point
ContentView.swift: UI implementation for testing web view features

sequenceDiagram
    participant App as WebShell App
    participant Core as BraveCoreMain
    participant WebView as CWVWebView
    participant Client as WebClient
    
    App->>Core: Initialize
    Core->>Client: Configure WebClient
    App->>Core: Get WebViewConfiguration
    Core-->>App: Return configuration
    App->>WebView: Create with configuration
    WebView->>Client: Setup navigation handlers
    WebView->>WebView: Attach Brave features
    App->>WebView: Load initial URL
    WebView->>Client: Process navigation

Security Hotspots

JavascriptFeature implementations - these need to be carefully reviewed as they offer direct page script access
SSL certificate and error handling - critical security UI elements must accurately reflect security states
Universal link handling - proper URL validation needed to prevent malicious redirects

Possible Issues

Memory management for browser state cleanup between regular and private browsing
Performance impact of additional security checks during navigation
Compatibility concerns with existing Chrome iOS features when using Brave's modifications

brave-builds · 2025-02-28T00:04:38Z

Released in v1.78.4

kylehickinson self-assigned this Dec 10, 2024

kylehickinson requested review from a team as code owners December 10, 2024 18:15

github-actions bot added the puLL-Merge label Dec 10, 2024

kylehickinson mentioned this pull request Dec 10, 2024

[iOS] Use Chromium web embedder (CWVWebView) #24657

Closed

36 tasks

Brandon-T approved these changes Dec 13, 2024

View reviewed changes

kylehickinson force-pushed the ios-cwvwebview-core branch 2 times, most recently from 66c250f to 18f7570 Compare January 13, 2025 14:48

kylehickinson requested a review from a team as a code owner January 13, 2025 14:48

kylehickinson force-pushed the ios-cwvwebview-core branch 2 times, most recently from 58f7166 to dbd74e5 Compare January 24, 2025 20:15

github-actions bot added the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Jan 27, 2025

kylehickinson force-pushed the ios-cwvwebview-core branch from dbd74e5 to 7e2255a Compare January 27, 2025 18:59

github-actions bot removed the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Jan 27, 2025

bridiver reviewed Jan 27, 2025

View reviewed changes

patches/ios-web_view-public-cwv_web_view.h.patch Show resolved Hide resolved

bridiver reviewed Jan 28, 2025

View reviewed changes

bridiver reviewed Feb 26, 2025

View reviewed changes

bridiver approved these changes Feb 26, 2025

View reviewed changes

kylehickinson force-pushed the ios-cwvwebview-core branch from 6ce5992 to 9b658bf Compare February 26, 2025 22:33

kylehickinson added 14 commits February 27, 2025 09:38

[iOS] Prepare CWVWebView web embedder for use in iOS

9b85560

[wip] subclass CWVWebView instead of patches

103a525

remove ssl pinning patches, only block js on web schemes

10714af

remove CRWWKNavigationHandler patch and handle with a subclass

fd32796

improve https upgrade tab helper patch

877e155

DEPS cleanup

dabf32d

move CWVWebView subclass into ios/browser, fix gn/presubmit

3cce559

remove duplicated base value conversion code

4a7c746

remove _stub suffix on filename

e503728

use subclass for WKWebViewConfigurationProvider patch

53323e8

clean up navigation handler util methods

cc6f3d6

add comments for chromium_src overrides & CWVWebView extensions

d876344

remove https upgrade tab helper usage

128c168

expose canTakeSnapshot, adjust comments, cleanup

b3ed162

kylehickinson force-pushed the ios-cwvwebview-core branch from 9b658bf to b3ed162 Compare February 27, 2025 15:28

kylehickinson merged commit 1f2ba13 into master Feb 27, 2025
18 checks passed

kylehickinson deleted the ios-cwvwebview-core branch February 27, 2025 17:01

github-actions bot added this to the 1.78.x - Nightly milestone Feb 27, 2025

aseren mentioned this pull request Apr 23, 2025

[ads] RichNTT: iOS brave/brave-browser#43514

Open

5 tasks

		@@ -0,0 +1,2 @@
		# Common linting false posititives from Obj-C syntax in headers
		filter=-readability/casting,-whitespace/parens,-whitespace/operators

[iOS] Prepare CWVWebView web embedder for use in iOS #26957

[iOS] Prepare CWVWebView web embedder for use in iOS #26957

Conversation

kylehickinson commented Dec 10, 2024 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

github-actions bot commented Jan 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kylehickinson Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bridiver left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 27, 2025

Description

Changes

Security Hotspots

Possible Issues

brave-builds commented Feb 28, 2025

kylehickinson commented Dec 10, 2024 •

edited

Loading

kylehickinson Jan 30, 2025 •

edited

Loading