Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2563933003:
Remove cancelWithError() from DocumentThreadableLoader (Closed)
|
Created:
4 years ago by tyoshino (SeeGerritForStatus) Modified:
4 years ago Reviewers:
yhirano CC:
chromium-reviews, blink-reviews, loading-reviews_chromium.org, tyoshino+watch_chromium.org, Nate Chapin, gavinp+loader_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemove cancelWithError() from DocumentThreadableLoader
cancelWithError() should be removed because:
- The code creating a ResourceError when error is null in
cancelWithError() can be moved to cancel() to simplify the logic.
- Check on m_client and resource() is unnecessary when it's called from
didTimeout.
As a bonus, factor out the common logic that detaches m_client and
calls didFail()/didFailAccessControlCheck() on the detached client into
methods.
R=yhirano@chromium.org
BUG=none
Committed: https://crrev.com/fc84f8cceac793a5290e11d429ec50dd19c9f293
Cr-Commit-Position: refs/heads/master@{#437895}
Patch Set 1 #Patch Set 2 : a #
Total comments: 2
Patch Set 3 : Rebase, Addressed #9 #
Messages
Total messages: 21 (14 generated)
Description was changed from ========== Remove cancelWithError() from DocumentThreadableLoader BUG= ========== to ========== Remove cancelWithError() from DocumentThreadableLoader cancelWithError() should be removed because: - The code creating a ResourceError when error is null in cancelWithError() can be moved to cancel() to simply the logic. - Check on m_client and resource() is unnecessary when it's called from didTimeout. As a bonus, factor out the common logic that detaches m_client and calls didFail()/didFailAccessControlCheck() on the detached client into methods. BUG=none ==========
Description was changed from ========== Remove cancelWithError() from DocumentThreadableLoader cancelWithError() should be removed because: - The code creating a ResourceError when error is null in cancelWithError() can be moved to cancel() to simply the logic. - Check on m_client and resource() is unnecessary when it's called from didTimeout. As a bonus, factor out the common logic that detaches m_client and calls didFail()/didFailAccessControlCheck() on the detached client into methods. BUG=none ========== to ========== Remove cancelWithError() from DocumentThreadableLoader cancelWithError() should be removed because: - The code creating a ResourceError when error is null in cancelWithError() can be moved to cancel() to simply the logic. - Check on m_client and resource() is unnecessary when it's called from didTimeout. As a bonus, factor out the common logic that detaches m_client and calls didFail()/didFailAccessControlCheck() on the detached client into methods. BUG=none ==========
The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
tyoshino@chromium.org changed reviewers: + yhirano@chromium.org
lgtm > cancelWithError() can be moved to cancel() to simply the logic. s/simply/simplify/ https://codereview.chromium.org/2563933003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h (right): https://codereview.chromium.org/2563933003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h:113: void cancelWithError(const ResourceError&); Please remove this declaration.
Description was changed from ========== Remove cancelWithError() from DocumentThreadableLoader cancelWithError() should be removed because: - The code creating a ResourceError when error is null in cancelWithError() can be moved to cancel() to simply the logic. - Check on m_client and resource() is unnecessary when it's called from didTimeout. As a bonus, factor out the common logic that detaches m_client and calls didFail()/didFailAccessControlCheck() on the detached client into methods. BUG=none ========== to ========== Remove cancelWithError() from DocumentThreadableLoader cancelWithError() should be removed because: - The code creating a ResourceError when error is null in cancelWithError() can be moved to cancel() to simplify the logic. - Check on m_client and resource() is unnecessary when it's called from didTimeout. As a bonus, factor out the common logic that detaches m_client and calls didFail()/didFailAccessControlCheck() on the detached client into methods. BUG=none ==========
https://codereview.chromium.org/2563933003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h (right): https://codereview.chromium.org/2563933003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h:113: void cancelWithError(const ResourceError&); On 2016/12/12 08:46:55, yhirano wrote: > Please remove this declaration. Done.
On 2016/12/12 08:46:55, yhirano wrote: > > cancelWithError() can be moved to cancel() to simply the logic. > > s/simply/simplify/ Fixed!
Description was changed from ========== Remove cancelWithError() from DocumentThreadableLoader cancelWithError() should be removed because: - The code creating a ResourceError when error is null in cancelWithError() can be moved to cancel() to simplify the logic. - Check on m_client and resource() is unnecessary when it's called from didTimeout. As a bonus, factor out the common logic that detaches m_client and calls didFail()/didFailAccessControlCheck() on the detached client into methods. BUG=none ========== to ========== Remove cancelWithError() from DocumentThreadableLoader cancelWithError() should be removed because: - The code creating a ResourceError when error is null in cancelWithError() can be moved to cancel() to simplify the logic. - Check on m_client and resource() is unnecessary when it's called from didTimeout. As a bonus, factor out the common logic that detaches m_client and calls didFail()/didFailAccessControlCheck() on the detached client into methods. R=yhirano@chromium.org BUG=none ==========
The CQ bit was checked by tyoshino@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yhirano@chromium.org Link to the patchset: https://codereview.chromium.org/2563933003/#ps40001 (title: "Rebase, Addressed #9")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481557566198610,
"parent_rev": "0d5547b94b1b1e81ab29b17c1402e7a721248a28", "commit_rev":
"43984c675675bd0f31f2b3fc647bfa819c862cea"}
Message was sent while issue was closed.
Description was changed from ========== Remove cancelWithError() from DocumentThreadableLoader cancelWithError() should be removed because: - The code creating a ResourceError when error is null in cancelWithError() can be moved to cancel() to simplify the logic. - Check on m_client and resource() is unnecessary when it's called from didTimeout. As a bonus, factor out the common logic that detaches m_client and calls didFail()/didFailAccessControlCheck() on the detached client into methods. R=yhirano@chromium.org BUG=none ========== to ========== Remove cancelWithError() from DocumentThreadableLoader cancelWithError() should be removed because: - The code creating a ResourceError when error is null in cancelWithError() can be moved to cancel() to simplify the logic. - Check on m_client and resource() is unnecessary when it's called from didTimeout. As a bonus, factor out the common logic that detaches m_client and calls didFail()/didFailAccessControlCheck() on the detached client into methods. R=yhirano@chromium.org BUG=none Review-Url: https://codereview.chromium.org/2563933003 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Remove cancelWithError() from DocumentThreadableLoader cancelWithError() should be removed because: - The code creating a ResourceError when error is null in cancelWithError() can be moved to cancel() to simplify the logic. - Check on m_client and resource() is unnecessary when it's called from didTimeout. As a bonus, factor out the common logic that detaches m_client and calls didFail()/didFailAccessControlCheck() on the detached client into methods. R=yhirano@chromium.org BUG=none Review-Url: https://codereview.chromium.org/2563933003 ========== to ========== Remove cancelWithError() from DocumentThreadableLoader cancelWithError() should be removed because: - The code creating a ResourceError when error is null in cancelWithError() can be moved to cancel() to simplify the logic. - Check on m_client and resource() is unnecessary when it's called from didTimeout. As a bonus, factor out the common logic that detaches m_client and calls didFail()/didFailAccessControlCheck() on the detached client into methods. R=yhirano@chromium.org BUG=none Committed: https://crrev.com/fc84f8cceac793a5290e11d429ec50dd19c9f293 Cr-Commit-Position: refs/heads/master@{#437895} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/fc84f8cceac793a5290e11d429ec50dd19c9f293 Cr-Commit-Position: refs/heads/master@{#437895} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
