[iOS] Track failures in Reading Mode web state usage of LoadData. [chromium/src : main]
Olivier Robin (Gerrit)
Olivier Robin added 8 comments![]( "Open in Gerrit")
I am not sure what is the purpose of the CL.
Do you expect to still see the same freeze, and so want to collect data if that happens?
inline constexpr base::TimeDelta kReaderModePageLoadTimeout = base::Seconds(10);this is extremely long for a local page load. Should this be 1 second?
web_state()->LoadData(content_data, @"text/html", std::move(content_url));I think the problem is in content_url.
The WKWebView [1] document this as "A URL that you use to resolve relative URLs within the document."
It probably does not expect the URL to have a fragment, and it is probably what causes the issue (as we don't see the problem for URLs without fragment).
Also, the content we load is the whole page, not the part related to the fragment.
Should we just remove the fragment here?
```
if (!navigation_manager->GetLastCommittedItem()) {
// `LoadData` requires an already committed navigation item.
std::vector<std::unique_ptr<web::NavigationItem>> navigation_items;
navigation_items.push_back(web::NavigationItem::Create());
navigation_manager->Restore(0, std::move(navigation_items));
}
GURL url = content_url;
GURL::Replacements replacements;
replacements.ClearRef();
url = url.ReplaceComponents(replacements);
web_state()->LoadData(content_data, @"text/html", std::move(url));
```
web::PageLoadCompletionStatus load_completion_status) {should we check load_completion_status in the method?
Also, if timer timed out, should we avoid calling the delegate?
// `ReaderModeBrowserAgent` will bypass the call to `LoadContent`.I understand why we need to call stop, but I don't understand this comment.
base::CommandLine::ForCurrentProcess()->AppendSwitch(ScopedCommandLine
data_load_timer_ = std::make_unique<base::ElapsedTimer>();so you have 2 timer testing for the same thing? Can you have only one?
base::UmaHistogramLongTimes100(kReaderModeDataLoadLatencyHistogram,is it not supposed to be less than 10 seconds? Should you use UmaHistogramTimes ?
Related details
- Nohemi Fernandez
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |