Added import of links and text content to ipfs #8422

spylogsster · 2021-04-02T16:36:02Z

Resolves brave/brave-browser#14769
Resolves brave/brave-browser#15176

Added submenu to render context menu
Added base import worker which implements importing flow for ipfs: add/mkdir/cp apis should be called sequentially
Added http{s} links importer which downloads a link content to file and uploads it to ipfs
Added text importer that puts to ipfs a file with passed text, an index for file calculated based on content hash, same text will have same index.
Launch ipfs daemon if import called and daemon is turned off
Open of ipfs webui page with imported folder
Copy to clipboard of shareable link for uploaded content
Import calls are proxied through tab helper to have access to chrome/ for opening new tabs when import is finished
Pre warm request for shareable link
Added notifications, if import successful it opens shareable link, otherwise brave://ipfs

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
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, npm run lint, 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:

Import different objects on pages as described in ticket
for each imported object should be opened web page from local node webui in case of success with brave import directory
for each imported object should be copied shareable link to clipboard which allows to access imported content via public gateway from any other computer
please pay attention that if you import different selected texts from same page they should be imported all to different text files with indexes
if you import same selected text it should not add new text file to import folder
Make sure imported content (link or text) is same as original, try to download file by shareable link it should be same without additional bytes
If import successful by click on notification should be opened shareable link, otherwise brave://ipfs-internal

chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc

app/brave_generated_resources.grd

components/ipfs/ipfs_text_import_worker.h

components/ipfs/ipfs_utils.cc

chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc

components/ipfs/ipfs_link_import_worker.h

components/ipfs/ipfs_text_import_worker.cc

components/ipfs/ipfs_link_import_worker.cc

components/ipfs/ipfs_import_worker_base.cc

chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc

components/speedreader/rust/ffi/Cargo.lock

components/ipfs/ipfs_text_import_worker.h

components/ipfs/ipfs_link_import_worker.h

components/ipfs/ipfs_text_import_worker.h

components/ipfs/ipfs_import_worker_base.h

components/ipfs/ipfs_link_import_worker.cc

yrliou · 2021-04-14T22:12:47Z

@spylogsster Please don't force-push and replace the commit directly during review, I can't see the interdiff using github.
Please push new commits when addressing comments so I could see what has been changed from the previous review round easily, thanks.

yrliou

LGTM

yrliou · 2021-04-15T20:14:36Z

components/ipfs/ipfs_utils.cc

