Disable IPFS WebUI on Android #17999

AlexeyBarabash · 2023-04-11T19:31:55Z

Resolves brave/brave-browser#29697
Resolves brave/brave-browser#25103

This PR removes resources and some native code for ipfs-internals web ui page on Android. It allows to save only 162KB for now. This result may be improved with fixing of brave/brave-browser#28060 .

Submitter Checklist:

I confirm that no security/privacy review is needed, or that I have requested one
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 lint, 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:

Test Plan:

Make fresh installation
Open ipfs://bafybeies3odi24wyk3e22rnautr57tiuk3b56nxrd53fxgtvr37abmz5j4/66.png link
Ensure you can still see this page
Press Use a public gateway
Ensure you can see the image
Navigate to about:chrome-urls
Ensure there is no brave://ipfs-internals

AlexeyBarabash · 2023-04-13T17:08:07Z

components/resources/ipfs_strings.grdp

-    <message name="IDS_IPFS_ADDRESSES_CONFIG_TITLE" desc="Title of addresses config section">Addresses</message>
-    <message name="IDS_IPFS_REPO_STATS_TITLE" desc="Title of repo stats section">Repo Stats</message>
-    <message name="IDS_IPFS_NODE_INFO_TITLE" desc="Title of node info section">Node info</message>
+    <if expr="enable_ipfs_internals_webui">


Below are the strings from this section:
https://github.com/brave/brave-core/blob/master/browser/ui/webui/brave_webui_source.cc#L713

AlexeyBarabash · 2023-04-13T17:12:27Z

browser/ipfs/import/ipfs_import_controller.cc

