WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
61192
Web Inspector: do not issue frontendReused for reload or navigate.
https://bugs.webkit.org/show_bug.cgi?id=61192
Summary
Web Inspector: do not issue frontendReused for reload or navigate.
Pavel Feldman
Reported
2011-05-20 09:05:46 PDT
We should only issue it while connecting to the page that is loaded prior to attach only.
Attachments
Patch
(8.12 KB, patch)
2011-05-20 09:09 PDT
,
Pavel Feldman
no flags
Details
Formatted Diff
Diff
Patch
(8.24 KB, patch)
2011-05-20 09:46 PDT
,
Pavel Feldman
pfeldman
: review+
commit-queue
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from cr-jail-8
(207.50 KB, application/zip)
2011-05-20 12:01 PDT
,
WebKit Commit Bot
no flags
Details
patch
(5.75 KB, patch)
2011-05-23 02:30 PDT
,
Andrey Kosyakov
no flags
Details
Formatted Diff
Diff
patch (untabified; sorry)
(5.78 KB, patch)
2011-05-23 02:44 PDT
,
Andrey Kosyakov
pfeldman
: review-
Details
Formatted Diff
Diff
patch
(14.14 KB, patch)
2011-05-25 09:11 PDT
,
Andrey Kosyakov
pfeldman
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Pavel Feldman
Comment 1
2011-05-20 09:09:47 PDT
Created
attachment 94232
[details]
Patch
Yury Semikhatsky
Comment 2
2011-05-20 09:35:46 PDT
Comment on
attachment 94232
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=94232&action=review
> Source/WebCore/inspector/front-end/ResourcesPanel.js:248 > + if (!event.data.frame.parentId) {
Please add a comment explaining this logic.
Pavel Feldman
Comment 3
2011-05-20 09:46:19 PDT
Created
attachment 94235
[details]
Patch
WebKit Commit Bot
Comment 4
2011-05-20 12:01:30 PDT
Comment on
attachment 94235
[details]
Patch Rejecting
attachment 94235
[details]
from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'build-..." exit_code: 2 Last 500 characters of output: tests/xmlhttprequest ................................................................................................................................................................................ http/tests/xmlhttprequest/web-apps ............... http/tests/xmlhttprequest/workers ........... http/tests/xmlviewer . http/tests/xmlviewer/dumpAsText ........... 767.94s total testing time 23585 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 17 test cases (<1%) had stderr output Full output:
http://queues.webkit.org/results/8718550
WebKit Commit Bot
Comment 5
2011-05-20 12:01:35 PDT
Created
attachment 94255
[details]
Archive of layout-test-results from cr-jail-8 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: cr-jail-8 Port: Mac Platform: Mac OS X 10.6.7
Andrey Kosyakov
Comment 6
2011-05-23 02:30:02 PDT
Created
attachment 94389
[details]
patch The way we remove old frames and preserve the main frame upon navigation is somewhat unfortunate, so let's at least try to keep it on the model level, rather than have similar logic proliferate to the higher-level ResourcePanel. We already have notifications going from model to panel upon resource changes, so it shouldn't be necessary to have a separate logic to reset it upon navigation (unless for performance reasons, which, I think, are not really an issue here). Here's my shot at fixing this -- mostly similar, but remove obsolete frames one-by-one via "usual" means (_frameDetached). To Pavel: I couldn't reproduce the bug you were observing with id collision of old and new main frames in different renderers with my approach. OTOH, I don't see why there should be such a bug, given that we seem to properly replace old frames with new ones in case of id collision. May be I'm missing something... :)
WebKit Review Bot
Comment 7
2011-05-23 02:32:04 PDT
Attachment 94389
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/inspector/front-end/ResourceTreeModel.js:128: Line contains tab character. [whitespace/tab] [5] Source/WebCore/inspector/front-end/ResourceTreeModel.js:160: Line contains tab character. [whitespace/tab] [5] Source/WebCore/inspector/front-end/ResourceTreeModel.js:161: Line contains tab character. [whitespace/tab] [5] Total errors found: 3 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Andrey Kosyakov
Comment 8
2011-05-23 02:44:04 PDT
Created
attachment 94391
[details]
patch (untabified; sorry)
Pavel Feldman
Comment 9
2011-05-24 10:32:32 PDT
Comment on
attachment 94391
[details]
patch (untabified; sorry) This will regress HTML5 resources panel cleanup that was performed from resourcetreemodel's frontendReused callback.
Andrey Kosyakov
Comment 10
2011-05-25 09:11:46 PDT
Created
attachment 94790
[details]
patch - fixed HTML5 storage problem (exposed StoragePanel.reset()) - changed order of notifications from backend upon didCommitLoad (first do all resets, then notify page agent on navigation) - now reset m_DidCommitLoadFired when detaching front-end - create main resource early, to get resource names in resource tree abbreviated consistently
Pavel Feldman
Comment 11
2011-05-25 11:10:19 PDT
Comment on
attachment 94790
[details]
patch View in context:
https://bugs.webkit.org/attachment.cgi?id=94790&action=review
> Source/WebCore/inspector/InspectorInstrumentation.cpp:608 > + pageAgent->frameNavigated(loader);
I am positive it regresses :) Did you run the tests?
Andrey Kosyakov
Comment 12
2011-05-26 05:04:51 PDT
(In reply to
comment #11
)
> (From update of
attachment 94790
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=94790&action=review
> > > Source/WebCore/inspector/InspectorInstrumentation.cpp:608 > > + pageAgent->frameNavigated(loader); > > I am positive it regresses :) Did you run the tests?
All tests are passing, manual tests look sane too. Can you please be a bit more specific?
Pavel Feldman
Comment 13
2011-05-27 12:23:31 PDT
Comment on
attachment 94790
[details]
patch Ok, it looks sane. Lets see how it goes.
Andrey Kosyakov
Comment 14
2011-05-30 02:22:51 PDT
Manually committed
r87681
:
http://trac.webkit.org/changeset/87681
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug