-
Notifications
You must be signed in to change notification settings - Fork 893
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[iOS] Remove tab storage on all tab content scripts #27162
Conversation
@@ -227,10 +227,6 @@ public class BrowserViewController: UIViewController { | |||
// allow us to re-trigger the `URLRequest` if the user requests a file to be downloaded. | |||
var pendingRequests = [String: URLRequest]() | |||
|
|||
// This is set when the user taps "Download Link" from the context menu. We then force a | |||
// download of the next request through the `WKNavigationDelegate` that matches this web view. | |||
weak var pendingDownloadWebView: WKWebView? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was never set so I removed it
...erContent/UserScripts/Scripts_Dynamic/ScriptHandlers/Sandboxed/ReaderModeScriptHandler.swift
Show resolved
Hide resolved
a6857a4
to
e74ed87
Compare
e74ed87
to
d1b8724
Compare
[puLL-Merge] - brave/brave-core@27162 DescriptionThis PR significantly refactors the TabContentScript protocol and its implementations. The main changes involve removing the Possible Issues
Security HotspotsNo significant security issues are introduced by this change. However, careful testing should be done to ensure that all script handlers are correctly handling the new method signature, especially in security-sensitive areas like the wallet and authentication handlers. ChangesChanges
sequenceDiagram
participant BVC as BrowserViewController
participant Tab
participant TSM as TabScriptManager
participant SH as ScriptHandler
BVC->>Tab: Creates Tab
Tab->>TSM: Creates TabScriptManager
BVC->>Tab: Adds ScriptHandlers
Tab->>TSM: Registers ScriptHandlers
Note over Tab,TSM: ScriptHandlers no longer hold reference to Tab
WKWebView->>TSM: Receives script message
TSM->>SH: Calls tab(_:receivedScriptMessage:replyHandler:)
SH->>Tab: Interacts with Tab as needed
SH->>TSM: Returns result
TSM->>WKWebView: Sends reply
|
This change prepares for using the Chromium web embedder by removing weak storage of `Tab` objects passed in when creating classes confomring to `TabContentScript` which better matches `JavaScriptFeature` and its requirements
d1b8724
to
8690a3c
Compare
This change prepares for using the Chromium web embedder by removing weak storage of
Tab
objects passed in when creating classes confomring toTabContentScript
which better matchesJavaScriptFeature
and its requirements.This doesn't change how any underlying JavaScript functions or logic of the message handlers
Resolves brave/brave-browser#43184
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: