New issue
Advanced search Search tips

Issue 913602 link

Starred by 1 user

Issue metadata

Status: Available
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: 2019-03-10
OS: iOS
Pri: 3
Type: Task

Blocking:
issue 725239



Sign in to add a comment

Remove ExternalFileController

Project Member Reported by eugenebut@google.com, Dec 10

Issue description

ExternalFileController is built on top of deprecated CRWNativeContent API.
 
Blocking: 725239
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cfd54e326d7b6d9ba6835f6c29cdd85fb2f270e4

commit cfd54e326d7b6d9ba6835f6c29cdd85fb2f270e4
Author: Eugene But <eugenebut@google.com>
Date: Mon Jan 07 16:52:19 2019

Fix DCHECK in WebStateImpl::GetCurrentURL.

currentURLWithTrustLevel: returns "virtual URL" for native content if
available and "URL" in other cases. Updated DCHECK to compare "virtual
URLs with virtual URLs" and "URLs with URLs".

Old DCHECK started triggering for file:// URLs which have overriden
virtual URLs:
https://chromium-review.googlesource.com/1395563

Bug: 913602
Change-Id: I4d9601f9bab32df960ef48670423b215044d290b
Reviewed-on: https://chromium-review.googlesource.com/c/1394918
Reviewed-by: Danyao Wang <danyao@chromium.org>
Commit-Queue: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620356}
[modify] https://crrev.com/cfd54e326d7b6d9ba6835f6c29cdd85fb2f270e4/ios/web/web_state/web_state_impl.mm

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a95a6954a06c29e860cc1bc36aaa6db20983c2ff

commit a95a6954a06c29e860cc1bc36aaa6db20983c2ff
Author: Eugene But <eugenebut@google.com>
Date: Mon Jan 07 17:02:26 2019

Return false from URLNeedsUserAgentType for file:// URLs.

file:// URLs will be used to load external PDF files, which are
currently loaded with ExternalFileController. This change will disable
"Request Desktop Site" menu item for those file URLs.

Bug: 913602
Change-Id: I48afc14afe0ab499945f0c8d26e78740bfcf3d0d
Reviewed-on: https://chromium-review.googlesource.com/c/1396330
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620360}
[modify] https://crrev.com/a95a6954a06c29e860cc1bc36aaa6db20983c2ff/ios/web/navigation/wk_navigation_util.mm
[modify] https://crrev.com/a95a6954a06c29e860cc1bc36aaa6db20983c2ff/ios/web/navigation/wk_navigation_util_unittest.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66e917008b54ffdbb9f8bde942737e07a0dc2a56

commit 66e917008b54ffdbb9f8bde942737e07a0dc2a56
Author: Eugene But <eugenebut@google.com>
Date: Mon Jan 07 23:38:52 2019

Load external URLs in WebState instead of using ExternalFileController.

This change is made behind kExternalFilesLoadedInWebState flag which is
disabled by default. Without enabling this flag the app should not
change the bahavior.

ExternalFileController uses Web View to load external file. The usage
of ExternalFileController is unnecessary, because WebState is fully
capable to load file URLs. ExternalFileController is built on top of
deprecated CRWNativeContent API and should be removed.

Notable changes:
 - completeURL property was pushed from ChromeAppStartupParameters to AppStartupParameters
 - dismissModalsAndOpenSelectedTabInMode now takes virtualURL in the arguments
 - introduced kExternalFilesLoadedInWebState feature flag
 - file://URL is used instead of chrome:// URL to load external files if
   kExternalFilesLoadedInWebState is on (chrome:// is set as virtual URL
   so user friendly URL is still displayed in the omnibox)
 - UserActivityHandlerTest is parametrized to test kExternalFilesLoadedInWebState on and off
 - Removed UserActivityHandlerNoFixtureTest fixture (its purpose was not clear)
 - ExternalFileController is not created by BVC if kExternalFilesLoadedInWebState is on

Bug: 913602
Change-Id: Id64dcf63705d2da84f11a193c5ac9bbad12a6105
Reviewed-on: https://chromium-review.googlesource.com/c/1395563
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620518}
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/app/application_delegate/mock_tab_opener.h
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/app/application_delegate/mock_tab_opener.mm
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/app/application_delegate/tab_opening.h
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/app/application_delegate/url_opener.mm
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/app/application_delegate/user_activity_handler.mm
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/app/application_delegate/user_activity_handler_unittest.mm
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/app/main_controller.mm
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/app/startup/chrome_app_startup_parameters.h
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/app/startup/chrome_app_startup_parameters.mm
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/browser/about_flags.mm
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/browser/app_startup_parameters.h
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/browser/app_startup_parameters.mm
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/browser/experimental_flags.h
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/browser/experimental_flags.mm
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/browser/ios_chrome_flag_descriptions.cc
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/browser/ios_chrome_flag_descriptions.h
[modify] https://crrev.com/66e917008b54ffdbb9f8bde942737e07a0dc2a56/ios/chrome/browser/ui/browser_view_controller.mm

Owner: eugene...@chromium.org
Status: Started (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5b992fd1cf04812dac28bc0aa471c38fdf5261c

commit b5b992fd1cf04812dac28bc0aa471c38fdf5261c
Author: Eugene But <eugenebut@google.com>
Date: Wed Jan 09 18:31:17 2019

Fix URLs check for slim and legacy navigation manager URLs.

Document URL should always represent URL, instead of "virtual url",
so the check should compare "url" and "url".

Bug: 913602
Change-Id: Iaa63e591d7edd30e6a9b586b89c23bb0085515ba
Reviewed-on: https://chromium-review.googlesource.com/c/1400840
Reviewed-by: Danyao Wang <danyao@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621229}
[modify] https://crrev.com/b5b992fd1cf04812dac28bc0aa471c38fdf5261c/ios/web/web_state/ui/crw_web_controller.mm

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e2d97ddaed7b76f22de1f3f26d7683b52f77d98f

commit e2d97ddaed7b76f22de1f3f26d7683b52f77d98f
Author: Eugene But <eugenebut@google.com>
Date: Thu Jan 10 21:51:01 2019

Allow WebState to load file:// URLs if virtual URL is app-specific.

Chrome for iOS allows loading PDF files from file system which were
received by external apps. Currently the feature is implemented with
ExternalFileController that uses separate WKWebView to load the file.

ExternalFileController is built on top of deprecated NativeContent API
which should be removed.

This CL allows WebState to load file URLs for browser-initiated
navigations that have app-specific virtual URL. This way external files
can be rendered in WebState onstead of using ExternalFileController.

Bug: 913602
Change-Id: I3453e367a8b57482ea01e4e97d24712bb976a8c0
Reviewed-on: https://chromium-review.googlesource.com/c/1401436
Reviewed-by: Ali Juma <ajuma@chromium.org>
Reviewed-by: Danyao Wang <danyao@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621754}
[modify] https://crrev.com/e2d97ddaed7b76f22de1f3f26d7683b52f77d98f/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/e2d97ddaed7b76f22de1f3f26d7683b52f77d98f/ios/web/web_state/web_state_observer_inttest.mm

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4329a94ac9885ccce407f5117bd1ef4c3ba93205

commit 4329a94ac9885ccce407f5117bd1ef4c3ba93205
Author: Eugene But <eugenebut@google.com>
Date: Tue Jan 15 23:56:42 2019

Fix virtual URL piping in openSelectedTabInMode:.

Previously virtual URL was set to pending item after requesting
the load. This change passes virtual URL to load params.

This approach looks cleaner and prevents incorrect URL showing
up in the onmibox.

Bug: 913602
Change-Id: I0d302a68c64915003cffec7811cf09505334d260
Reviewed-on: https://chromium-review.googlesource.com/c/1405430
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622882}
[modify] https://crrev.com/4329a94ac9885ccce407f5117bd1ef4c3ba93205/ios/chrome/app/main_controller.mm
[modify] https://crrev.com/4329a94ac9885ccce407f5117bd1ef4c3ba93205/ios/chrome/browser/ui/main/tab_switcher.h
[modify] https://crrev.com/4329a94ac9885ccce407f5117bd1ef4c3ba93205/ios/chrome/browser/ui/tab_grid/tab_grid_adaptor.mm
[modify] https://crrev.com/4329a94ac9885ccce407f5117bd1ef4c3ba93205/ios/chrome/test/app/tab_test_util.mm

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4329a94ac9885ccce407f5117bd1ef4c3ba93205

commit 4329a94ac9885ccce407f5117bd1ef4c3ba93205
Author: Eugene But <eugenebut@google.com>
Date: Tue Jan 15 23:56:42 2019

Fix virtual URL piping in openSelectedTabInMode:.

Previously virtual URL was set to pending item after requesting
the load. This change passes virtual URL to load params.

This approach looks cleaner and prevents incorrect URL showing
up in the onmibox.

Bug: 913602
Change-Id: I0d302a68c64915003cffec7811cf09505334d260
Reviewed-on: https://chromium-review.googlesource.com/c/1405430
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622882}
[modify] https://crrev.com/4329a94ac9885ccce407f5117bd1ef4c3ba93205/ios/chrome/app/main_controller.mm
[modify] https://crrev.com/4329a94ac9885ccce407f5117bd1ef4c3ba93205/ios/chrome/browser/ui/main/tab_switcher.h
[modify] https://crrev.com/4329a94ac9885ccce407f5117bd1ef4c3ba93205/ios/chrome/browser/ui/tab_grid/tab_grid_adaptor.mm
[modify] https://crrev.com/4329a94ac9885ccce407f5117bd1ef4c3ba93205/ios/chrome/test/app/tab_test_util.mm

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 18 (5 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1bbd3a15ec6bce7ad92bd9bbb9aa91bbf54ceadb

commit 1bbd3a15ec6bce7ad92bd9bbb9aa91bbf54ceadb
Author: Eugene But <eugenebut@chromium.org>
Date: Fri Jan 18 00:04:14 2019

Enable ExternalFilesLoadedInWebState feature flag.

 - Load PDF in Safari
 - Tap Share
 - Tap Copy to Chrome

Bug: 913602
Test: 
Change-Id: Ibf5a891392924b86441c66cb6fbcdc34b5b0631a
Reviewed-on: https://chromium-review.googlesource.com/c/1400301
Reviewed-by: Peter Lee <pkl@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623919}
[modify] https://crrev.com/1bbd3a15ec6bce7ad92bd9bbb9aa91bbf54ceadb/ios/chrome/browser/experimental_flags.mm

Comment 11 by eugene...@chromium.org, Jan 18 (5 days ago)

NextAction: 2019-03-10
Status: Available (was: Started)

Sign in to add a comment