Ahh, I see, the use of chrome.fileSystem.retainEntry was only added recently which is why it wasn't on my list of APIs.
jdeokule, mnilsson: could you guys give me a heads up before any new Chrome Extension API usage in the future? We want to begin fishfooding Linux GVC on AppShell and this API became a blocker when Hotrod started depending on it.
Is there a way to "watch" the hotrodapp code tree in Critique, like the WATCHLISTS files we have in Chrome, so I get CC'd on new CLs?
cl/156596868
Having trouble working around issue 736930.
To recap, the problem is basically that Chrome's GetAssociatedWebContents() looks for ANY browser for the profile. //extensions can't really do that.
When an extension's bg page calls chrome.fileSystem.chooseEntry (which is only possible from component extensions/tests), chooseEntry can show the dialog as long as any tab exists, which it apparently does in the FileManager tests.
Moving chrome.fileSystem.chooseEntry out of //chrome means that chooseEntry from a component extension's bg page fails. IMO this is reasonable... we shouldn't task the extensions system with finding a random tab to anchor a dialog off of. But it breaks the tests in issue 736930.
The dialog itself is sane for requiring a WebContents. That's here: https://cs.chromium.org/chromium/src/chrome/browser/ui/views/select_file_dialog_extension.cc?type=cs&q=selectfiledialogextension+check%5C(web_contents%5C)&l=332
I'm short of ideas, so let's see if the file manager tests can become a platform app...
Devlin/Ben: What if I added a FileSystemDelegate method for this? Instead of calling GetAssociatedWebContents() from file_system_api.cc, we call a delegate method. Chrome's impl can do whatever crazy nonsense that ChromeExtensionFunctionDetails currently does, and AppShell can say "lolwut" and fail for this case.
I actually like this a lot because it speaks to the root of the problem: only Chrome has component extensions, so this situation isn't even possible in AppShell. And only Chrome keeps a list of browser tabs, so this situation wouldn't be readily resolvable in AppShell.
@10 that's pretty similar to what I was going to suggest.
Do you need chooseEntry from AppShell? If not, you could also make it even simpler and just delegate the entire method, and AppShell can throw a "not supported" error. Either way is fine - yours is cleaner if there's a desire to have some chooseEntry()-type functionality now or in the future.
#11: that's tempting, as chrome.fileSystem.chooseEntry and its helper methods take up half the file_system_api.cc file... but it seems hacky to delegate out the entire implementation of just one method out of 15.
Michael - comment 3, there isn't a good way just yet to *just* get alerted when chrome apis are being used. We are proposing to be more systematic about these things and that could open up for that. I'll include you in that thread.
For now you can do this in our tree :-) :
find . -type f | xargs grep -Eho 'chrome\.[a-z0-9_A-Z.]*' | sort | uniq -c
I've added a list to be CC'd on changes, which seems to work okay. Mostly, I wanted to make sure the teams working on frontend stuff were aware of the deprecation of apps APIs and didn't make my life too much more difficult :-)
Comment 1 by michae...@chromium.org
, Jun 5 2017