Invalidate Custom Scrollbars irrespective of focused frame.

Invalidate Custom Scrollbars irrespective of focused frame.

Invalidate Custom Scrollbars irrespective of focused frame. As the CSS
property indicates that custom scrollbars needs to be invalidated on losing
window focus, so if the focus is on iframe element and window lost focus
then the custom scrollbars of entire page needs to be invalidated to justify
window-inactive property.

BUG=438596
(Bug 3)

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

[MuVen, 2014-12-11 05:33:46]

sataya.m@samsung.com changed reviewers:
+ rune@opera.com

[MuVen, 2014-12-11 05:34:08]

PTAL.

[MuVen, 2014-12-11 16:00:22]

Rune, I have missed out this case, I have added the fix and test-case. PTAL.

[MuVen, 2014-12-15 16:07:15]

On 2014/12/11 16:00:22, muven wrote:
> Rune, I have missed out this case, I have added the fix and test-case. PTAL.

friendly ping !!!

[MuVen, 2014-12-16 17:54:59]

sataya.m@samsung.com changed reviewers:
+ skobes@chromium.org

[MuVen, 2014-12-16 17:55:50]

@ Skobes/Rune

PTAL.

Thanks,
~MuVen.

[skobes, 2014-12-16 22:19:52]

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
File Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
Source/core/editing/FrameSelection.cpp:1469: // needs to be applied for entire
page. So invalidate from root FrameView
If the frame's focus state is irrelevant, this seems like the wrong place to
trigger the invalidation.  Maybe it should hook into FocusController::setActive
instead?

[MuVen, 2014-12-17 17:17:15]

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
File Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
Source/core/editing/FrameSelection.cpp:1469: // needs to be applied for entire
page. So invalidate from root FrameView
On 2014/12/16 22:19:52, skobes wrote:
> If the frame's focus state is irrelevant, this seems like the wrong place to
> trigger the invalidation.  Maybe it should hook into
FocusController::setActive
> instead?

yes, i tried earlier with what you have mentioned. But the following API's for
Caret appears in the active frame, Element focusStateChanged will go for toss
which eventually fails lost of test cases. Due to these reasons i have written
it here.

[skobes, 2014-12-17 17:41:31]

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
File Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
Source/core/editing/FrameSelection.cpp:1469: // needs to be applied for entire
page. So invalidate from root FrameView
On 2014/12/17 17:17:15, muven wrote:
> yes, i tried earlier with what you have mentioned. But the following API's for
> Caret appears in the active frame, Element focusStateChanged will go for toss
> which eventually fails lost of test cases.

I'm sorry, I don't understand what you're trying to say here.  Can you rephrase
this sentence and/or provide more detail?

[MuVen, 2014-12-17 18:06:52]

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
File Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
Source/core/editing/FrameSelection.cpp:1469: // needs to be applied for entire
page. So invalidate from root FrameView
I have earlier tried calling focusorActiveStateChanged on root Frame from
SetActive;   
    Frame* frame = focusedOrMainFrame();
    if (frame->isLocalFrame())
        toLocalFrame(frame)->selection().pageActivationChanged();
//tried
toLocalFrame(frame)->localFrameRoot()->selection().pageActivationChanged

But many test cases have failed due to this as,
spellchecker/CaretVisiblilty/FocusedElement  api's implementation is based on
focused frame (if available).

[skobes, 2014-12-17 18:51:22]

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
File Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
Source/core/editing/FrameSelection.cpp:1469: // needs to be applied for entire
page. So invalidate from root FrameView
On 2014/12/17 18:06:52, muven wrote:
> I have earlier tried calling focusorActiveStateChanged on root Frame from
> SetActive;   
>     Frame* frame = focusedOrMainFrame();
>     if (frame->isLocalFrame())
>         toLocalFrame(frame)->selection().pageActivationChanged();
> //tried
> toLocalFrame(frame)->localFrameRoot()->selection().pageActivationChanged
> 
> But many test cases have failed due to this as,
> spellchecker/CaretVisiblilty/FocusedElement  api's implementation is based on
> focused frame (if available).

I didn't mean for FocusController::setActive to call focusorActiveStateChanged
on the root frame.  I meant that FocusController::setActive should invalidate
all of the page's custom scrollbars.

[MuVen, 2014-12-17 18:59:43]

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
File Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/792233004/diff/1/Source/core/editing/FrameSel...
Source/core/editing/FrameSelection.cpp:1469: // needs to be applied for entire
page. So invalidate from root FrameView
Oh ok ok !!! Got it, Make sense even. I shall update the same by tomo.
I'm sorry for miss-understanding & for my english.

Thanks,

[MuVen, 2014-12-18 10:32:42]

PTAL. addressed as suggested by you.

[skobes, 2014-12-18 19:08:17]

lgtm

[MuVen, 2014-12-19 03:25:58]

The CQ bit was checked by sataya.m@samsung.com

[commit-bot: I haz the power, 2014-12-19 03:26:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/792233004/20001

[commit-bot: I haz the power, 2014-12-19 03:34:21]

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

[commit-bot: I haz the power, 2014-12-19 03:34:23]

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

[MuVen, 2014-12-22 14:56:04]

sataya.m@samsung.com changed reviewers:
+ pdr@chromium.org
- rune@opera.com

[MuVen, 2014-12-22 14:56:23]

@pdr, for owners stamp.

[pdr., 2014-12-22 18:34:48]

LGTM

https://codereview.chromium.org/792233004/diff/20001/Source/core/page/FocusCo...
File Source/core/page/FocusController.cpp (right):

https://codereview.chromium.org/792233004/diff/20001/Source/core/page/FocusCo...
Source/core/page/FocusController.cpp:729: // Invalidate All CustomScrollbars as
CustomScrollbars supports
Please rephrase this as something like the following:
        // Invalidate all custom scrollbars because they support the CSS
        // window-active attribute. This should be applied to the entire page so
        // we invalidate from the root FrameView instead of just the focused
frame.

[MuVen, 2014-12-23 05:58:44]

The CQ bit was checked by sataya.m@samsung.com

[commit-bot: I haz the power, 2014-12-23 05:59:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/792233004/40001

[commit-bot: I haz the power, 2014-12-23 07:09:38]

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

[commit-bot: I haz the power, 2014-12-23 07:09:38]

Failed to commit the patch.
svn: E155010: Commit failed (details follow):
svn: E155010:
'/b/infra_internal/commit_queue/workdir/blink/LayoutTests/virtual/slimmingpaint/svg'
is scheduled for addition, but is missing

[commit-bot: I haz the power, 2014-12-23 07:09:49]

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

[MuVen, 2014-12-23 07:10:33]

The CQ bit was checked by sataya.m@samsung.com

[commit-bot: I haz the power, 2014-12-23 07:10:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/792233004/40001

[commit-bot: I haz the power, 2014-12-23 07:11:26]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187637