Make DeclarativeUserScriptManager non-shared between profiles.

Make DeclarativeUserScriptManager non-shared between profiles.

DeclarativeUserScriptManager is now a part of ExtensionSystem::Shared and only
references the regular profile. However, we want the webview content scripts
from WebUI can work with incongnito mode.

We make DeclarativeUserScriptManager becomes a KeyedService. Therefore, each
profile will have its own DeclarativeUserScriptManager object to manage the 
master objects created for its profile.


BUG=461052

[Xi Han, 2015-04-07 18:08:54]

Patchset #1 (id:1) has been deleted

[Xi Han, 2015-04-07 18:13:23]

hanxi@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Xi Han, 2015-04-07 18:13:24]

Hi Devlin: this is a small refactoring to make webview's content scripts work on
WebUI in incognito mode (signin page on ChromeOS), please take a look. Thanks!

[Xi Han, 2015-04-09 21:35:39]

Patchset #3 (id:60001) has been deleted

[Xi Han, 2015-04-09 21:36:01]

Patchset #2 (id:40001) has been deleted

[Xi Han, 2015-04-09 23:06:41]

Hi Delvin,
I adopt another way to make DeclarativeUserScriptManager be a KeyedService and
introduce DeclarativeUserScriptManagerFactory for it. It resolves the memory
leak issue. PTAL, thanks!

[Devlin, 2015-04-20 19:55:58]

On 2015/04/09 23:06:41, Xi Han wrote:
> Hi Delvin,
> I adopt another way to make DeclarativeUserScriptManager be a KeyedService and
> introduce DeclarativeUserScriptManagerFactory for it. It resolves the memory
> leak issue. PTAL, thanks!

Why doesn't it work as-is with incognito mode?  How does this patch help?

[Xi Han, 2015-04-20 19:58:44]

On 2015/04/20 19:55:58, Devlin wrote:
> On 2015/04/09 23:06:41, Xi Han wrote:
> > Hi Delvin,
> > I adopt another way to make DeclarativeUserScriptManager be a KeyedService
and
> > introduce DeclarativeUserScriptManagerFactory for it. It resolves the memory
> > leak issue. PTAL, thanks!
> 
> Why doesn't it work as-is with incognito mode?  How does this patch help?

The inline login of chromeos starts from incognito mode, and once login, it will
create a new profile and switch. However, DeclarativeUserScriptManager is
defined as a shared service, so it always refers to its original profile.

[Devlin, 2015-04-20 20:03:27]

On 2015/04/20 19:58:44, Xi Han wrote:
> On 2015/04/20 19:55:58, Devlin wrote:
> > On 2015/04/09 23:06:41, Xi Han wrote:
> > > Hi Delvin,
> > > I adopt another way to make DeclarativeUserScriptManager be a KeyedService
> and
> > > introduce DeclarativeUserScriptManagerFactory for it. It resolves the
memory
> > > leak issue. PTAL, thanks!
> > 
> > Why doesn't it work as-is with incognito mode?  How does this patch help?
> 
> The inline login of chromeos starts from incognito mode, and once login, it
will
> create a new profile and switch. However, DeclarativeUserScriptManager is
> defined as a shared service, so it always refers to its original profile.

A "shared service" doesn't always refer to the original profile; it's supposed
to service both On- and Off-The-Record profiles.  At what point is it
referencing the original that it should be referencing the incognito?

[xiyuan, 2015-04-20 20:11:31]

On 2015/04/20 20:03:27, Devlin wrote:
> On 2015/04/20 19:58:44, Xi Han wrote:
> > On 2015/04/20 19:55:58, Devlin wrote:
> > > On 2015/04/09 23:06:41, Xi Han wrote:
> > > > Hi Delvin,
> > > > I adopt another way to make DeclarativeUserScriptManager be a
KeyedService
> > and
> > > > introduce DeclarativeUserScriptManagerFactory for it. It resolves the
> memory
> > > > leak issue. PTAL, thanks!
> > > 
> > > Why doesn't it work as-is with incognito mode?  How does this patch help?
> > 
> > The inline login of chromeos starts from incognito mode, and once login, it
> will
> > create a new profile and switch. However, DeclarativeUserScriptManager is
> > defined as a shared service, so it always refers to its original profile.
> 
> A "shared service" doesn't always refer to the original profile; it's supposed
> to service both On- and Off-The-Record profiles.  At what point is it
> referencing the original that it should be referencing the incognito?

In DeclarativeUserScriptManager::CreateDeclarativeUserScriptMaster, the manager
creates a master with  its |browser_context_|, which is always the original
profile. The master uses that to create the loader and loader uses that context
to create URLFetcher to get webui files.

In ChromeOS login case, the login webui runs in incognito profile and URL
Fetcher would fail to load the webui files because original profile does not
know about the webui.

[Devlin, 2015-04-20 20:15:04]

On 2015/04/20 20:11:31, xiyuan wrote:
> On 2015/04/20 20:03:27, Devlin wrote:
> > On 2015/04/20 19:58:44, Xi Han wrote:
> > > On 2015/04/20 19:55:58, Devlin wrote:
> > > > On 2015/04/09 23:06:41, Xi Han wrote:
> > > > > Hi Delvin,
> > > > > I adopt another way to make DeclarativeUserScriptManager be a
> KeyedService
> > > and
> > > > > introduce DeclarativeUserScriptManagerFactory for it. It resolves the
> > memory
> > > > > leak issue. PTAL, thanks!
> > > > 
> > > > Why doesn't it work as-is with incognito mode?  How does this patch
help?
> > > 
> > > The inline login of chromeos starts from incognito mode, and once login,
it
> > will
> > > create a new profile and switch. However, DeclarativeUserScriptManager is
> > > defined as a shared service, so it always refers to its original profile.
> > 
> > A "shared service" doesn't always refer to the original profile; it's
supposed
> > to service both On- and Off-The-Record profiles.  At what point is it
> > referencing the original that it should be referencing the incognito?
> 
> In DeclarativeUserScriptManager::CreateDeclarativeUserScriptMaster, the
manager
> creates a master with  its |browser_context_|, which is always the original
> profile. The master uses that to create the loader and loader uses that
context
> to create URLFetcher to get webui files.
> 
> In ChromeOS login case, the login webui runs in incognito profile and URL
> Fetcher would fail to load the webui files because original profile does not
> know about the webui.

Wait, so this is based on the other CL that introduces fetching the WebUI files?
 That clears up a lot.  I'm still inclined to say it doesn't make sense to have
different DeclarativeUserScriptMasters for regular/incognito, though - most
commonly, you'll want them to share the data.  Would it be appropriate to
instead put functionality for OTR profiles into the WebUI loader, since that's
the only place that will need it?

[Fady Samuel, 2015-08-14 14:57:21]

fsamuel@chromium.org changed reviewers:
+ fsamuel@chromium.org

[Fady Samuel, 2015-08-14 14:57:21]

Can we close this issue?

[Xi Han, 2015-08-14 14:59:46]

On 2015/08/14 14:57:21, Fady Samuel wrote:
> Can we close this issue?

This patch is abandoned. Close it now.