DevTools: show extension name in console context switcher

DevTools: show extension name in console context switcher

The patch does the following:

- Add DOMWrapperWorld.isolatedWorldHumanReadableName, which stores
the name of extension.

- In devtools remote debugging protocol, the executionContext.name
in fact stores execution context origin. The patch sets
"executionContextDecriptor.name" to store the isolated context
human-readable name, and adds "executionContextDescriptor.origin"

BUG=160977
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181086

[lushnikov, 2013-05-23 15:35:20]

Please have a look

[caseq, 2013-05-23 17:39:38]

Alternatively, we could provide a InspectorClient method to map extension's
security origin to a human-readable extension name. Given that we already have
security origin, carrying human readable name used just for devtools seems a bit
excessive.

[yurys, 2013-05-24 04:58:44]

https://codereview.chromium.org/15764004/diff/1/Source/bindings/v8/DOMWrapper...
File Source/bindings/v8/DOMWrapperWorld.cpp (right):

https://codereview.chromium.org/15764004/diff/1/Source/bindings/v8/DOMWrapper...
Source/bindings/v8/DOMWrapperWorld.cpp:210:
IsolatedWorldHumanReadableNameMap::iterator it =
humanReadableNames.find(worldId());
Hm, why does DOMWrapperWorld use several static storages to map world id to one
of the worlds properties instead of having single maps world id ->
DOMWrapperWorld and storing all properties on the object? In this particular
case human readable name looks like a natural field on DOMWrapperWorld.

https://codereview.chromium.org/15764004/diff/1/Source/core/inspector/PageRun...
File Source/core/inspector/PageRuntimeAgent.cpp (right):

https://codereview.chromium.org/15764004/diff/1/Source/core/inspector/PageRun...
Source/core/inspector/PageRuntimeAgent.cpp:184: v8::HandleScope handleScope;
We try to avoid using v8 API from the agents directly. There are Script*
abstractions for accessing underlying VM.

[lushnikov, 2013-05-29 13:11:54]

removed v8 api usages in agent

[pfeldman, 2013-08-05 16:44:48]

https://codereview.chromium.org/15764004/diff/7001/Source/bindings/v8/DOMWrap...
File Source/bindings/v8/DOMWrapperWorld.cpp (right):

https://codereview.chromium.org/15764004/diff/7001/Source/bindings/v8/DOMWrap...
Source/bindings/v8/DOMWrapperWorld.cpp:176:
DEFINE_STATIC_LOCAL(IsolatedWorldHumanReadableNameMap, map, ());
You should not create yet another map, we can extend existing one to store more
bits associated with the world. It is also worth checking if the lazy binding is
already fixed so that you could simply make it a field of the world. See
https://bugs.webkit.org/show_bug.cgi?id=97398 and talk to Dan.

[lushnikov, 2014-08-23 20:37:46]

I'm bringing this patch back to life. Please, take a look!

https://codereview.chromium.org/15764004/diff/7001/Source/bindings/v8/DOMWrap...
File Source/bindings/v8/DOMWrapperWorld.cpp (right):

https://codereview.chromium.org/15764004/diff/7001/Source/bindings/v8/DOMWrap...
Source/bindings/v8/DOMWrapperWorld.cpp:176:
DEFINE_STATIC_LOCAL(IsolatedWorldHumanReadableNameMap, map, ());
On 2013/08/05 16:44:49, pfeldman wrote:
> You should not create yet another map, we can extend existing one to store
more
> bits associated with the world. It is also worth checking if the lazy binding
is
> already fixed so that you could simply make it a field of the world. See
> https://bugs.webkit.org/show_bug.cgi?id=97398 and talk to Dan.

So I've got to create this map due to the fact that the world is reported in
inspector instrumentation at the moment of its creation, deeper in stack. That
means that the humanReadableName should be preset before the actual world
creation.

The same thing actually happens with other two maps here.

[lushnikov, 2014-08-23 20:48:37]

This is a blink-side of the patch.
The chromium side: https://codereview.chromium.org/500823002

[lushnikov, 2014-08-23 20:52:11]

The patch doesn't solve the content scripts domain names in sources panel.
But with this patch in place, we'll have a map chrome-extension-url <==>
chrome-extension-name, so it could be easily fixed in a follow-up.

[yurys, 2014-08-25 09:22:30]

lgtm

https://codereview.chromium.org/15764004/diff/17001/Source/bindings/core/v8/D...
File Source/bindings/core/v8/DOMWrapperWorld.cpp (right):

https://codereview.chromium.org/15764004/diff/17001/Source/bindings/core/v8/D...
Source/bindings/core/v8/DOMWrapperWorld.cpp:187:
IsolatedWorldHumanReadableNameMap::iterator it =
humanReadableNames.find(worldId());
humanReadableNames.get(worldId()) ?

https://codereview.chromium.org/15764004/diff/17001/Source/bindings/core/v8/D...
Source/bindings/core/v8/DOMWrapperWorld.cpp:194: if
(!humanReadableName.isEmpty())
Why do you need this check?

https://codereview.chromium.org/15764004/diff/17001/Source/devtools/front_end...
File Source/devtools/front_end/sdk/RuntimeModel.js (right):

https://codereview.chromium.org/15764004/diff/17001/Source/devtools/front_end...
Source/devtools/front_end/sdk/RuntimeModel.js:178: this.origin = isPageContext
&& !origin ? "<page context>" : origin;
I guess it should be assigned into .name ?

[lushnikov, 2014-08-26 09:57:46]

lushnikov@chromium.org changed reviewers:
+ sergeyv@chromium.org

[lushnikov, 2014-08-27 08:18:46]

lushnikov@chromium.org changed reviewers:
+ dglazkov@chromium.org

[lushnikov, 2014-08-27 08:19:14]

On 2014/08/27 08:18:46, lushnikov wrote:
> mailto:lushnikov@chromium.org changed reviewers:
> + mailto:dglazkov@chromium.org

@dglazkov: could you please review changes in Source/web & public/web?

[lushnikov, 2014-08-27 08:20:26]

https://codereview.chromium.org/15764004/diff/17001/Source/bindings/core/v8/D...
File Source/bindings/core/v8/DOMWrapperWorld.cpp (right):

https://codereview.chromium.org/15764004/diff/17001/Source/bindings/core/v8/D...
Source/bindings/core/v8/DOMWrapperWorld.cpp:187:
IsolatedWorldHumanReadableNameMap::iterator it =
humanReadableNames.find(worldId());
On 2014/08/25 09:22:30, yurys wrote:
> humanReadableNames.get(worldId()) ?

Done.

https://codereview.chromium.org/15764004/diff/17001/Source/bindings/core/v8/D...
Source/bindings/core/v8/DOMWrapperWorld.cpp:194: if
(!humanReadableName.isEmpty())
On 2014/08/25 09:22:30, yurys wrote:
> Why do you need this check?

Done.

https://codereview.chromium.org/15764004/diff/17001/Source/devtools/front_end...
File Source/devtools/front_end/sdk/RuntimeModel.js (right):

https://codereview.chromium.org/15764004/diff/17001/Source/devtools/front_end...
Source/devtools/front_end/sdk/RuntimeModel.js:178: this.origin = isPageContext
&& !origin ? "<page context>" : origin;
On 2014/08/25 09:22:30, yurys wrote:
> I guess it should be assigned into .name ?

Done.

[dglazkov, 2014-08-27 19:54:15]

lgtm

[lushnikov, 2014-08-27 19:55:00]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2014-08-27 19:55:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lushnikov@chromium.org/15764004/57001

[commit-bot: I haz the power, 2014-08-27 20:44:47]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)

[commit-bot: I haz the power, 2014-08-27 21:04:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-27 21:04:03]

Try jobs failed on following builders:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)

[lushnikov, 2014-08-28 08:27:57]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2014-08-28 08:29:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lushnikov@chromium.org/15764004/57001

[commit-bot: I haz the power, 2014-08-28 08:52:59]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)

[commit-bot: I haz the power, 2014-08-28 09:12:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-28 09:12:57]

Try jobs failed on following builders:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)

[lushnikov, 2014-08-29 09:21:26]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2014-08-29 09:21:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lushnikov@chromium.org/15764004/97001

[commit-bot: I haz the power, 2014-08-29 09:59:38]

Committed patchset #7 (id:97001) as 181086

[dcheng, 2014-09-22 22:09:49]

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

[dcheng, 2014-09-22 22:09:50]

https://codereview.chromium.org/15764004/diff/97001/public/web/WebFrame.h
File public/web/WebFrame.h (right):

https://codereview.chromium.org/15764004/diff/97001/public/web/WebFrame.h#new...
public/web/WebFrame.h:290: virtual void setIsolatedWorldHumanReadableName(
Please move this function to WebLocalFrame.h. I don't think we'll ever implement
it for WebRemoteFrame.

[lushnikov, 2014-09-23 09:08:25]

On 2014/09/22 22:09:50, dcheng wrote:
> https://codereview.chromium.org/15764004/diff/97001/public/web/WebFrame.h
> File public/web/WebFrame.h (right):
> 
>
https://codereview.chromium.org/15764004/diff/97001/public/web/WebFrame.h#new...
> public/web/WebFrame.h:290: virtual void setIsolatedWorldHumanReadableName(
> Please move this function to WebLocalFrame.h. I don't think we'll ever
implement
> it for WebRemoteFrame.

I believe we would like to support this for RemoteFrame as well - are there any
obstacles?

[dcheng, 2014-09-23 09:10:05]

On 2014/09/23 at 09:08:25, lushnikov wrote:
> On 2014/09/22 22:09:50, dcheng wrote:
> > https://codereview.chromium.org/15764004/diff/97001/public/web/WebFrame.h
> > File public/web/WebFrame.h (right):
> > 
> >
https://codereview.chromium.org/15764004/diff/97001/public/web/WebFrame.h#new...
> > public/web/WebFrame.h:290: virtual void setIsolatedWorldHumanReadableName(
> > Please move this function to WebLocalFrame.h. I don't think we'll ever
implement
> > it for WebRemoteFrame.
> 
> I believe we would like to support this for RemoteFrame as well - are there
any obstacles?

Can you help me understand why we'd need this for remote frames? They don't have
an execution context.