New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 657552 link

Starred by 1 user
Status: Fixed
Owner:
█ lushnikov@chromium.org
Last visit 29 days ago
Closed: Oct 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

DevTools: sources navigator throws exception

Project Member Reported by lushnikov@chromium.org, Oct 19 2016 
SourcesNavigator sources throws an exception at uiSourceCodeAdded, because due to
a bad timing, uiSourceCode happens before a target has ever being created:

    /**
     * @override
     * @param {!WebInspector.UISourceCode} uiSourceCode
     */
    uiSourceCodeAdded: function(uiSourceCode)
    {
        var inspectedPageURL = WebInspector.targetManager.mainTarget().inspectedURL();
        if (uiSourceCode.url() === inspectedPageURL)
            this.revealUISourceCode(uiSourceCode, true);
    },
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 19 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c81a7da4e2cbef80f12d8fdd3a2a6d1bb535c219

commit c81a7da4e2cbef80f12d8fdd3a2a6d1bb535c219
Author: lushnikov <lushnikov@chromium.org>
Date: Wed Oct 19 23:13:05 2016

DevTools: [Sources] sourcesNavigator should not rely on existance of main target

BUG= 657552 
R=dgozman

Review-Url: https://chromiumcodereview.appspot.com/2434853003
Cr-Commit-Position: refs/heads/master@{#426324}

[modify] https://crrev.com/c81a7da4e2cbef80f12d8fdd3a2a6d1bb535c219/third_party/WebKit/Source/devtools/front_end/sources/SourcesNavigator.js

Comment 2 by lushnikov@chromium.org, Oct 25 2016

Labels: M-55 Merge-Request-55
Works great; it would be nice to have this merged.

Comment 3 by dimu@chromium.org, Oct 26 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)

Comment 4 by dimu@chromium.org, Oct 26 2016 

Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 26 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/346f746833177f384afa09c6ed62764d8017e087

commit 346f746833177f384afa09c6ed62764d8017e087
Author: Andrey Lushnikov <lushnikov@chromium.org>
Date: Wed Oct 26 23:18:47 2016

DevTools: [Sources] sourcesNavigator should not rely on existance of main target

BUG= 657552 
R=dgozman

Review-Url: https://chromiumcodereview.appspot.com/2434853003
Cr-Commit-Position: refs/heads/master@{#426324}
(cherry picked from commit c81a7da4e2cbef80f12d8fdd3a2a6d1bb535c219)

Review URL: https://codereview.chromium.org/2453213002 .

Cr-Commit-Position: refs/branch-heads/2883@{#324}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/346f746833177f384afa09c6ed62764d8017e087/third_party/WebKit/Source/devtools/front_end/sources/SourcesNavigator.js
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 26 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/346f746833177f384afa09c6ed62764d8017e087

commit 346f746833177f384afa09c6ed62764d8017e087
Author: Andrey Lushnikov <lushnikov@chromium.org>
Date: Wed Oct 26 23:18:47 2016

DevTools: [Sources] sourcesNavigator should not rely on existance of main target

BUG= 657552 
R=dgozman

Review-Url: https://chromiumcodereview.appspot.com/2434853003
Cr-Commit-Position: refs/heads/master@{#426324}
(cherry picked from commit c81a7da4e2cbef80f12d8fdd3a2a6d1bb535c219)

Review URL: https://codereview.chromium.org/2453213002 .

Cr-Commit-Position: refs/branch-heads/2883@{#324}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/346f746833177f384afa09c6ed62764d8017e087/third_party/WebKit/Source/devtools/front_end/sources/SourcesNavigator.js

Comment 7 by lushnikov@chromium.org, Oct 27 2016

Status: Fixed (was: Assigned)

Comment 8 by pucchakayala@chromium.org, Oct 27 2016

Labels: Needs-Feedback
lushnikov@, can this be tested manually ?
If so, can you please provide us with the steps ?

Comment 9 by lushnikov@chromium.org, Oct 27 2016 

@pucchakayala: there's a race condition which resulted in an exception and shouldn't any more; i'm not aware of any way to trigger that behavior.
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/346f746833177f384afa09c6ed62764d8017e087

commit 346f746833177f384afa09c6ed62764d8017e087
Author: Andrey Lushnikov <lushnikov@chromium.org>
Date: Wed Oct 26 23:18:47 2016

DevTools: [Sources] sourcesNavigator should not rely on existance of main target

BUG= 657552 
R=dgozman

Review-Url: https://chromiumcodereview.appspot.com/2434853003
Cr-Commit-Position: refs/heads/master@{#426324}
(cherry picked from commit c81a7da4e2cbef80f12d8fdd3a2a6d1bb535c219)

Review URL: https://codereview.chromium.org/2453213002 .

Cr-Commit-Position: refs/branch-heads/2883@{#324}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/346f746833177f384afa09c6ed62764d8017e087/third_party/WebKit/Source/devtools/front_end/sources/SourcesNavigator.js

Comment 11 by dimu@google.com, Nov 4 2016 

[Automated comment] removing mislabelled merge-merged-2840

Comment 12 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840
►

Sign in to add a comment