+}
+
+bool IsIpfsMenuEnabled(content::BrowserContext* browser_context) {
+  return ipfs::IsIpfsEnabled(browser_context) &&


nit: You can just use IsIpfsEnabled(browser_context) && IsLocalGatewayConfigured(browser_context) here.

iefremov · 2021-05-11T09:19:56Z

components/ipfs/ipfs_import_worker_base.cc

+  base::PostTaskAndReplyWithResult(
+      io_task_runner_.get(), FROM_HERE,
+      base::BindOnce(&IpfsImportWorkerBase::CreateResourceRequest,
+                     base::Unretained(this), std::move(blob_builder_callback),


Passing Unretained pointers like this is not safe. Fortunately, this function doesn't really need this, so I'd suggest we make it a free function instead of a class method

iefremov · 2021-05-11T09:21:11Z

components/ipfs/ipfs_import_worker_base.cc

+  auto blob_storage_context_getter =
+      content::BrowserContext::GetBlobStorageContext(browser_context_);
+  base::PostTaskAndReplyWithResult(
+      io_task_runner_.get(), FROM_HERE,


you can simply do io_task_runner_->PostTaskAndReply...

iefremov · 2021-05-11T09:22:05Z

components/ipfs/ipfs_import_worker_base.cc

+  url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      url_loader_factory_.get(),
+      base::BindOnce(&IpfsImportWorkerBase::OnImportAddComplete,
+                     weak_factory_.GetWeakPtr()));


in this case it is safe to use Unretained

iefremov · 2021-05-11T09:26:56Z

components/ipfs/ipfs_import_worker_base.cc

+      io_task_runner_(base::CreateSequencedTaskRunner(
+          {base::MayBlock(), content::BrowserThread::IO,
+           base::TaskPriority::BEST_EFFORT,
+           base::TaskShutdownBehavior::BLOCK_SHUTDOWN})),


not sure it really should block the shutdown

iefremov · 2021-05-11T10:02:34Z

components/ipfs/ipfs_service.cc

+  auto import_completed_callback =
+      base::BindOnce(&IpfsService::OnImportFinished, weak_factory_.GetWeakPtr(),
+                     std::move(callback), key);
+  importers_[key] = std::make_unique<IpfsTextImportWorker>(


if IpfsTextImportWorker::StartImportText early returns in case text or host is empty, it will synchronously call the callback, that won't erase the failed importer from the map (because it is not inserted yet), correct? After that it will be inserted and will live in the map forever. So maybe we'd better avoid running complex logic that involves chains of callbacks in constructors

On iOS 17 rotating the device with a full screen modal presented (e.g. Playlist, Tab Tray) to landscape then back to portrait does not trigger `traitCollectionDidChange`/`willTransition`/etc calls and so the toolbar remains in the wrong state.

spylogsster requested a review from a team as a code owner April 2, 2021 16:36

spylogsster self-assigned this Apr 2, 2021

spylogsster force-pushed the import-to-ipfs branch 4 times, most recently from b48bbef to 15b1010 Compare April 2, 2021 17:58

spylogsster requested a review from bbondy April 2, 2021 18:06

spylogsster force-pushed the import-to-ipfs branch from 15b1010 to cf1e851 Compare April 2, 2021 18:38

mkarolin reviewed Apr 2, 2021

View reviewed changes

chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc Outdated Show resolved Hide resolved

mkarolin reviewed Apr 2, 2021

View reviewed changes

chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc Outdated Show resolved Hide resolved

mkarolin reviewed Apr 2, 2021

View reviewed changes

chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc Show resolved Hide resolved

mkarolin reviewed Apr 2, 2021

View reviewed changes

chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc Outdated Show resolved Hide resolved

mkarolin reviewed Apr 2, 2021

View reviewed changes

app/brave_generated_resources.grd Outdated Show resolved Hide resolved

spylogsster force-pushed the import-to-ipfs branch 10 times, most recently from d9016a4 to 4b10ce9 Compare April 6, 2021 12:10

spylogsster requested a review from bridiver as a code owner April 6, 2021 12:10

spylogsster force-pushed the import-to-ipfs branch 6 times, most recently from 87d09d8 to 78f5840 Compare April 7, 2021 12:33

spylogsster force-pushed the import-to-ipfs branch 4 times, most recently from 80f1769 to e6c64d0 Compare April 8, 2021 21:31

yrliou reviewed Apr 12, 2021

View reviewed changes

spylogsster force-pushed the import-to-ipfs branch 5 times, most recently from ad4ea71 to 087cb33 Compare April 13, 2021 12:52

Added import of links and text content to ipfs

1e8418b

spylogsster force-pushed the import-to-ipfs branch from 087cb33 to 1e8418b Compare April 13, 2021 16:45

yrliou reviewed Apr 14, 2021

View reviewed changes

spylogsster force-pushed the import-to-ipfs branch 3 times, most recently from 8afe5d7 to e9e28e3 Compare April 15, 2021 11:03

review fixes

beee648

spylogsster force-pushed the import-to-ipfs branch from e9e28e3 to beee648 Compare April 15, 2021 11:59

yrliou approved these changes Apr 15, 2021

View reviewed changes

bbondy merged commit 9303c55 into master Apr 16, 2021

bbondy deleted the import-to-ipfs branch April 16, 2021 14:36

bbondy added this to the 1.25.x - Nightly milestone Apr 16, 2021

stephendonner mentioned this pull request Apr 20, 2021

Add import to IPFS context menu which pins content brave/brave-browser#14769

Closed

iefremov reviewed May 11, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added import of links and text content to ipfs #8422

Added import of links and text content to ipfs #8422

spylogsster commented Apr 2, 2021 •

edited

Loading

yrliou commented Apr 14, 2021

yrliou left a comment

yrliou Apr 15, 2021

iefremov May 11, 2021

iefremov May 11, 2021

iefremov May 11, 2021

iefremov May 11, 2021

iefremov May 11, 2021

Added import of links and text content to ipfs #8422

Added import of links and text content to ipfs #8422

Conversation

spylogsster commented Apr 2, 2021 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

yrliou commented Apr 14, 2021

yrliou left a comment

Choose a reason for hiding this comment

yrliou Apr 15, 2021

Choose a reason for hiding this comment

iefremov May 11, 2021

Choose a reason for hiding this comment

iefremov May 11, 2021

Choose a reason for hiding this comment

iefremov May 11, 2021

Choose a reason for hiding this comment

iefremov May 11, 2021

Choose a reason for hiding this comment

iefremov May 11, 2021

Choose a reason for hiding this comment

spylogsster commented Apr 2, 2021 •

edited

Loading