DevTools: do not clear console when preserving log

DevTools: do not clear console when preserving log

The 'console.clear()' will not clear the console when 'Preserve log' is enabled.

BUG=735440
Review-Url: https://codereview.chromium.org/2970313002
Cr-Commit-Position: refs/heads/master@{#487782}
Committed: https://chromium.googlesource.com/chromium/src/+/bc0bb7cc655ccb4c1eb7ece209e82bc806de71cf

[luoe, 2017-07-06 22:15:40]

Description was changed from

==========
DevTools: do not clear console when preserving log or in non-main frame

BUG=735440
==========

to

==========
DevTools: do not clear console when preserving log or in non-main frame

The 'console.clear()' will not clear the console when 'Preserve log' is enabled
or if it does not originate from the main frame. Cmd-L and the clear button
still work all the time.

BUG=735440
==========

[luoe, 2017-07-06 22:16:00]

luoe@chromium.org changed reviewers:
+ pfeldman@chromium.org

[luoe, 2017-07-06 22:16:01]

Please take a look.

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js
(right):

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js:53:
if (Common.moduleSetting('preserveConsoleLog').get() ||
ConsoleModel is the only place that knows that preserveLog setting should
prevent console.clear().  Maybe _clearIfNecessary() should be merged with this,
and we make 'message' an optional parameter?

[pfeldman, 2017-07-12 21:17:29]

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
(right):

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js:209:
Common.UIString('console.clear() was prevented from non-main frame or \'Preserve
log\'');
This does not sounds like a consistent sentence to me.

`console.clear() was prevented due to 'Preserve log'` or
`console.clear() from iframe was ignored`

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js
(right):

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js:53:
if (Common.moduleSetting('preserveConsoleLog').get() ||
On 2017/07/06 22:16:00, luoe wrote:
> ConsoleModel is the only place that knows that preserveLog setting should
> prevent console.clear().  Maybe _clearIfNecessary() should be merged with
this,
> and we make 'message' an optional parameter?

As you prefer.

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js:58:
var executionContext =
runtimeModel.executionContext(message.executionContextId);
when executionContext.isDefault === false, we should not clear console. this is
like iframe to use in this case...

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js:60:
var frame = resourceTreeModel && executionContext && executionContext.frameId &&
If there is no resourceTreeModel, we should allow clear.

[luoe, 2017-07-13 00:42:03]

Please take another look

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
(right):

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js:209:
Common.UIString('console.clear() was prevented from non-main frame or \'Preserve
log\'');
On 2017/07/12 21:17:28, pfeldman wrote:
> This does not sounds like a consistent sentence to me.
> 
> `console.clear() was prevented due to 'Preserve log'` or
> `console.clear() from iframe was ignored`

Done.

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js
(right):

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js:58:
var executionContext =
runtimeModel.executionContext(message.executionContextId);
On 2017/07/12 21:17:28, pfeldman wrote:
> when executionContext.isDefault === false, we should not clear console. this
is
> like iframe to use in this case...

Done.

https://codereview.chromium.org/2970313002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js:60:
var frame = resourceTreeModel && executionContext && executionContext.frameId &&
On 2017/07/12 21:17:28, pfeldman wrote:
> If there is no resourceTreeModel, we should allow clear.

Done.

[luoe, 2017-07-18 22:49:00]

Description was changed from

==========
DevTools: do not clear console when preserving log or in non-main frame

The 'console.clear()' will not clear the console when 'Preserve log' is enabled
or if it does not originate from the main frame. Cmd-L and the clear button
still work all the time.

BUG=735440
==========

to

==========
DevTools: do not clear console when preserving log

The 'console.clear()' will not clear the console when 'Preserve log' is enabled.

BUG=735440
==========

[luoe, 2017-07-18 22:52:12]

luoe@chromium.org changed reviewers:
+ kozyatinskiy@chromium.org

[luoe, 2017-07-18 22:52:13]

In an offline discussion with kozy@, we may want to consider the iframe case
when a solution is settled in https://crbug.com/682916

For now, we can fix the regression in the original bug.

Please take a look, pfeldman@

[luoe, 2017-07-19 00:37:19]

Patchset #4 (id:60001) has been deleted

[luoe, 2017-07-19 03:05:22]

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

[lushnikov, 2017-07-19 03:12:58]

lgtm
(please make sure @pfeldman is happy with this change before landing)

[pfeldman, 2017-07-19 05:43:41]

fine by me

[luoe, 2017-07-19 05:44:16]

The CQ bit was checked by luoe@chromium.org

[commit-bot: I haz the power, 2017-07-19 05:44:30]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-19 07:40:01]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1500443056123640,
"parent_rev": "780ed524823d11e245f40c2f68ebf03a0b2c3ee0", "commit_rev":
"bc0bb7cc655ccb4c1eb7ece209e82bc806de71cf"}

[commit-bot: I haz the power, 2017-07-19 07:40:14]

Description was changed from

==========
DevTools: do not clear console when preserving log

The 'console.clear()' will not clear the console when 'Preserve log' is enabled.

BUG=735440
==========

to

==========
DevTools: do not clear console when preserving log

The 'console.clear()' will not clear the console when 'Preserve log' is enabled.

BUG=735440

Review-Url: https://codereview.chromium.org/2970313002
Cr-Commit-Position: refs/heads/master@{#487782}
Committed:
https://chromium.googlesource.com/chromium/src/+/bc0bb7cc655ccb4c1eb7ece209e8...
==========

[commit-bot: I haz the power, 2017-07-19 07:40:16]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/bc0bb7cc655ccb4c1eb7ece209e8...