@@ -294,10 +294,12 @@ void IpfsImportController::OnWebPageImportCompleted(

 void IpfsImportController::OnImportCompleted(const ipfs::ImportedData& data) {
  auto link = CreateAndCopyShareableLink(data);
+#if BUILDFLAG(ENABLE_IPFS_INTERNALS_WEBUI)


This may look strange, but on desktop this flag is true, and on Android whole IpfsImportController class seems to be unused, but compiled. It looks it will be fixed with brave/brave-browser#28060.

My attempt to make it failed - https://github.com/brave/brave-core/compare/android_no_ipfs_webui_BAK01?expand=1#diff-87dbeae647f32aa0670c977a84d99be9862a63bf9cc1f0a8b191da86f1d1a21fR11 - with such change I could not open ipfs:// pages at all.

AlexeyBarabash · 2023-04-13T18:13:00Z

Converted to draft as the unit test has failed on Android and error is related to the PR

AlexeyBarabash · 2023-04-14T11:41:12Z

CI fails with some tests on Windows-x86/Windows-x64, which are not related to the PR. I am going to open the PR again for the review.

simonhong

++

components/ipfs/buildflags/buildflags.gni

cypt4 · 2023-04-17T10:39:39Z

Is disabling enable_ipfs_local_node on android is much harder to implement than using separate build flag?

AlexeyBarabash · 2023-04-17T13:50:22Z

@cypt4

Is disabling enable_ipfs_local_node on android is much harder to implement than using separate build flag?

At this PR I tried to remove the obviously unused or useless data from APK.

I mentioned above that direct disabling enable_ipfs_local_node build flags makes IPFS unusable on Android, it seems that the enable_ipfs_local_node flag disables some required classes when using external gateway. Looks like this is know issue: brave/brave-browser#28060.

I don't know which classes should be under enable_ipfs_local_node and which should not. I can't say is it much harder or not.

In anyway, when brave/brave-browser#28060 will be fixed, current PR will still be useful, it will allow to exclude ipfs-internals page resources and handlers depending on the enable_ipfs_local_node flag, just by renaming ENABLE_IPFS_INTERNALS_WEBUI to ENABLE_IPFS_LOCAL_NODE and removing enable_ipfs_internals_webui flag.

cypt4

LGTM

components/ipfs_ui/BUILD.gn

goodov

chromium_src, DEPS lgtm

chromium_src/chrome/common/webui_url_constants.cc

components/ipfs_ui/BUILD.gn

AlexeyBarabash · 2023-04-19T14:11:02Z

Squashed and rebased

mkarolin

strings ++

AlexeyBarabash · 2023-04-19T16:21:29Z

Rebased and resolved conflict

Fixes brave/brave-browser#29697. Fixes brave/brave-browser#25103. Excluded iOS from enable_ipfs_internals_webui flag (codereview notice) Assert to ensure ipfs-internals web UI is not built on iOS (codereview notice) Removed unnecessary #if #endif (codereview notice)

AlexeyBarabash · 2023-04-20T10:57:58Z

Rebased again.

AlexeyBarabash added the CI/skip Do not run CI builds (except noplatform) label Apr 11, 2023

github-actions bot assigned AlexeyBarabash Apr 11, 2023

github-actions bot added the potential-layer-violation-fixes This PR touches a BUILD.gn file with check_includes=false label Apr 11, 2023

AlexeyBarabash force-pushed the android_no_ipfs_webui branch from c8892b1 to d3e3741 Compare April 13, 2023 15:46

AlexeyBarabash removed the CI/skip Do not run CI builds (except noplatform) label Apr 13, 2023

AlexeyBarabash force-pushed the android_no_ipfs_webui branch from d3e3741 to 94c3d2a Compare April 13, 2023 16:17

AlexeyBarabash commented Apr 13, 2023

View reviewed changes

AlexeyBarabash marked this pull request as ready for review April 13, 2023 17:14

AlexeyBarabash requested review from a team as code owners April 13, 2023 17:14

AlexeyBarabash requested review from simonhong, atuchin-m and cypt4 April 13, 2023 17:15

AlexeyBarabash marked this pull request as draft April 13, 2023 18:11

AlexeyBarabash force-pushed the android_no_ipfs_webui branch 2 times, most recently from 2d7a9f2 to 3f4b048 Compare April 14, 2023 07:13

AlexeyBarabash marked this pull request as ready for review April 14, 2023 11:41

simonhong approved these changes Apr 17, 2023

View reviewed changes

components/ipfs/buildflags/buildflags.gni Outdated Show resolved Hide resolved

cypt4 approved these changes Apr 17, 2023

View reviewed changes

components/ipfs_ui/BUILD.gn Show resolved Hide resolved

goodov approved these changes Apr 18, 2023

View reviewed changes

chromium_src/chrome/common/webui_url_constants.cc Outdated Show resolved Hide resolved

components/ipfs_ui/BUILD.gn Show resolved Hide resolved

AlexeyBarabash force-pushed the android_no_ipfs_webui branch from 21aefb7 to 6d241f9 Compare April 19, 2023 14:10

mkarolin approved these changes Apr 19, 2023

View reviewed changes

AlexeyBarabash force-pushed the android_no_ipfs_webui branch from 6d241f9 to b3cd5ce Compare April 19, 2023 16:20

AlexeyBarabash force-pushed the android_no_ipfs_webui branch from b3cd5ce to 944883e Compare April 20, 2023 10:56

AlexeyBarabash merged commit b91a203 into master Apr 20, 2023

AlexeyBarabash deleted the android_no_ipfs_webui branch April 20, 2023 14:52

github-actions bot added this to the 1.52.x - Nightly milestone Apr 20, 2023

This was referenced Apr 20, 2023

Install and start button shouldn't be available on brave://ipfs-internals for Android brave/brave-browser#25103

Closed

enable_ipfs_local_node should be disabled for android brave/brave-browser#28060

Open

srirambv mentioned this pull request May 11, 2023

Disable ipfs-internals page on Android brave/brave-browser#29697

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable IPFS WebUI on Android #17999

Disable IPFS WebUI on Android #17999

AlexeyBarabash commented Apr 11, 2023 •

edited by kjozwiak

Loading

AlexeyBarabash Apr 13, 2023

AlexeyBarabash Apr 13, 2023

AlexeyBarabash commented Apr 13, 2023

AlexeyBarabash commented Apr 14, 2023

simonhong left a comment

cypt4 commented Apr 17, 2023

AlexeyBarabash commented Apr 17, 2023

cypt4 left a comment

goodov left a comment

AlexeyBarabash commented Apr 19, 2023

mkarolin left a comment

AlexeyBarabash commented Apr 19, 2023

AlexeyBarabash commented Apr 20, 2023

Disable IPFS WebUI on Android #17999

Disable IPFS WebUI on Android #17999

Conversation

AlexeyBarabash commented Apr 11, 2023 • edited by kjozwiak Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

AlexeyBarabash Apr 13, 2023

Choose a reason for hiding this comment

AlexeyBarabash Apr 13, 2023

Choose a reason for hiding this comment

AlexeyBarabash commented Apr 13, 2023

AlexeyBarabash commented Apr 14, 2023

simonhong left a comment

Choose a reason for hiding this comment

cypt4 commented Apr 17, 2023

AlexeyBarabash commented Apr 17, 2023

cypt4 left a comment

Choose a reason for hiding this comment

goodov left a comment

Choose a reason for hiding this comment

AlexeyBarabash commented Apr 19, 2023

mkarolin left a comment

Choose a reason for hiding this comment

AlexeyBarabash commented Apr 19, 2023

AlexeyBarabash commented Apr 20, 2023

AlexeyBarabash commented Apr 11, 2023 •

edited by kjozwiak

Loading