DevTools: Add properties and constants support to "Documentation" module

DevTools: Add properties and constants support to "Documentation" module

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

[apavlov, 2014-08-13 13:35:35]

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/documentation/DocumentationURLProvider.js
(right):

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:47:
WebInspector.DocumentationURLProvider._createProperty = function(sourceUrl,
objectName, searchTerm)
Turn this into a nested function inside itemDescriptors(), since it's only used
there. As a bonus, you won't have to pass sourceUrl and objectName, both of
which you can extract from sourceRef.

Also, let's name it "createDescriptor", since we have moved from properties to
item descriptors.

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:52: }
Missing ';'

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:62: var
possibleProperties = [];
var descriptors = [];

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:68: var
property = WebInspector.DocumentationURLProvider._createProperty(sourceRef.url,
sourceRef.name, "constants");
We discourage multiple var declarations in conditional branches. Actually, you
can rewrite the entire snippet (from if (searchTerm...) to
possibleProperties.push) in a much shorter way (provided that we rename
possibleProperties -> descriptors):

descriptors.push(createDescriptor(searchTerm.toUpperCase() === searchTerm ?
"constants" : searchTerm));

[lushnikov, 2014-08-13 13:42:15]

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/documentation/DocumentationURLProvider.js
(right):

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:52: }
semicolon

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:65: if
(!(searchTerm in sourceRef.source))
lets use hasOwnProperty instead of "in"

[apavlov, 2014-08-13 13:53:07]

"compatibility" !== "support", so the subject/description should be fixed

[semeny, 2014-08-13 14:10:18]

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/documentation/DocumentationURLProvider.js
(right):

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:47:
WebInspector.DocumentationURLProvider._createProperty = function(sourceUrl,
objectName, searchTerm)
On 2014/08/13 13:35:35, apavlov wrote:
> Turn this into a nested function inside itemDescriptors(), since it's only
used
> there. As a bonus, you won't have to pass sourceUrl and objectName, both of
> which you can extract from sourceRef.
> 
> Also, let's name it "createDescriptor", since we have moved from properties to
> item descriptors.

Done.

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:52: }
On 2014/08/13 13:35:35, apavlov wrote:
> Missing ';'

Done.

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:62: var
possibleProperties = [];
On 2014/08/13 13:35:34, apavlov wrote:
> var descriptors = [];

Done.

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:65: if
(!(searchTerm in sourceRef.source))
On 2014/08/13 13:42:15, lushnikov wrote:
> lets use hasOwnProperty instead of "in"

Done.

https://codereview.chromium.org/472483002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:68: var
property = WebInspector.DocumentationURLProvider._createProperty(sourceRef.url,
sourceRef.name, "constants");
On 2014/08/13 13:35:35, apavlov wrote:
> We discourage multiple var declarations in conditional branches. Actually, you
> can rewrite the entire snippet (from if (searchTerm...) to
> possibleProperties.push) in a much shorter way (provided that we rename
> possibleProperties -> descriptors):
> 
> descriptors.push(createDescriptor(searchTerm.toUpperCase() === searchTerm ?
> "constants" : searchTerm));

Done.

[apavlov, 2014-08-13 14:33:31]

Please remove the comments mentioned, and we are good to go. lgtm

https://codereview.chromium.org/472483002/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/documentation/DocumentationURLProvider.js
(right):

https://codereview.chromium.org/472483002/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationURLProvider.js:41: //
FIXME: method hasOwnProperty can be used with every source object.
Please remove these comment lines. Since we check each object's prototype, we
are guaranteed to find the required property/function there.

[semeny, 2014-08-13 14:40:10]

The CQ bit was checked by semeny@google.com

[commit-bot: I haz the power, 2014-08-13 14:40:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/semeny@google.com/472483002/80001

[commit-bot: I haz the power, 2014-08-13 15:48:07]

Committed patchset #4 (80001) as 180184