Bug 78602

Created attachment 126971 [details] Patch

Attachment 126971 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource First, rewinding head to replay your work on top of it... Applying: [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets Using index info to reconstruct a base tree... <stdin>:1578: trailing whitespace. <stdin>:1647: trailing whitespace. <stdin>:1657: trailing whitespace. <stdin>:1672: trailing whitespace. return 0; <stdin>:1674: trailing whitespace. warning: squelched 7 whitespace errors warning: 12 lines add whitespace errors. Falling back to patching base and 3-way merge... warning: too many files (created: 168776 deleted: 3), skipping inexact rename detection Auto-merging LayoutTests/ChangeLog CONFLICT (content): Merge conflict in LayoutTests/ChangeLog Auto-merging Source/WebCore/ChangeLog CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog Auto-merging Source/WebKit2/ChangeLog CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog Auto-merging Tools/ChangeLog CONFLICT (content): Merge conflict in Tools/ChangeLog Failed to merge in the changes. Patch failed at 0001 [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 126980 [details] Patch

Attachment 126980 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource From git://git.webkit.org/WebKit 1475f6d..6640514 master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 107710 = 1475f6d06ac1bdc58f6b7b27ade48d967e113e10 r107712 = 66405148c5f7c19d2179ddbd3d3d5ee458d4b6d6 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Applying: [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets Using index info to reconstruct a base tree... <stdin>:1578: trailing whitespace. <stdin>:1647: trailing whitespace. <stdin>:1657: trailing whitespace. <stdin>:1672: trailing whitespace. return 0; <stdin>:1674: trailing whitespace. warning: squelched 7 whitespace errors warning: 12 lines add whitespace errors. Falling back to patching base and 3-way merge... warning: too many files (created: 168776 deleted: 3), skipping inexact rename detection Auto-merging LayoutTests/ChangeLog Auto-merging Source/WebCore/ChangeLog CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog Auto-merging Source/WebKit2/ChangeLog CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog Auto-merging Tools/ChangeLog CONFLICT (content): Merge conflict in Tools/ChangeLog Failed to merge in the changes. Patch failed at 0001 [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 126980 [details] Patch This is wrong, you do not need the page to be resumed in order to paint. you just need to emit the viewportTrajectoryVectorChanged and that should be it.

Created attachment 127171 [details] Patch

Attachment 127171 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Last 3072 characters of output: M ChangeLog M LayoutTests/platform/chromium-linux/tables/mozilla/bugs/bug11026-expected.png M LayoutTests/platform/chromium-linux/tables/mozilla/bugs/bug10565-expected.png M LayoutTests/platform/chromium-linux/tables/mozilla/bugs/bug1188-expected.png M LayoutTests/platform/chromium/test_expectations.txt A LayoutTests/platform/chromium-mac-snowleopard/tables/mozilla/bugs/bug11026-expected.png A LayoutTests/platform/chromium-mac-snowleopard/tables/mozilla/bugs/bug10565-expected.png M LayoutTests/platform/chromium-mac-snowleopard/tables/mozilla/bugs/bug1188-expected.png D LayoutTests/platform/chromium-mac/tables/mozilla/bugs/bug11026-expected.png D LayoutTests/platform/chromium-mac/tables/mozilla/bugs/bug10565-expected.png M LayoutTests/platform/chromium-mac-leopard/tables/mozilla/bugs/bug11026-expected.png M LayoutTests/platform/chromium-mac-leopard/tables/mozilla/bugs/bug10565-expected.png M LayoutTests/platform/chromium-mac-leopard/tables/mozilla/bugs/bug1188-expected.png M LayoutTests/platform/chromium-win/tables/mozilla/bugs/bug1188-expected.png M LayoutTests/platform/chromium-win/tables/mozilla/bugs/bug11026-expected.png M LayoutTests/platform/chromium-win/tables/mozilla/bugs/bug10565-expected.png A LayoutTests/inspector/elements/resolve-node-blocked-expected.txt A LayoutTests/inspector/elements/resolve-node-blocked.html M LayoutTests/ChangeLog M Source/cmake/FindCFLite.cmake M Source/WebCore/ChangeLog M Source/WebCore/inspector/InjectedScript.cpp M Source/WebCore/inspector/InjectedScript.h W: -empty_dir: trunk/LayoutTests/platform/chromium-mac/tables/mozilla/bugs/bug10565-expected.png W: -empty_dir: trunk/LayoutTests/platform/chromium-mac/tables/mozilla/bugs/bug11026-expected.png r107809 = 26c3dbf4d929380fdd9660a6218cc2abc86f8070 (refs/remotes/origin/master) First, rewinding head to replay your work on top of it... Applying: Web Inspector: crash when inspecting an element on a page with eval disabled by CSP Using index info to reconstruct a base tree... <stdin>:35: new blank line at EOF. + warning: 1 line adds whitespace errors. Falling back to patching base and 3-way merge... Auto-merging LayoutTests/ChangeLog CONFLICT (content): Merge conflict in LayoutTests/ChangeLog Auto-merging Source/WebCore/ChangeLog CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog Auto-merging Source/WebCore/inspector/InjectedScript.cpp CONFLICT (content): Merge conflict in Source/WebCore/inspector/InjectedScript.cpp Auto-merging Source/WebCore/inspector/InjectedScript.h CONFLICT (content): Merge conflict in Source/WebCore/inspector/InjectedScript.h Failed to merge in the changes. Patch failed at 0001 Web Inspector: crash when inspecting an element on a page with eval disabled by CSP When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 127171 [details] Patch r- wrong solution

Created attachment 128465 [details] Patch

Comment on attachment 128465 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=128465&action=review A couple of questions first :) > Source/WebKit2/UIProcess/DrawingAreaProxy.h:97 > + virtual void setVisibleContentRectTrajectoryVector(const WebCore::IntRect& visibleContentsRect, const WebCore::FloatPoint&) { } Hm, if you add the variable name to the first argument, why not the second? > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:206 > + connect(m_flickProvider, SIGNAL(contentXChanged()), SLOT(flickableMovingPositionUpdate()), Qt::DirectConnection); > + connect(m_flickProvider, SIGNAL(contentYChanged()), SLOT(flickableMovingPositionUpdate()), Qt::DirectConnection); IMHO we should leave out the DirectConnection parameters from connect() unless we are knowingly in a threaded-context. > Source/WebKit2/WebProcess/WebPage/qt/LayerTreeHostQt.cpp:-424 > - if (rect == m_visibleContentsRect && scale == m_contentsScale) > - return; Hm, why did you remove the check? Is there a reason why schedulerLayerFlush() needs to be called even if the parameters haven't changed? > Source/WebKit2/WebProcess/WebPage/qt/LayerTreeHostQt.cpp:434 > +void LayerTreeHostQt::setVisibleContentRectTrajectoryVector(const IntRect& rect, const FloatPoint& trajectoryVector) Just thinking, should it be called setVisibleContentRectAndTrajectoryVector?

(In reply to comment #10) > (From update of attachment 128465 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=128465&action=review > > A couple of questions first :) > > > Source/WebKit2/UIProcess/DrawingAreaProxy.h:97 > > + virtual void setVisibleContentRectTrajectoryVector(const WebCore::IntRect& visibleContentsRect, const WebCore::FloatPoint&) { } > > Hm, if you add the variable name to the first argument, why not the second? Maybe I was too mechanical :-) > > > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:206 > > + connect(m_flickProvider, SIGNAL(contentXChanged()), SLOT(flickableMovingPositionUpdate()), Qt::DirectConnection); > > + connect(m_flickProvider, SIGNAL(contentYChanged()), SLOT(flickableMovingPositionUpdate()), Qt::DirectConnection); > > IMHO we should leave out the DirectConnection parameters from connect() unless we are knowingly in a threaded-context. OK, though andras added them all over > > > Source/WebKit2/WebProcess/WebPage/qt/LayerTreeHostQt.cpp:-424 > > - if (rect == m_visibleContentsRect && scale == m_contentsScale) > > - return; > > Hm, why did you remove the check? Is there a reason why schedulerLayerFlush() needs to be called even if the parameters haven't changed? Because two methods are used...so if I want to do this check I will need to store the trajectory vector as well and compare it. Not really worth it. > > > Source/WebKit2/WebProcess/WebPage/qt/LayerTreeHostQt.cpp:434 > > +void LayerTreeHostQt::setVisibleContentRectTrajectoryVector(const IntRect& rect, const FloatPoint& trajectoryVector) > > Just thinking, should it be called setVisibleContentRectAndTrajectoryVector? I didn't want to do the renaming, but I would suggest that we go for LayerTreeHostQt::setVisibleContentRectForPanning(const FloatPoint& trajectoryVector) LayerTreeHostQt::setVisibleContentRectForScaling(const IntRect& rect, float scale)

LayerTreeHostQt::setVisibleContentRectForPanning(const IntRect& rect, const FloatPoint& trajectoryVector) LayerTreeHostQt::setVisibleContentRectForScaling(const IntRect& rect, float scale) I mean :-)

Comment on attachment 128465 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=128465&action=review >>> Source/WebKit2/WebProcess/WebPage/qt/LayerTreeHostQt.cpp:434 >>> +void LayerTreeHostQt::setVisibleContentRectTrajectoryVector(const IntRect& rect, const FloatPoint& trajectoryVector) >> >> Just thinking, should it be called setVisibleContentRectAndTrajectoryVector? > > I didn't want to do the renaming, but I would suggest that we go for > > LayerTreeHostQt::setVisibleContentRectForPanning(const FloatPoint& trajectoryVector) > LayerTreeHostQt::setVisibleContentRectForScaling(const IntRect& rect, float scale) _Those_ sounds like nice and descriptive names :)

Created attachment 128468 [details] Patch

Landed in r108625.

The Flickable component has movementStarted() and movementEnded() signals, I think they can be used to avoid multiple calls to flickableMovingStateChanged() due to multiple emissions of movingChanged().

(In reply to comment #16) > The Flickable component has movementStarted() and movementEnded() signals, I think they can be used to avoid multiple calls to flickableMovingStateChanged() due to multiple emissions of movingChanged(). Up to doing a patch? :-)

Yes, I can do that.