Make WebUI webview content script work with incognito.

Make WebUI webview content script work with incognito.

**** FYI only, not intended for checkin ****

- Add GetDeclarativeUserScriptMasterByID that takes a BrowserContext
  to built DeclarativeUserScriptMaster; This is needed because
  DeclarativeUserScriptManager is part of ExtensionSystem::Shared and
  only references the regular profile;
- Allow content script to run in incognito if the addContentScript
  is called within an incognito profile;

BUG=461052

[xiyuan, 2015-04-03 21:39:59]

xiyuan@chromium.org changed reviewers:
+ hanxi@chromium.org

[xiyuan, 2015-04-03 21:40:00]

This is on top of your https://codereview.chromium.org/1056533002/ to make the
api work when called from webui running in an incognito profile (ChromeOS login
runs in such case).

I am not sure the change in declarative_user_script_manager.cc is proper or not
since I know nothing about the code. But we need to pass correct BrowserContext
down to WebUIUserScriptLoader.Otherwise URLFetcher won't be able to load data.

The other part is we need also to set_incognito_enabled flag. Otherwise, script
won't run.

One final thing, it appeas that with your patch, extension's content script is
injected into webview as well. Is this intentional?

Hope this does not ruin your day after the holiday. :)

[Xi Han, 2015-04-03 23:06:05]

Thank you for creating this patch to fix the incognito issue. I think we may
need to create different DeclarativeUserScriptManager with different profile,
rather than master. The manager should be responsible for managing a set of
masters with the same profile. I am thinking whether we can hang the
DeclarativeUserScriptManager to BrowserContext, rather than ExtensionSystem. 

I don't have any clear clue why the extension's content scripts will be injected
to <webview>, but they are not supposed to. I need to look deep into the code.

[xiyuan, 2015-04-03 23:13:03]

On 2015/04/03 23:06:05, Xi Han wrote:
> Thank you for creating this patch to fix the incognito issue. I think we may
> need to create different DeclarativeUserScriptManager with different profile,
> rather than master. The manager should be responsible for managing a set of
> masters with the same profile. I am thinking whether we can hang the
> DeclarativeUserScriptManager to BrowserContext, rather than ExtensionSystem. 
> 

In that case, we probably can just move DeclarativeUserScriptManager it out of
ExtensionSystemImpl::Shared and put it in ExtensionSystemImpl to make it
per-profile. Per doc extension_system_impl.h:

// ExtensionSystemImpl, a KeyedService with separate incognito
// instances. A private Shared class (also a KeyedService,
// but with a shared instance for incognito) keeps the common services.
 
> I don't have any clear clue why the extension's content scripts will be
injected
> to <webview>, but they are not supposed to. I need to look deep into the code.

It is probably because we remove the process->IsIsolatedGuest() check in
UserScriptLoader::SendUpdate. But I am not what is the proper way to fix it. :p

Hey, Both you and Fady should not be reading emails during holiday. :)

[Xi Han, 2015-04-06 03:08:31]

On 2015/04/03 23:13:03, xiyuan wrote:
> On 2015/04/03 23:06:05, Xi Han wrote:
> > Thank you for creating this patch to fix the incognito issue. I think we may
> > need to create different DeclarativeUserScriptManager with different
profile,
> > rather than master. The manager should be responsible for managing a set of
> > masters with the same profile. I am thinking whether we can hang the
> > DeclarativeUserScriptManager to BrowserContext, rather than ExtensionSystem.

> > 
> 
> In that case, we probably can just move DeclarativeUserScriptManager it out of
> ExtensionSystemImpl::Shared and put it in ExtensionSystemImpl to make it
> per-profile. Per doc extension_system_impl.h:
> 
> // ExtensionSystemImpl, a KeyedService with separate incognito
> // instances. A private Shared class (also a KeyedService,
> // but with a shared instance for incognito) keeps the common services.
>  
> > I don't have any clear clue why the extension's content scripts will be
> injected
> > to <webview>, but they are not supposed to. I need to look deep into the
code.
> 
> It is probably because we remove the process->IsIsolatedGuest() check in
> UserScriptLoader::SendUpdate. But I am not what is the proper way to fix it.
:p
> 
> Hey, Both you and Fady should not be reading emails during holiday. :)


1) It is a good idea just move DeclarativeUserScriptManager out of
ExtensionSystemImpl::Shared,
 I think it will solve the problem:)
2) To avoid injecting extensions scripts, can you try in
UserScriptLoader::SendUpdate() to add the
 following check back and change it to:

// Don't allow injection of content scripts into <webview>.
  if (process->IsIsolatedGuest() && host_id().id().empty())
    return;

Hopefully that can help.

[xiyuan, 2015-04-06 18:05:44]

On 2015/04/06 03:08:31, Xi Han wrote:
> 1) It is a good idea just move DeclarativeUserScriptManager out of
> ExtensionSystemImpl::Shared,
>  I think it will solve the problem:)

That works. But I am not so sure whether it is the right thing to do. Would you
work with extension team to see whether this is okay?

> 2) To avoid injecting extensions scripts, can you try in
> UserScriptLoader::SendUpdate() to add the
>  following check back and change it to:
> 
> // Don't allow injection of content scripts into <webview>.
>   if (process->IsIsolatedGuest() && host_id().id().empty())
>     return;
> 
> Hopefully that can help.

Yep, that fixed the problem.

[chromium-reviews, 2015-04-06 18:20:19]

I will add these code in my review tomorrow and let a extension's owner to
review it:)
On Apr 6, 2015 2:05 PM, <xiyuan@chromium.org> wrote:

> On 2015/04/06 03:08:31, Xi Han wrote:
>
>> 1) It is a good idea just move DeclarativeUserScriptManager out of
>> ExtensionSystemImpl::Shared,
>>   I think it will solve the problem:)
>>
>
> That works. But I am not so sure whether it is the right thing to do.
> Would you
> work with extension team to see whether this is okay?
>
>  2) To avoid injecting extensions scripts, can you try in
>> UserScriptLoader::SendUpdate() to add the
>>   following check back and change it to:
>>
>
>  // Don't allow injection of content scripts into <webview>.
>>    if (process->IsIsolatedGuest() && host_id().id().empty())
>>      return;
>>
>
>  Hopefully that can help.
>>
>
> Yep, that fixed the problem.
>
>
>
> https://codereview.chromium.org/1056133005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Xi Han, 2015-04-07 18:19:05]

I split the fix of the 2 problems that you found in two patches:
1) DeclarativeUserScriptManager in https://codereview.chromium.org/1066993002/.

2) The issue of avoiding injecting extensions' scripts to webview in my previous
patch https://codereview.chromium.org/959413003.

The new created issue 1066993002 is an independent patch. I will rebase issue
959413003 to make you can apply them together.