Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Side by Side Diff: src/GlobalJsObject.cpp

Issue 29393573: Issue 4692 - Dont' keep a strong reference from timer thread (Closed)
Patch Set: address comments Created March 24, 2017, 12:28 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « include/AdblockPlus/JsEngine.h ('k') | src/JsEngine.cpp » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-2016 Eyeo GmbH 3 * Copyright (C) 2006-2016 Eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 18 matching lines...) Expand all
29 #include "Thread.h" 29 #include "Thread.h"
30 #include "Utils.h" 30 #include "Utils.h"
31 31
32 using namespace AdblockPlus; 32 using namespace AdblockPlus;
33 33
34 namespace 34 namespace
35 { 35 {
36 class TimeoutThread : public Thread 36 class TimeoutThread : public Thread
37 { 37 {
38 public: 38 public:
39 TimeoutThread(JsValueList& arguments) 39 TimeoutThread(const JsEngine::TimerTask& timerTask)
40 : Thread(true) 40 : Thread(true), timerTask(timerTask)
41 { 41 {
42 if (arguments.size() < 2)
43 throw std::runtime_error("setTimeout requires at least 2 parameters");
44
45 if (!arguments[0]->IsFunction())
46 throw std::runtime_error(
47 "First argument to setTimeout must be a function");
48
49 function = arguments[0];
50 delay = arguments[1]->AsInt();
51 for (size_t i = 2; i < arguments.size(); i++)
52 functionArguments.push_back(arguments[i]);
53 } 42 }
54 43
55 void Run() 44 void Run()
56 { 45 {
57 Sleep(delay); 46 Sleep(timerTask.taskInfoIterator->delay);
58 47 if (auto jsEngine = timerTask.weakJsEngine.lock())
59 function->Call(functionArguments); 48 jsEngine->CallTimerTask(timerTask.taskInfoIterator);
60 } 49 }
61 50
62 private: 51 private:
63 JsValuePtr function; 52 JsEngine::TimerTask timerTask;
64 int delay;
65 JsValueList functionArguments;
66 }; 53 };
67 54
68 v8::Handle<v8::Value> SetTimeoutCallback(const v8::Arguments& arguments) 55 v8::Handle<v8::Value> SetTimeoutCallback(const v8::Arguments& arguments)
69 { 56 {
70 TimeoutThread* timeoutThread; 57 TimeoutThread* timeoutThread;
71 try 58 try
72 { 59 {
73 AdblockPlus::JsValueList converted = 60 auto jsEngine = AdblockPlus::JsEngine::FromArguments(arguments);
74 AdblockPlus::JsEngine::FromArguments(arguments) 61 timeoutThread = new TimeoutThread(jsEngine->CreateTimerTask(arguments));
75 ->ConvertArguments(arguments);
76 timeoutThread = new TimeoutThread(converted);
77 } 62 }
78 catch (const std::exception& e) 63 catch (const std::exception& e)
79 { 64 {
80 v8::Isolate* isolate = arguments.GetIsolate(); 65 v8::Isolate* isolate = arguments.GetIsolate();
81 return v8::ThrowException(Utils::ToV8String(isolate, e.what())); 66 return v8::ThrowException(Utils::ToV8String(isolate, e.what()));
82 } 67 }
83 timeoutThread->Start(); 68 timeoutThread->Start();
84 69
85 // We should actually return the timer ID here, which could be 70 // We should actually return the timer ID here, which could be
86 // used via clearTimeout(). But since we don't seem to need 71 // used via clearTimeout(). But since we don't seem to need
(...skipping 26 matching lines...) Expand all
113 obj->SetProperty("_fileSystem", 98 obj->SetProperty("_fileSystem",
114 FileSystemJsObject::Setup(jsEngine, jsEngine->NewObject())); 99 FileSystemJsObject::Setup(jsEngine, jsEngine->NewObject()));
115 obj->SetProperty("_webRequest", 100 obj->SetProperty("_webRequest",
116 WebRequestJsObject::Setup(jsEngine, jsEngine->NewObject())); 101 WebRequestJsObject::Setup(jsEngine, jsEngine->NewObject()));
117 obj->SetProperty("console", 102 obj->SetProperty("console",
118 ConsoleJsObject::Setup(jsEngine, jsEngine->NewObject())); 103 ConsoleJsObject::Setup(jsEngine, jsEngine->NewObject()));
119 obj->SetProperty("_appInfo", 104 obj->SetProperty("_appInfo",
120 AppInfoJsObject::Setup(jsEngine, appInfo, jsEngine->NewObject())); 105 AppInfoJsObject::Setup(jsEngine, appInfo, jsEngine->NewObject()));
121 return obj; 106 return obj;
122 } 107 }
OLDNEW
« no previous file with comments | « include/AdblockPlus/JsEngine.h ('k') | src/JsEngine.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld